From 52e19d5c631d9cbe6cc41b5ea34d7493160f26ee Mon Sep 17 00:00:00 2001 From: Daniel Schobel Date: Thu, 10 Aug 2017 17:50:00 -0600 Subject: [PATCH] Strip http 'expect' headers when expectation response is produced Motivation: HttpObjectAggregator differs from HttpServerExpectContinueHandler's handling of expect headers by not stripping the 'expect' header when a response is generated. Modifications: HttpObjectAggregator now removes the 'expect' header in cases where it generates a response. Result: Consistent and correct behavior between HttpObjectAggregator and HttpServerExpectContinueHandler. --- .../codec/http/HttpObjectAggregator.java | 15 +++++++++++++-- .../codec/http/HttpObjectAggregatorTest.java | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 2 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 3b3fa42207..0b8bc2e29b 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 @@ -30,6 +30,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import static io.netty.handler.codec.http.HttpHeaderNames.CONNECTION; import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_LENGTH; +import static io.netty.handler.codec.http.HttpHeaderNames.EXPECT; import static io.netty.handler.codec.http.HttpUtil.getContentLength; /** @@ -158,8 +159,7 @@ public class HttpObjectAggregator } } - @Override - protected Object newContinueResponse(HttpMessage start, int maxContentLength, ChannelPipeline pipeline) { + private Object continueResponse(HttpMessage start, int maxContentLength, ChannelPipeline pipeline) { if (HttpUtil.isUnsupportedExpectation(start)) { // if the request contains an unsupported expectation, we return 417 pipeline.fireUserEventTriggered(HttpExpectationFailedEvent.INSTANCE); @@ -176,6 +176,17 @@ public class HttpObjectAggregator return null; } + @Override + protected Object newContinueResponse(HttpMessage start, int maxContentLength, ChannelPipeline pipeline) { + Object response = continueResponse(start, maxContentLength, pipeline); + // we're going to respond based on the request expectation so there's no + // need to propagate the expectation further. + if (response != null) { + start.headers().remove(EXPECT); + } + return response; + } + @Override protected boolean closeAfterContinueResponse(Object msg) { return closeOnExpectationFailed && ignoreContentAfterContinueResponse(msg); 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 8a100a7680..7dc0ac5a51 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 @@ -370,6 +370,23 @@ public class HttpObjectAggregatorTest { assertFalse(embedder.finish()); } + @Test + public void testValidRequestWith100ContinueAndDecoder() { + EmbeddedChannel embedder = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(100)); + embedder.writeInbound(Unpooled.copiedBuffer( + "GET /upload HTTP/1.1\r\n" + + "Expect: 100-continue\r\n" + + "Content-Length: 0\r\n\r\n", CharsetUtil.US_ASCII)); + + FullHttpResponse response = embedder.readOutbound(); + assertEquals(HttpResponseStatus.CONTINUE, response.status()); + FullHttpRequest request = embedder.readInbound(); + assertFalse(request.headers().contains(HttpHeaderNames.EXPECT)); + request.release(); + response.release(); + assertFalse(embedder.finish()); + } + @Test public void testOversizedRequestWith100ContinueAndDecoder() { EmbeddedChannel embedder = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(4));