diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java index ea87e03db..7871dd4cb 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamHandler.java @@ -41,10 +41,12 @@ import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.HttpVersion; +import org.apache.hc.core5.http.Method; import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.impl.BasicHttpConnectionMetrics; import org.apache.hc.core5.http.impl.IncomingEntityDetails; import org.apache.hc.core5.http.impl.nio.MessageState; +import org.apache.hc.core5.http.message.MessageSupport; import org.apache.hc.core5.http.message.StatusLine; import org.apache.hc.core5.http.nio.AsyncClientExchangeHandler; import org.apache.hc.core5.http.nio.AsyncPushConsumer; @@ -72,6 +74,10 @@ class ClientH2StreamHandler implements H2StreamHandler { private final AtomicBoolean failed; private final AtomicBoolean done; + private volatile String method = null; + private volatile long declaredContentLen = -1; + private volatile long actualContentLen = 0; + ClientH2StreamHandler( final H2StreamChannel outputChannel, final HttpProcessor httpProcessor, @@ -141,6 +147,8 @@ private void commitRequest(final HttpRequest request, final EntityDetails entity httpProcessor.process(request, entityDetails, context); + method = request.getMethod(); + final List
headers = DefaultH2RequestConverter.INSTANCE.convert(request); if (entityDetails == null) { requestState.set(MessageState.COMPLETE); @@ -207,6 +215,13 @@ public void consumeHeader(final List
headers, final boolean endStream) t return; } + if (!Method.HEAD.isSame(method) && MessageSupport.canResponseHaveBody(method, response)) { + declaredContentLen = MessageSupport.getContentLength(response); + if (endStream) { + validateContentLength(); + } + } + final EntityDetails entityDetails = endStream ? null : new IncomingEntityDetails(response, -1); context.setResponse(response); httpProcessor.process(response, entityDetails, context); @@ -236,14 +251,22 @@ public void consumeData(final ByteBuffer src, final boolean endStream) throws Ht throw new ProtocolException("Unexpected message data"); } if (src != null) { + actualContentLen += src.remaining(); exchangeHandler.consume(src); } if (endStream) { responseState.set(MessageState.COMPLETE); + validateContentLength(); exchangeHandler.streamEnd(null); } } + private void validateContentLength() throws ProtocolException { + if (declaredContentLen >= 0 && declaredContentLen != actualContentLen) { + throw new ProtocolException("Invalid content-length (does not equal the sum of data payload)"); + } + } + @Override public void handle(final HttpException ex, final boolean endStream) throws HttpException, IOException { throw ex; diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientPushH2StreamHandler.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientPushH2StreamHandler.java index d911c689e..3b32536ca 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientPushH2StreamHandler.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientPushH2StreamHandler.java @@ -41,6 +41,7 @@ import org.apache.hc.core5.http.impl.BasicHttpConnectionMetrics; import org.apache.hc.core5.http.impl.IncomingEntityDetails; import org.apache.hc.core5.http.impl.nio.MessageState; +import org.apache.hc.core5.http.message.MessageSupport; import org.apache.hc.core5.http.nio.AsyncPushConsumer; import org.apache.hc.core5.http.nio.HandlerFactory; import org.apache.hc.core5.http.protocol.HttpCoreContext; @@ -67,6 +68,9 @@ class ClientPushH2StreamHandler implements H2StreamHandler { private volatile MessageState requestState; private volatile MessageState responseState; + private volatile long declaredContentLen = -1; + private volatile long actualContentLen = 0; + ClientPushH2StreamHandler( final H2StreamChannel outputChannel, final HttpProcessor httpProcessor, @@ -132,6 +136,14 @@ public void consumeHeader(final List
headers, final boolean endStream) t Asserts.notNull(exchangeHandler, "Exchange handler"); final HttpResponse response = DefaultH2ResponseConverter.INSTANCE.convert(headers); + + if (MessageSupport.canResponseHaveBody(response)) { + declaredContentLen = MessageSupport.getContentLength(response); + if (endStream) { + validateContentLength(); + } + } + final EntityDetails entityDetails = endStream ? null : new IncomingEntityDetails(request, -1); context.setResponse(response); @@ -163,14 +175,22 @@ public void consumeData(final ByteBuffer src, final boolean endStream) throws Ht } Asserts.notNull(exchangeHandler, "Exchange handler"); if (src != null) { + actualContentLen += src.remaining(); exchangeHandler.consume(src); } if (endStream) { responseState = MessageState.COMPLETE; + validateContentLength(); exchangeHandler.streamEnd(null); } } + private void validateContentLength() throws ProtocolException { + if (declaredContentLen >= 0 && declaredContentLen != actualContentLen) { + throw new ProtocolException("Invalid content-length (does not equal the sum of data payload)"); + } + } + public boolean isDone() { return responseState == MessageState.COMPLETE; } diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamHandler.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamHandler.java index 2313d2ec9..a51aea1e6 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamHandler.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamHandler.java @@ -45,6 +45,7 @@ import org.apache.hc.core5.http.impl.IncomingEntityDetails; import org.apache.hc.core5.http.impl.ServerSupport; import org.apache.hc.core5.http.impl.nio.MessageState; +import org.apache.hc.core5.http.message.MessageSupport; import org.apache.hc.core5.http.nio.AsyncPushConsumer; import org.apache.hc.core5.http.nio.AsyncPushProducer; import org.apache.hc.core5.http.nio.AsyncResponseProducer; @@ -81,6 +82,8 @@ class ServerH2StreamHandler implements H2StreamHandler { private volatile AsyncServerExchangeHandler exchangeHandler; private volatile HttpRequest receivedRequest; + private volatile long declaredContentLen = -1; + private volatile long actualContentLen = 0; ServerH2StreamHandler( final H2StreamChannel outputChannel, @@ -230,6 +233,12 @@ public void consumeHeader(final List
headers, final boolean endStream) t requestState.set(endStream ? MessageState.COMPLETE : MessageState.BODY); final HttpRequest request = DefaultH2RequestConverter.INSTANCE.convert(headers); + + declaredContentLen = MessageSupport.getContentLength(request); + if (endStream) { + validateContentLength(); + } + final EntityDetails requestEntityDetails = endStream ? null : new IncomingEntityDetails(request, -1); final AsyncServerExchangeHandler handler; @@ -295,14 +304,22 @@ public void consumeData(final ByteBuffer src, final boolean endStream) throws Ht } Asserts.notNull(exchangeHandler, "Exchange handler"); if (src != null) { + actualContentLen += src.remaining(); exchangeHandler.consume(src); } if (endStream) { requestState.set(MessageState.COMPLETE); + validateContentLength(); exchangeHandler.streamEnd(null); } } + private void validateContentLength() throws ProtocolException { + if (declaredContentLen >= 0 && declaredContentLen != actualContentLen) { + throw new ProtocolException("Invalid content-length (does not equal the sum of data payload)"); + } + } + @Override public boolean isOutputReady() { return responseState.get() == MessageState.BODY && exchangeHandler != null && exchangeHandler.available() > 0; diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamHandler.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamHandler.java index ff3f9ea74..1df777354 100644 --- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamHandler.java +++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamHandler.java @@ -27,6 +27,7 @@ package org.apache.hc.core5.http2.impl.nio; import java.nio.ByteBuffer; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -116,4 +117,34 @@ void consumeTrailersWithPseudoHeaderRejected() throws Exception { Mockito.verify(exchangeHandler, Mockito.never()).streamEnd(Mockito.anyList()); } + @Test + void contentLengthValid() throws Exception { + final List
responseHeaders = Arrays.asList( + new BasicHeader(":status", "200"), + new BasicHeader("content-length", "12")); + handler.consumeHeader(responseHeaders, false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }), false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1 }), true); + } + + @Test + void contentLengthInvalid() throws Exception { + final List
responseHeaders = Arrays.asList( + new BasicHeader(":status", "200"), + new BasicHeader("content-length", "12")); + handler.consumeHeader(responseHeaders, false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }), false); + Assertions.assertThrows(ProtocolException.class, () -> + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2 }), true)); + } + + @Test + void contentLengthInvalidNoBody() throws Exception { + final List
responseHeaders = Arrays.asList( + new BasicHeader(":status", "200"), + new BasicHeader("content-length", "12")); + Assertions.assertThrows(ProtocolException.class, () -> + handler.consumeHeader(responseHeaders, true)); + } + } diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientPushH2StreamHandler.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientPushH2StreamHandler.java index a7264c0be..8fe51fc24 100644 --- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientPushH2StreamHandler.java +++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientPushH2StreamHandler.java @@ -27,7 +27,10 @@ package org.apache.hc.core5.http2.impl.nio; import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.List; +import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.impl.BasicHttpConnectionMetrics; import org.apache.hc.core5.http.impl.BasicHttpTransportMetrics; @@ -42,6 +45,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; class TestClientPushH2StreamHandler { @@ -51,6 +55,8 @@ class TestClientPushH2StreamHandler { @Mock HttpProcessor httpProcessor; @Mock + AsyncPushConsumer pushConsumer; + @Mock HandlerFactory pushHandlerFactory; ClientPushH2StreamHandler handler; @@ -88,4 +94,55 @@ void updateCapacityFailsWithoutHandler() { Assertions.assertThrows(IllegalStateException.class, handler::updateInputCapacity); } + @Test + void contentLengthValid() throws Exception { + Mockito.when(pushHandlerFactory.create(Mockito.any(), Mockito.any())).thenReturn(pushConsumer); + handler.consumePromise(java.util.Arrays.asList( + new BasicHeader(":method", "GET"), + new BasicHeader(":scheme", "https"), + new BasicHeader(":authority", "example.com"), + new BasicHeader(":path", "/"))); + + final List
responseHeaders = Arrays.asList( + new BasicHeader(":status", "200"), + new BasicHeader("content-length", "12")); + handler.consumeHeader(responseHeaders, false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }), false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1 }), true); + } + + @Test + void contentLengthInvalid() throws Exception { + Mockito.when(pushHandlerFactory.create(Mockito.any(), Mockito.any())).thenReturn(pushConsumer); + handler.consumePromise(java.util.Arrays.asList( + new BasicHeader(":method", "GET"), + new BasicHeader(":scheme", "https"), + new BasicHeader(":authority", "example.com"), + new BasicHeader(":path", "/"))); + + final List
responseHeaders = Arrays.asList( + new BasicHeader(":status", "200"), + new BasicHeader("content-length", "12")); + handler.consumeHeader(responseHeaders, false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }), false); + Assertions.assertThrows(ProtocolException.class, () -> + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2 }), true)); + } + + @Test + void contentLengthInvalidNoBody() throws Exception { + Mockito.when(pushHandlerFactory.create(Mockito.any(), Mockito.any())).thenReturn(pushConsumer); + handler.consumePromise(java.util.Arrays.asList( + new BasicHeader(":method", "GET"), + new BasicHeader(":scheme", "https"), + new BasicHeader(":authority", "example.com"), + new BasicHeader(":path", "/"))); + + final List
responseHeaders = Arrays.asList( + new BasicHeader(":status", "200"), + new BasicHeader("content-length", "12")); + Assertions.assertThrows(ProtocolException.class, () -> + handler.consumeHeader(responseHeaders, true)); + } + } diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamHandler.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamHandler.java index 28c1d539b..be33ee8ae 100644 --- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamHandler.java +++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamHandler.java @@ -136,4 +136,49 @@ void consumeTrailersWithoutEndStreamRejected() throws Exception { Mockito.verify(exchangeHandler, Mockito.never()).streamEnd(ArgumentMatchers.anyList()); } + @Test + void contentLengthValid() throws Exception { + Mockito.when(exchangeHandlerFactory.create(ArgumentMatchers.any(), ArgumentMatchers.any())) + .thenReturn(exchangeHandler); + final List
requestHeaders = Arrays.asList( + new BasicHeader(":method", "POST"), + new BasicHeader(":scheme", "https"), + new BasicHeader(":authority", "example.test"), + new BasicHeader(":path", "/upload"), + new BasicHeader("content-length", "12")); + handler.consumeHeader(requestHeaders, false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }), false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1 }), true); + } + + @Test + void contentLengthInvalid() throws Exception { + Mockito.when(exchangeHandlerFactory.create(ArgumentMatchers.any(), ArgumentMatchers.any())) + .thenReturn(exchangeHandler); + final List
requestHeaders = Arrays.asList( + new BasicHeader(":method", "POST"), + new BasicHeader(":scheme", "https"), + new BasicHeader(":authority", "example.test"), + new BasicHeader(":path", "/upload"), + new BasicHeader("content-length", "12")); + handler.consumeHeader(requestHeaders, false); + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }), false); + Assertions.assertThrows(ProtocolException.class, () -> + handler.consumeData(ByteBuffer.wrap(new byte[] { 0, 1, 2 }), true)); + } + + @Test + void contentLengthInvalidNoBody() throws Exception { + Mockito.when(exchangeHandlerFactory.create(ArgumentMatchers.any(), ArgumentMatchers.any())) + .thenReturn(exchangeHandler); + final List
requestHeaders = Arrays.asList( + new BasicHeader(":method", "POST"), + new BasicHeader(":scheme", "https"), + new BasicHeader(":authority", "example.test"), + new BasicHeader(":path", "/upload"), + new BasicHeader("content-length", "12")); + Assertions.assertThrows(ProtocolException.class, () -> + handler.consumeHeader(requestHeaders, true)); + } + }