From ecdb2feff432898b1616bebb8da9a4c80d90f8c7 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 12 Dec 2016 07:54:58 +0100 Subject: [PATCH] [#5831] HttpServerCodec cannot encode a respons e to HEAD request with a 'content-encoding: chunked' header Motivation: It is valid to send a response to a HEAD request that contains a transfer-encoding: chunked header, but it is not valid to include a body, and there is no way to do this using the netty4 HttpServerCodec. The root cause is that the netty4 HttpObjectEncoder will transition to the state ST_CONTENT_CHUNK and the only way to transition back to ST_INIT is through the encodeChunkedContent method which will write the terminating length (0\r\n\r\n\r\n), a protocol error when responding to a HEAD request Modifications: - Keep track of the method of the request and depending on it handle the response differently when encoding it. - Added a unit test. Result: Correclty handle HEAD responses that are chunked. --- .../handler/codec/http/HttpObjectEncoder.java | 74 +++++++++++-------- .../handler/codec/http/HttpServerCodec.java | 59 +++++++++++++-- .../codec/http/HttpServerCodecTest.java | 31 ++++++++ 3 files changed, 127 insertions(+), 37 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java index 65cf4310c5..4a5a1c93c1 100755 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java @@ -51,6 +51,7 @@ public abstract class HttpObjectEncoder extends MessageTo private static final int ST_INIT = 0; private static final int ST_CONTENT_NON_CHUNK = 1; private static final int ST_CONTENT_CHUNK = 2; + private static final int ST_CONTENT_ALWAYS_EMPTY = 3; @SuppressWarnings("RedundantFieldInitialization") private int state = ST_INIT; @@ -71,7 +72,8 @@ public abstract class HttpObjectEncoder extends MessageTo encodeInitialLine(buf, m); encodeHeaders(m.headers(), buf); buf.writeBytes(CRLF); - state = HttpHeaders.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK; + state = isContentAlwaysEmpty(m) ? ST_CONTENT_ALWAYS_EMPTY : + HttpHeaders.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK; } // Bypass the encoder in case of an empty buffer, so that the following idiom works: @@ -86,44 +88,50 @@ public abstract class HttpObjectEncoder extends MessageTo } if (msg instanceof HttpContent || msg instanceof ByteBuf || msg instanceof FileRegion) { - - if (state == ST_INIT) { - throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg)); - } - - final long contentLength = contentLength(msg); - if (state == ST_CONTENT_NON_CHUNK) { - if (contentLength > 0) { - if (buf != null && buf.writableBytes() >= contentLength && msg instanceof HttpContent) { - // merge into other buffer for performance reasons - buf.writeBytes(((HttpContent) msg).content()); - out.add(buf); + switch (state) { + case ST_INIT: + throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg)); + case ST_CONTENT_ALWAYS_EMPTY: + out.add(EMPTY_BUFFER); + if (msg instanceof LastHttpContent) { + state = ST_INIT; + } + return; + case ST_CONTENT_NON_CHUNK: + final long contentLength = contentLength(msg); + if (contentLength > 0) { + if (buf != null && buf.writableBytes() >= contentLength && msg instanceof HttpContent) { + // merge into other buffer for performance reasons + buf.writeBytes(((HttpContent) msg).content()); + out.add(buf); + } else { + if (buf != null) { + out.add(buf); + } + out.add(encodeAndRetain(msg)); + } } else { if (buf != null) { out.add(buf); + } else { + // Need to produce some output otherwise an + // IllegalStateException will be thrown + out.add(EMPTY_BUFFER); } - out.add(encodeAndRetain(msg)); } - } else { + + if (msg instanceof LastHttpContent) { + state = ST_INIT; + } + return; + case ST_CONTENT_CHUNK: if (buf != null) { out.add(buf); - } else { - // Need to produce some output otherwise an - // IllegalStateException will be thrown - out.add(EMPTY_BUFFER); } - } - - if (msg instanceof LastHttpContent) { - state = ST_INIT; - } - } else if (state == ST_CONTENT_CHUNK) { - if (buf != null) { - out.add(buf); - } - encodeChunkedContent(ctx, msg, contentLength, out); - } else { - throw new Error(); + encodeChunkedContent(ctx, msg, contentLength(msg), out); + return; + default: + throw new Error(); } } else { if (buf != null) { @@ -172,6 +180,10 @@ public abstract class HttpObjectEncoder extends MessageTo } } + boolean isContentAlwaysEmpty(@SuppressWarnings("unused") H msg) { + return false; + } + @Override public boolean acceptOutboundMessage(Object msg) throws Exception { return msg instanceof HttpObject || msg instanceof ByteBuf || msg instanceof FileRegion; diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java index 98e95bfee7..7a54095356 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java @@ -15,8 +15,13 @@ */ package io.netty.handler.codec.http; +import io.netty.buffer.ByteBuf; +import io.netty.channel.ChannelHandlerContext; import io.netty.channel.CombinedChannelDuplexHandler; +import java.util.ArrayDeque; +import java.util.List; +import java.util.Queue; /** * A combination of {@link HttpRequestDecoder} and {@link HttpResponseEncoder} @@ -27,6 +32,9 @@ import io.netty.channel.CombinedChannelDuplexHandler; public final class HttpServerCodec extends CombinedChannelDuplexHandler { + /** A queue that is used for correlating a request and a response. */ + private final Queue queue = new ArrayDeque(); + /** * Creates a new instance with the default decoder options * ({@code maxInitialLineLength (4096}}, {@code maxHeaderSize (8192)}, and @@ -40,15 +48,16 @@ public final class HttpServerCodec * Creates a new instance with the specified decoder options. */ public HttpServerCodec(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize) { - super(new HttpRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize), new HttpResponseEncoder()); + init(new HttpServerRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize), + new HttpServerResponseEncoder()); } /** * Creates a new instance with the specified decoder options. */ public HttpServerCodec(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders) { - super(new HttpRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders), - new HttpResponseEncoder()); + init(new HttpServerRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders), + new HttpServerResponseEncoder()); } /** @@ -56,8 +65,46 @@ public final class HttpServerCodec */ public HttpServerCodec(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders, int initialBufferSize) { - super( - new HttpRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize), - new HttpResponseEncoder()); + init( + new HttpServerRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, + validateHeaders, initialBufferSize), + new HttpServerResponseEncoder()); + } + + private final class HttpServerRequestDecoder extends HttpRequestDecoder { + HttpServerRequestDecoder(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize) { + super(maxInitialLineLength, maxHeaderSize, maxChunkSize); + } + + HttpServerRequestDecoder(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, + boolean validateHeaders) { + super(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders); + } + + HttpServerRequestDecoder(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, + boolean validateHeaders, int initialBufferSize) { + super(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize); + } + + @Override + protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List out) throws Exception { + int oldSize = out.size(); + super.decode(ctx, buffer, out); + int size = out.size(); + for (int i = oldSize; i < size; i++) { + Object obj = out.get(i); + if (obj instanceof HttpRequest) { + queue.add(((HttpRequest) obj).getMethod()); + } + } + } + } + + private final class HttpServerResponseEncoder extends HttpResponseEncoder { + + @Override + boolean isContentAlwaysEmpty(@SuppressWarnings("unused") HttpResponse msg) { + return HttpMethod.HEAD.equals(queue.poll()); + } } } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerCodecTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerCodecTest.java index 3ecd40895d..b207b6d3d7 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerCodecTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerCodecTest.java @@ -114,6 +114,37 @@ public class HttpServerCodecTest { ch.finish(); } + @Test + public void testChunkedHeadResponse() { + EmbeddedChannel ch = new EmbeddedChannel(new HttpServerCodec()); + + // Send the request headers. + assertTrue(ch.writeInbound(Unpooled.copiedBuffer( + "HEAD / HTTP/1.1\r\n\r\n", CharsetUtil.UTF_8))); + + HttpRequest request = (HttpRequest) ch.readInbound(); + assertEquals(HttpMethod.HEAD, request.getMethod()); + LastHttpContent content = (LastHttpContent) ch.readInbound(); + assertFalse(content.content().isReadable()); + content.release(); + + HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK); + HttpHeaders.setTransferEncodingChunked(response); + assertTrue(ch.writeOutbound(response)); + assertTrue(ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT)); + assertTrue(ch.finish()); + + ByteBuf buf = (ByteBuf) ch.readOutbound(); + assertEquals("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n", buf.toString(CharsetUtil.US_ASCII)); + buf.release(); + + buf = (ByteBuf) ch.readOutbound(); + assertFalse(buf.isReadable()); + buf.release(); + + assertFalse(ch.finishAndReleaseAll()); + } + private static ByteBuf prepareDataChunk(int size) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < size; ++i) {