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.
This commit is contained in:
parent
8aeba78ecc
commit
424bb09d24
@ -20,23 +20,27 @@ import io.netty.buffer.ByteBufAllocator;
|
|||||||
import io.netty.buffer.Unpooled;
|
import io.netty.buffer.Unpooled;
|
||||||
import io.netty.channel.Channel;
|
import io.netty.channel.Channel;
|
||||||
import io.netty.channel.ChannelHandler;
|
import io.netty.channel.ChannelHandler;
|
||||||
|
import io.netty.channel.ChannelHandler.Sharable;
|
||||||
import io.netty.channel.ChannelHandlerContext;
|
import io.netty.channel.ChannelHandlerContext;
|
||||||
|
import io.netty.handler.codec.EncoderException;
|
||||||
import io.netty.handler.codec.MessageToMessageCodec;
|
import io.netty.handler.codec.MessageToMessageCodec;
|
||||||
import io.netty.handler.codec.http.DefaultHttpContent;
|
import io.netty.handler.codec.http.DefaultHttpContent;
|
||||||
import io.netty.handler.codec.http.DefaultLastHttpContent;
|
import io.netty.handler.codec.http.DefaultLastHttpContent;
|
||||||
import io.netty.handler.codec.http.FullHttpMessage;
|
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.HttpContent;
|
||||||
import io.netty.handler.codec.http.HttpHeaderNames;
|
import io.netty.handler.codec.http.HttpHeaderNames;
|
||||||
import io.netty.handler.codec.http.HttpHeaderValues;
|
import io.netty.handler.codec.http.HttpHeaderValues;
|
||||||
import io.netty.handler.codec.http.HttpMessage;
|
import io.netty.handler.codec.http.HttpMessage;
|
||||||
import io.netty.handler.codec.http.HttpObject;
|
import io.netty.handler.codec.http.HttpObject;
|
||||||
import io.netty.handler.codec.http.HttpRequest;
|
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.HttpScheme;
|
||||||
import io.netty.handler.codec.http.HttpUtil;
|
import io.netty.handler.codec.http.HttpUtil;
|
||||||
import io.netty.handler.codec.http.HttpVersion;
|
import io.netty.handler.codec.http.HttpVersion;
|
||||||
import io.netty.handler.codec.http.LastHttpContent;
|
import io.netty.handler.codec.http.LastHttpContent;
|
||||||
import io.netty.handler.ssl.SslHandler;
|
import io.netty.handler.ssl.SslHandler;
|
||||||
import io.netty.util.ReferenceCountUtil;
|
|
||||||
import io.netty.util.internal.UnstableApi;
|
import io.netty.util.internal.UnstableApi;
|
||||||
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
@ -51,6 +55,7 @@ import java.util.List;
|
|||||||
* is a single header.
|
* is a single header.
|
||||||
*/
|
*/
|
||||||
@UnstableApi
|
@UnstableApi
|
||||||
|
@Sharable
|
||||||
public class Http2StreamFrameToHttpObjectCodec extends MessageToMessageCodec<Http2StreamFrame, HttpObject> {
|
public class Http2StreamFrameToHttpObjectCodec extends MessageToMessageCodec<Http2StreamFrame, HttpObject> {
|
||||||
private final boolean isServer;
|
private final boolean isServer;
|
||||||
private final boolean validateHeaders;
|
private final boolean validateHeaders;
|
||||||
@ -80,8 +85,18 @@ public class Http2StreamFrameToHttpObjectCodec extends MessageToMessageCodec<Htt
|
|||||||
Http2HeadersFrame headersFrame = (Http2HeadersFrame) frame;
|
Http2HeadersFrame headersFrame = (Http2HeadersFrame) frame;
|
||||||
Http2Headers headers = headersFrame.headers();
|
Http2Headers headers = headersFrame.headers();
|
||||||
|
|
||||||
|
final CharSequence status = headers.status();
|
||||||
|
|
||||||
|
// 100-continue response is a special case where Http2HeadersFrame#isEndStream=false
|
||||||
|
// but we need to decode it as a FullHttpResponse to play nice with HttpObjectAggregator.
|
||||||
|
if (null != status && HttpResponseStatus.CONTINUE.codeAsText().contentEquals(status)) {
|
||||||
|
final FullHttpMessage fullMsg = newFullMessage(id, headers, ctx.alloc());
|
||||||
|
out.add(fullMsg);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (headersFrame.isEndStream()) {
|
if (headersFrame.isEndStream()) {
|
||||||
if (headers.method() == null && headers.status() == null) {
|
if (headers.method() == null && status == null) {
|
||||||
LastHttpContent last = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER, validateHeaders);
|
LastHttpContent last = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER, validateHeaders);
|
||||||
HttpConversionUtil.addHttp2ToHttpHeaders(id, headers, last.trailingHeaders(),
|
HttpConversionUtil.addHttp2ToHttpHeaders(id, headers, last.trailingHeaders(),
|
||||||
HttpVersion.HTTP_1_1, true, true);
|
HttpVersion.HTTP_1_1, true, true);
|
||||||
@ -118,8 +133,36 @@ public class Http2StreamFrameToHttpObjectCodec extends MessageToMessageCodec<Htt
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Encode from an {@link HttpObject} to an {@link Http2StreamFrame}. This method will
|
||||||
|
* be called for each written message that can be handled by this encoder.
|
||||||
|
*
|
||||||
|
* NOTE: 100-Continue responses that are NOT {@link FullHttpResponse} will be rejected.
|
||||||
|
*
|
||||||
|
* @param ctx the {@link ChannelHandlerContext} which this handler belongs to
|
||||||
|
* @param obj the {@link HttpObject} message to encode
|
||||||
|
* @param out the {@link List} into which the encoded msg should be added
|
||||||
|
* needs to do some kind of aggregation
|
||||||
|
* @throws Exception is thrown if an error occurs
|
||||||
|
*/
|
||||||
@Override
|
@Override
|
||||||
protected void encode(ChannelHandlerContext ctx, HttpObject obj, List<Object> out) throws Exception {
|
protected void encode(ChannelHandlerContext ctx, HttpObject obj, List<Object> 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) {
|
if (obj instanceof HttpMessage) {
|
||||||
Http2Headers headers = toHttp2Headers((HttpMessage) obj);
|
Http2Headers headers = toHttp2Headers((HttpMessage) obj);
|
||||||
boolean noMoreFrames = false;
|
boolean noMoreFrames = false;
|
||||||
|
@ -23,6 +23,7 @@ import io.netty.channel.ChannelHandlerContext;
|
|||||||
import io.netty.channel.ChannelOutboundHandlerAdapter;
|
import io.netty.channel.ChannelOutboundHandlerAdapter;
|
||||||
import io.netty.channel.ChannelPromise;
|
import io.netty.channel.ChannelPromise;
|
||||||
import io.netty.channel.embedded.EmbeddedChannel;
|
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.DefaultFullHttpRequest;
|
||||||
import io.netty.handler.codec.http.DefaultFullHttpResponse;
|
import io.netty.handler.codec.http.DefaultFullHttpResponse;
|
||||||
import io.netty.handler.codec.http.DefaultHttpContent;
|
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.codec.http.LastHttpContent;
|
||||||
import io.netty.handler.ssl.SslContext;
|
import io.netty.handler.ssl.SslContext;
|
||||||
import io.netty.handler.ssl.SslContextBuilder;
|
import io.netty.handler.ssl.SslContextBuilder;
|
||||||
import io.netty.handler.ssl.SslHandler;
|
|
||||||
import io.netty.handler.ssl.SslProvider;
|
import io.netty.handler.ssl.SslProvider;
|
||||||
import io.netty.util.CharsetUtil;
|
import io.netty.util.CharsetUtil;
|
||||||
|
|
||||||
@ -51,7 +51,6 @@ import org.junit.Test;
|
|||||||
|
|
||||||
import java.util.Queue;
|
import java.util.Queue;
|
||||||
import java.util.concurrent.ConcurrentLinkedQueue;
|
import java.util.concurrent.ConcurrentLinkedQueue;
|
||||||
import java.util.concurrent.atomic.AtomicReference;
|
|
||||||
|
|
||||||
import static org.hamcrest.CoreMatchers.is;
|
import static org.hamcrest.CoreMatchers.is;
|
||||||
import static org.hamcrest.CoreMatchers.nullValue;
|
import static org.hamcrest.CoreMatchers.nullValue;
|
||||||
@ -76,6 +75,31 @@ public class Http2StreamFrameToHttpObjectCodecTest {
|
|||||||
assertFalse(ch.finish());
|
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
|
@Test
|
||||||
public void testUpgradeNonEmptyFullResponse() throws Exception {
|
public void testUpgradeNonEmptyFullResponse() throws Exception {
|
||||||
EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(true));
|
EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(true));
|
||||||
@ -659,6 +683,27 @@ public class Http2StreamFrameToHttpObjectCodecTest {
|
|||||||
assertFalse(ch.finish());
|
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
|
@Test
|
||||||
public void testDecodeResponseHeaders() throws Exception {
|
public void testDecodeResponseHeaders() throws Exception {
|
||||||
EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(false));
|
EmbeddedChannel ch = new EmbeddedChannel(new Http2StreamFrameToHttpObjectCodec(false));
|
||||||
|
Loading…
Reference in New Issue
Block a user