From 424bb09d2424114ee0f81a397f1079ccd21ebf64 Mon Sep 17 00:00:00 2001 From: Lionel Li Date: Mon, 16 Oct 2017 15:18:31 -0700 Subject: [PATCH] Http2StreamFrameToHttpObjectCodec should handle 100-Continue properly Motivation: Http2StreamFrameToHttpObjectCodec was not properly encoding and decoding 100-Continue HttpResponse/Http2SettingsFrame properly. It was encoding 100-Continue FullHttpResponse as an Http2SettingFrame with endStream=true, causing the child channel to terminate. It was not decoding 100-Continue Http2SettingsFrame (endStream=false) as FullHttpResponse. This should be fixed as it would cause http2 child stream to prematurely close, and could cause HttpObjectAggregator to fail if it's in the pipeline. Modification: - Fixed encode() to properly encode 100-Continue FullHttpResponse as Http2SettingsFrame with endStream=false - Reject 100-Continue HttpResponse that are NOT FullHttpResponse - Fixed decode() to properly decode 100-Continue Http2SettingsFrame (endStream=false) as a FullHttpResponse - made Http2StreamFrameToHttpObjectCodec sharable so that it can b used among child streams within the same Http2MultiplexCodec Result: Now Http2StreamFrameToHttpObjectCodec should be properly handling 100-Continue responses. --- .../Http2StreamFrameToHttpObjectCodec.java | 47 +++++++++++++++++- ...Http2StreamFrameToHttpObjectCodecTest.java | 49 ++++++++++++++++++- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodec.java index 0e2d052bd6..ed38d84d8c 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodec.java @@ -20,23 +20,27 @@ import io.netty.buffer.ByteBufAllocator; import io.netty.buffer.Unpooled; import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelHandler.Sharable; import io.netty.channel.ChannelHandlerContext; +import io.netty.handler.codec.EncoderException; import io.netty.handler.codec.MessageToMessageCodec; import io.netty.handler.codec.http.DefaultHttpContent; import io.netty.handler.codec.http.DefaultLastHttpContent; import io.netty.handler.codec.http.FullHttpMessage; +import io.netty.handler.codec.http.FullHttpResponse; import io.netty.handler.codec.http.HttpContent; import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaderValues; import io.netty.handler.codec.http.HttpMessage; import io.netty.handler.codec.http.HttpObject; import io.netty.handler.codec.http.HttpRequest; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpScheme; import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.codec.http.LastHttpContent; import io.netty.handler.ssl.SslHandler; -import io.netty.util.ReferenceCountUtil; import io.netty.util.internal.UnstableApi; import java.util.List; @@ -51,6 +55,7 @@ import java.util.List; * is a single header. */ @UnstableApi +@Sharable public class Http2StreamFrameToHttpObjectCodec extends MessageToMessageCodec { private final boolean isServer; private final boolean validateHeaders; @@ -80,8 +85,18 @@ public class Http2StreamFrameToHttpObjectCodec extends MessageToMessageCodec out) throws Exception { + // 100-continue is typically a FullHttpResponse, but the decoded + // Http2HeadersFrame should not be marked as endStream=true + if (obj instanceof HttpResponse) { + final HttpResponse res = (HttpResponse) obj; + if (res.status().equals(HttpResponseStatus.CONTINUE)) { + if (res instanceof FullHttpResponse) { + final Http2Headers headers = toHttp2Headers(res); + out.add(new DefaultHttp2HeadersFrame(headers, false)); + return; + } else { + throw new EncoderException( + HttpResponseStatus.CONTINUE.toString() + " must be a FullHttpResponse"); + } + } + } + if (obj instanceof HttpMessage) { Http2Headers headers = toHttp2Headers((HttpMessage) obj); boolean noMoreFrames = false; diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodecTest.java index 5ef309590a..f8fb1f6a88 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodecTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamFrameToHttpObjectCodecTest.java @@ -23,6 +23,7 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelPromise; import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.codec.EncoderException; import io.netty.handler.codec.http.DefaultFullHttpRequest; import io.netty.handler.codec.http.DefaultFullHttpResponse; import io.netty.handler.codec.http.DefaultHttpContent; @@ -43,7 +44,6 @@ import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.LastHttpContent; import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslContextBuilder; -import io.netty.handler.ssl.SslHandler; import io.netty.handler.ssl.SslProvider; import io.netty.util.CharsetUtil; @@ -51,7 +51,6 @@ import org.junit.Test; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; @@ -76,6 +75,31 @@ public class Http2StreamFrameToHttpObjectCodecTest { assertFalse(ch.finish()); } + @Test + public void encode100ContinueAsHttp2HeadersFrameThatIsNotEndStream() throws Exception { + EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(true)); + assertTrue(ch.writeOutbound(new DefaultFullHttpResponse( + HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE))); + + Http2HeadersFrame headersFrame = ch.readOutbound(); + assertThat(headersFrame.headers().status().toString(), is("100")); + assertFalse(headersFrame.isEndStream()); + + assertThat(ch.readOutbound(), is(nullValue())); + assertFalse(ch.finish()); + } + + @Test (expected = EncoderException.class) + public void encodeNonFullHttpResponse100ContinueIsRejected() throws Exception { + EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(true)); + try { + ch.writeOutbound(new DefaultHttpResponse( + HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE)); + } finally { + ch.finishAndReleaseAll(); + } + } + @Test public void testUpgradeNonEmptyFullResponse() throws Exception { EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(true)); @@ -659,6 +683,27 @@ public class Http2StreamFrameToHttpObjectCodecTest { assertFalse(ch.finish()); } + @Test + public void decode100ContinueHttp2HeadersAsFullHttpResponse() throws Exception { + EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(false)); + Http2Headers headers = new DefaultHttp2Headers(); + headers.scheme(HttpScheme.HTTP.name()); + headers.status(HttpResponseStatus.CONTINUE.codeAsText()); + + assertTrue(ch.writeInbound(new DefaultHttp2HeadersFrame(headers, false))); + + final FullHttpResponse response = ch.readInbound(); + try { + assertThat(response.status(), is(HttpResponseStatus.CONTINUE)); + assertThat(response.protocolVersion(), is(HttpVersion.HTTP_1_1)); + } finally { + response.release(); + } + + assertThat(ch.readInbound(), is(nullValue())); + assertFalse(ch.finish()); + } + @Test public void testDecodeResponseHeaders() throws Exception { EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(false));