[#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.

codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java

@@ -57,6 +57,7 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> 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;
@@ -77,7 +78,8 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
             encodeInitialLine(buf, m);
             encodeHeaders(m.headers(), buf);
             buf.writeBytes(CRLF);
-            state = HttpUtil.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK;
+            state = isContentAlwaysEmpty(m) ? ST_CONTENT_ALWAYS_EMPTY :
+                    HttpUtil.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK;
         }
 
         // Bypass the encoder in case of an empty buffer, so that the following idiom works:
@@ -92,44 +94,50 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
         }
 
         if (msg instanceof HttpContent || msg instanceof ByteBuf || msg instanceof FileRegion) {
-
+            switch (state) {
-            if (state == ST_INIT) {
+                case ST_INIT:
-                throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
+                    throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
-            }
+                case ST_CONTENT_ALWAYS_EMPTY:
-
+                    out.add(EMPTY_BUFFER);
-            final long contentLength = contentLength(msg);
+                    if (msg instanceof LastHttpContent) {
-            if (state == ST_CONTENT_NON_CHUNK) {
+                        state = ST_INIT;
-                if (contentLength > 0) {
+                    }
-                    if (buf != null && buf.writableBytes() >= contentLength && msg instanceof HttpContent) {
+                    return;
-                        // merge into other buffer for performance reasons
+                case ST_CONTENT_NON_CHUNK:
-                        buf.writeBytes(((HttpContent) msg).content());
+                    final long contentLength = contentLength(msg);
-                        out.add(buf);
+                    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);
                     }
-                }
+                    encodeChunkedContent(ctx, msg, contentLength(msg), out);
-
+                    return;
-                if (msg instanceof LastHttpContent) {
+                default:
-                    state = ST_INIT;
+                    throw new Error();
-                }
-            } else if (state == ST_CONTENT_CHUNK) {
-                if (buf != null) {
-                    out.add(buf);
-                }
-                encodeChunkedContent(ctx, msg, contentLength, out);
-            } else {
-                throw new Error();
             }
         } else {
             if (buf != null) {
@@ -187,6 +195,10 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> 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;

codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java

@@ -15,9 +15,14 @@
  */
 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}
  * which enables easier server side HTTP implementation.
@@ -27,6 +32,9 @@ import io.netty.channel.CombinedChannelDuplexHandler;
 public final class HttpServerCodec extends CombinedChannelDuplexHandler<HttpRequestDecoder, HttpResponseEncoder>
         implements HttpServerUpgradeHandler.SourceCodec {
 
+    /** A queue that is used for correlating a request and a response. */
+    private final Queue<HttpMethod> queue = new ArrayDeque<HttpMethod>();
+
     /**
      * Creates a new instance with the default decoder options
      * ({@code maxInitialLineLength (4096}}, {@code maxHeaderSize (8192)}, and
@@ -40,15 +48,16 @@ public final class HttpServerCodec extends CombinedChannelDuplexHandler<HttpRequ
      * 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),
+        init(new HttpServerRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders),
-                new HttpResponseEncoder());
+                new HttpServerResponseEncoder());
     }
 
     /**
@@ -56,9 +65,10 @@ public final class HttpServerCodec extends CombinedChannelDuplexHandler<HttpRequ
      */
     public HttpServerCodec(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders,
                            int initialBufferSize) {
-        super(
+        init(
-          new HttpRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize),
+          new HttpServerRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize,
-          new HttpResponseEncoder());
+                  validateHeaders, initialBufferSize),
+          new HttpServerResponseEncoder());
     }
 
     /**
@@ -69,4 +79,41 @@ public final class HttpServerCodec extends CombinedChannelDuplexHandler<HttpRequ
     public void upgradeFrom(ChannelHandlerContext ctx) {
         ctx.pipeline().remove(this);
     }
+
+    private final class HttpServerRequestDecoder extends HttpRequestDecoder {
+        public HttpServerRequestDecoder(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize) {
+            super(maxInitialLineLength, maxHeaderSize, maxChunkSize);
+        }
+
+        public HttpServerRequestDecoder(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize,
+                                        boolean validateHeaders) {
+            super(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders);
+        }
+
+        public 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<Object> 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).method());
+                }
+            }
+        }
+    }
+
+    private final class HttpServerResponseEncoder extends HttpResponseEncoder {
+
+        @Override
+        boolean isContentAlwaysEmpty(@SuppressWarnings("unused") HttpResponse msg) {
+            return HttpMethod.HEAD.equals(queue.poll());
+        }
+    }
 }

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 = ch.readInbound();
+        assertEquals(HttpMethod.HEAD, request.method());
+        LastHttpContent content = ch.readInbound();
+        assertFalse(content.content().isReadable());
+        content.release();
+
+        HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
+        HttpUtil.setTransferEncodingChunked(response, true);
+        assertTrue(ch.writeOutbound(response));
+        assertTrue(ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT));
+        assertTrue(ch.finish());
+
+        ByteBuf buf = ch.readOutbound();
+        assertEquals("HTTP/1.1 200 OK\r\ntransfer-encoding: chunked\r\n\r\n", buf.toString(CharsetUtil.US_ASCII));
+        buf.release();
+
+        buf = 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) {