From c565805f1b268bba81fe45369babee96856bacf1 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 17 May 2019 21:18:03 +0200 Subject: [PATCH] Do not manually reset HttpObjectDecoder in HttpObjectAggregator.handleOversizedMessage(...) (#9017) (#9156) Motivation: We did manually call HttpObjectDecoder.reset() in HttpObjectAggregator.handleOversizedMessage(...) which is incorrect and will prevent correct parsing of the next message. Modifications: - Remove call to HttpObjectDecoder.reset() - Add unit test Result: Verify that we can correctly parse the next request after we rejected a request. --- .../codec/http/HttpObjectAggregator.java | 7 --- .../codec/http/HttpObjectAggregatorTest.java | 51 +++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) 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 257581fe23..192a0ed139 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 @@ -271,13 +271,6 @@ public class HttpObjectAggregator } }); } - - // If an oversized request was handled properly and the connection is still alive - // (i.e. rejected 100-continue). the decoder should prepare to handle a new message. - HttpObjectDecoder decoder = ctx.pipeline().get(HttpObjectDecoder.class); - if (decoder != null) { - decoder.reset(); - } } else if (oversized instanceof HttpResponse) { ctx.close(); throw new TooLongFrameException("Response entity too large: " + oversized); 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 ac12e5853a..f963d2d207 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 @@ -144,6 +144,57 @@ public class HttpObjectAggregatorTest { assertFalse(embedder.finish()); } + @Test + public void testOversizedRequestWithContentLengthAndDecoder() { + EmbeddedChannel embedder = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(4, false)); + assertFalse(embedder.writeInbound(Unpooled.copiedBuffer( + "PUT /upload HTTP/1.1\r\n" + + "Content-Length: 5\r\n\r\n", CharsetUtil.US_ASCII))); + + assertNull(embedder.readInbound()); + + FullHttpResponse response = embedder.readOutbound(); + assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, response.status()); + assertEquals("0", response.headers().get(HttpHeaderNames.CONTENT_LENGTH)); + + assertTrue(embedder.isOpen()); + + assertFalse(embedder.writeInbound(Unpooled.wrappedBuffer(new byte[] { 1, 2, 3, 4 }))); + assertFalse(embedder.writeInbound(Unpooled.wrappedBuffer(new byte[] { 5 }))); + + assertNull(embedder.readOutbound()); + + assertFalse(embedder.writeInbound(Unpooled.copiedBuffer( + "PUT /upload HTTP/1.1\r\n" + + "Content-Length: 2\r\n\r\n", CharsetUtil.US_ASCII))); + + assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, response.status()); + assertEquals("0", response.headers().get(HttpHeaderNames.CONTENT_LENGTH)); + + assertThat(response, instanceOf(LastHttpContent.class)); + ReferenceCountUtil.release(response); + + assertTrue(embedder.isOpen()); + + assertFalse(embedder.writeInbound(Unpooled.copiedBuffer(new byte[] { 1 }))); + assertNull(embedder.readOutbound()); + assertTrue(embedder.writeInbound(Unpooled.copiedBuffer(new byte[] { 2 }))); + assertNull(embedder.readOutbound()); + + FullHttpRequest request = embedder.readInbound(); + assertEquals(HttpVersion.HTTP_1_1, request.protocolVersion()); + assertEquals(HttpMethod.PUT, request.method()); + assertEquals("/upload", request.uri()); + assertEquals(2, HttpUtil.getContentLength(request)); + + byte[] actual = new byte[request.content().readableBytes()]; + request.content().readBytes(actual); + assertArrayEquals(new byte[] { 1, 2 }, actual); + request.release(); + + assertFalse(embedder.finish()); + } + @Test public void testOversizedRequestWithoutKeepAlive() { // send a HTTP/1.0 request with no keep-alive header