diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java index ab1f5ef16c..bbb073a449 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java @@ -161,10 +161,9 @@ public class HttpObjectAggregator }); // If the client started to send data already, close because it's impossible to recover. - // If 'Expect: 100-continue' is missing, close becuase it's impossible to recover. - // If keep-alive is off, no need to leave the connection open. + // If keep-alive is off and 'Expect: 100-continue' is missing, no need to leave the connection open. if (oversized instanceof FullHttpMessage || - !HttpHeaders.is100ContinueExpected(oversized) || !HttpHeaders.isKeepAlive(oversized)) { + (!HttpHeaders.is100ContinueExpected(oversized) && !HttpHeaders.isKeepAlive(oversized))) { future.addListener(ChannelFutureListener.CLOSE); } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java index 276c305a27..21d9b865ae 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java @@ -223,6 +223,53 @@ public class HttpObjectAggregatorTest { assertFalse(embedder.finish()); } + @Test + public void testRequestAfterOversized100ContinueAndDecoder() { + EmbeddedChannel embedder = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(15)); + + // Write first request with Expect: 100-continue + HttpRequest message = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "http://localhost"); + HttpHeaders.set100ContinueExpected(message); + HttpHeaders.setContentLength(message, 16); + + HttpContent chunk1 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("some", CharsetUtil.US_ASCII))); + HttpContent chunk2 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("test", CharsetUtil.US_ASCII))); + HttpContent chunk3 = LastHttpContent.EMPTY_LAST_CONTENT; + + // Send a request with 100-continue + large Content-Length header value. + assertFalse(embedder.writeInbound(message)); + + // The agregator should respond with '413 Request Entity Too Large.' + FullHttpResponse response = embedder.readOutbound(); + assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, response.status()); + assertEquals("0", response.headers().get(Names.CONTENT_LENGTH)); + + // An ill-behaving client could continue to send data without a respect, and such data should be discarded. + assertFalse(embedder.writeInbound(chunk1)); + + // The aggregator should not close the connection because keep-alive is on. + assertTrue(embedder.isOpen()); + + // Now send a valid request. + HttpRequest message2 = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "http://localhost"); + + assertFalse(embedder.writeInbound(message2)); + assertFalse(embedder.writeInbound(chunk2)); + assertTrue(embedder.writeInbound(chunk3)); + + FullHttpRequest fullMsg = embedder.readInbound(); + assertNotNull(fullMsg); + + assertEquals( + chunk2.content().readableBytes() + chunk3.content().readableBytes(), + HttpHeaders.getContentLength(fullMsg)); + + assertEquals(HttpHeaders.getContentLength(fullMsg), fullMsg.content().readableBytes()); + + fullMsg.release(); + assertFalse(embedder.finish()); + } + private static void checkOversizedRequest(HttpRequest message) { EmbeddedChannel embedder = new EmbeddedChannel(new HttpObjectAggregator(4)); @@ -230,8 +277,25 @@ public class HttpObjectAggregatorTest { HttpResponse response = embedder.readOutbound(); assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, response.status()); assertEquals("0", response.headers().get(Names.CONTENT_LENGTH)); - assertFalse(embedder.isOpen()); - assertFalse(embedder.finish()); + + if (serverShouldCloseConnection(message)) { + assertFalse(embedder.isOpen()); + assertFalse(embedder.finish()); + } else { + assertTrue(embedder.isOpen()); + } + } + + private static boolean serverShouldCloseConnection(HttpRequest message) { + // The connection should only be kept open if Expect: 100-continue is set, + // or if keep-alive is on. + if (HttpHeaders.is100ContinueExpected(message)) { + return false; + } + if (HttpHeaders.isKeepAlive(message)) { + return false; + } + return true; } @Test