From dd1ba2a2521dca5f25bbe34314a8d66fe462b053 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 20 Sep 2016 20:45:03 -0700 Subject: [PATCH] HttpObjectDecoder resetRequested not updated after reset Motivation: HttpObjectDecoder maintains a resetRequested flag which is used to determine if internal state should be reset when a decode occurs. However after a reset is done the resetRequested flag is not set to false. This leads to all data after this point being discarded. Modifications: - Set resetRequested to false when a reset is done Result: HttpObjectDecoder can still function after a reset. --- .../handler/codec/http/HttpObjectDecoder.java | 1 + .../codec/http/HttpContentDecoderTest.java | 67 ++++++++++++++++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java index 03a5eb7ca6..77e3b2411d 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java @@ -509,6 +509,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { } } + resetRequested = false; currentState = State.SKIP_CONTROL_CHARS; } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java index c08fc3bf5b..575e896f79 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java @@ -18,6 +18,8 @@ package io.netty.handler.codec.http; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.compression.ZlibCodecFactory; import io.netty.handler.codec.compression.ZlibDecoder; @@ -30,9 +32,16 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; import java.util.Queue; +import java.util.concurrent.atomic.AtomicReference; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; public class HttpContentDecoderTest { private static final String HELLO_WORLD = "hello, world"; @@ -217,6 +226,60 @@ public class HttpContentDecoderTest { assertFalse(channel.finish()); } + @Test + public void testExpectContinueResetHttpObjectDecoder() { + // request with header "Expect: 100-continue" must be replied with one "100 Continue" response + // case 5: Test that HttpObjectDecoder correctly resets its internal state after a failed expectation. + HttpRequestDecoder decoder = new HttpRequestDecoder(); + final int maxBytes = 10; + HttpObjectAggregator aggregator = new HttpObjectAggregator(maxBytes); + final AtomicReference secondRequestRef = new AtomicReference(); + EmbeddedChannel channel = new EmbeddedChannel(decoder, aggregator, new ChannelInboundHandlerAdapter() { + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + if (msg instanceof FullHttpRequest) { + if (!secondRequestRef.compareAndSet(null, (FullHttpRequest) msg)) { + ((FullHttpRequest) msg).release(); + } + } else { + ReferenceCountUtil.release(msg); + } + } + }); + String req1 = "POST /1 HTTP/1.1\r\n" + + "Content-Length: " + (maxBytes + 1) + "\r\n" + + "Expect: 100-continue\r\n" + + "\r\n"; + assertFalse(channel.writeInbound(Unpooled.wrappedBuffer(req1.getBytes(CharsetUtil.US_ASCII)))); + + FullHttpResponse resp = channel.readOutbound(); + assertEquals(HttpStatusClass.CLIENT_ERROR, resp.status().codeClass()); + resp.release(); + + String req2 = "POST /2 HTTP/1.1\r\n" + + "Content-Length: " + maxBytes + "\r\n" + + "Expect: 100-continue\r\n" + + "\r\n"; + assertFalse(channel.writeInbound(Unpooled.wrappedBuffer(req2.getBytes(CharsetUtil.US_ASCII)))); + + resp = channel.readOutbound(); + assertEquals(100, resp.status().code()); + resp.release(); + + byte[] content = new byte[maxBytes]; + assertFalse(channel.writeInbound(Unpooled.wrappedBuffer(content))); + + FullHttpRequest req = secondRequestRef.get(); + assertNotNull(req); + assertEquals("/2", req.uri()); + assertEquals(10, req.content().readableBytes()); + req.release(); + + assertHasInboundMessages(channel, false); + assertHasOutboundMessages(channel, false); + assertFalse(channel.finish()); + } + @Test public void testRequestContentLength1() { // case 1: test that ContentDecompressor either sets the correct Content-Length header