Fix a bug where HttpContentEncoder generates an empty chunk even if it's not the last chunk

- Fixes #1312
- Added more test cases to ensure the fix

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

@@ -111,19 +111,28 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
                 if (isFull) {
                     // Pass through the full response with empty content and continue waiting for the the next resp.
                     if (!((ByteBufHolder) res).data().isReadable()) {
+                        // Set the content length to 0.
+                        res.headers().remove(Names.TRANSFER_ENCODING);
+                        res.headers().set(Names.CONTENT_LENGTH, "0");
                         out.add(BufUtil.retain(res));
                         break;
                     }
                 }
 
                 // Prepare to encode the content.
-                Result result = beginEncode(res, acceptEncoding);
+                final Result result = beginEncode(res, acceptEncoding);
 
                 // If unable to encode, pass through.
                 if (result == null) {
                     if (isFull) {
+                        // As an unchunked response
+                        res.headers().remove(Names.TRANSFER_ENCODING);
+                        res.headers().set(Names.CONTENT_LENGTH, ((ByteBufHolder) res).data().readableBytes());
                         out.add(BufUtil.retain(res));
                     } else {
+                        // As a chunked response
+                        res.headers().remove(Names.CONTENT_LENGTH);
+                        res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
                         out.add(res);
                         state = State.PASS_THROUGH;
                     }
@@ -193,6 +202,7 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
     private HttpContent[] encodeContent(HttpContent c) {
         ByteBuf newContent = Unpooled.buffer();
         ByteBuf content = c.data();
+
         encode(content, newContent);
 
         if (c instanceof LastHttpContent) {
@@ -202,7 +212,12 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
             // Generate an additional chunk if the decoder produced
             // the last product on closure,
             if (lastProduct.isReadable()) {
-                return new HttpContent[] { new DefaultHttpContent(newContent), new DefaultLastHttpContent(lastProduct)};
+                if (newContent.isReadable()) {
+                    return new HttpContent[] {
+                            new DefaultHttpContent(newContent), new DefaultLastHttpContent(lastProduct)};
+                } else {
+                    return new HttpContent[] { new DefaultLastHttpContent(lastProduct) };
+                }
             } else {
                 return new HttpContent[] { new DefaultLastHttpContent(newContent) };
             }

codec-http/src/test/java/io/netty/handler/codec/http/HttpContentCompressorTest.java

@@ -15,11 +15,14 @@
  */
 package io.netty.handler.codec.http;
 
+import io.netty.channel.embedded.EmbeddedMessageChannel;
 import io.netty.handler.codec.compression.ZlibWrapper;
-
+import io.netty.handler.codec.http.HttpHeaders.Names;
-import org.junit.Assert;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
 public class HttpContentCompressorTest {
     @Test
     public void testGetTargetContentEncoding() throws Exception {
@@ -51,10 +54,35 @@ public class HttpContentCompressorTest {
                     targetEncoding = "deflate";
                     break;
                 default:
-                    Assert.fail();
+                    fail();
                 }
             }
-            Assert.assertEquals(contentEncoding, targetEncoding);
+            assertEquals(contentEncoding, targetEncoding);
         }
     }
+
+    @Test
+    public void testEmptyContentCompression() throws Exception {
+        EmbeddedMessageChannel ch = new EmbeddedMessageChannel(new HttpContentCompressor());
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/");
+        req.headers().set(Names.ACCEPT_ENCODING, "deflate");
+        ch.writeInbound(req);
+
+        ch.writeOutbound(new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK));
+
+        HttpResponse res = (HttpResponse) ch.readOutbound();
+        assertThat(res, is(not(instanceOf(FullHttpResponse.class))));
+        assertThat(res.headers().get(Names.TRANSFER_ENCODING), is("chunked"));
+        assertThat(res.headers().get(Names.CONTENT_LENGTH), is(nullValue()));
+        assertThat(res.headers().get(Names.CONTENT_ENCODING), is("deflate"));
+
+        ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
+
+        HttpContent chunk;
+        chunk = (HttpContent) ch.readOutbound();
+        assertThat(chunk, is(instanceOf(LastHttpContent.class)));
+        assertThat(chunk.data().isReadable(), is(true));
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+    }
 }

codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java

@@ -115,6 +115,53 @@ public class HttpContentEncoderTest {
         assertThat(ch.readOutbound(), is(nullValue()));
     }
 
+    /**
+     * If the length of the content is unknown, {@link HttpContentEncoder} should not skip encoding even if the
+     * actual length is turned out to be 0.
+     */
+    @Test
+    public void testEmptySplitContent() throws Exception {
+        EmbeddedMessageChannel ch = new EmbeddedMessageChannel(new TestEncoder());
+        ch.writeInbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"));
+
+        ch.writeOutbound(new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK));
+        assertEncodedResponse(ch);
+
+        ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
+        HttpContent chunk = (HttpContent) ch.readOutbound();
+        assertThat(chunk.data().isReadable(), is(false));
+        assertThat(chunk, is(instanceOf(LastHttpContent.class)));
+        assertThat(ch.readOutbound(), is(nullValue()));
+    }
+
+    /**
+     * If the length of the content is 0 for sure, {@link HttpContentEncoder} should skip encoding.
+     */
+    @Test
+    public void testEmptyFullContent() throws Exception {
+        EmbeddedMessageChannel ch = new EmbeddedMessageChannel(new TestEncoder());
+        ch.writeInbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"));
+
+        FullHttpResponse res = new DefaultFullHttpResponse(
+                HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.EMPTY_BUFFER);
+        ch.writeOutbound(res);
+
+        Object o = ch.readOutbound();
+        assertThat(o, is(instanceOf(FullHttpResponse.class)));
+
+        res = (FullHttpResponse) o;
+        assertThat(res.headers().get(Names.TRANSFER_ENCODING), is(nullValue()));
+
+        // Length must be set to 0.
+        assertThat(res.headers().get(Names.CONTENT_LENGTH), is("0"));
+        // Content encoding shouldn't be modified.
+        assertThat(res.headers().get(Names.CONTENT_ENCODING), is(nullValue()));
+        assertThat(res.data().readableBytes(), is(0));
+        assertThat(res.data().toString(CharsetUtil.US_ASCII), is(""));
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+    }
+
     private static void assertEncodedResponse(EmbeddedMessageChannel ch) {
         Object o = ch.readOutbound();
         assertThat(o, is(instanceOf(HttpResponse.class)));