Correctly convert empty HttpContent to ByteBuf

Motivation:

93130b172a introduced a regression where we not "converted" an empty HttpContent to ByteBuf and just passed it on in the pipeline. This can lead to the situation that other handlers in the pipeline will see HttpContent instances which is not expected.

Modifications:

- Correctly convert HttpContent to ByteBuf when empty
- Add unit test.

Result:

Handlers in the pipeline will see the expected message type.

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

@@ -16,17 +16,19 @@
 package io.netty.handler.codec.http;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.FileRegion;
 import io.netty.handler.codec.MessageToMessageEncoder;
 import io.netty.util.CharsetUtil;
-import io.netty.util.ReferenceCountUtil;
 import io.netty.util.internal.StringUtil;
 
 import java.util.List;
 
-import static io.netty.buffer.Unpooled.*;
-import static io.netty.handler.codec.http.HttpConstants.*;
+import static io.netty.buffer.Unpooled.directBuffer;
+import static io.netty.buffer.Unpooled.unreleasableBuffer;
+import static io.netty.handler.codec.http.HttpConstants.CR;
+import static io.netty.handler.codec.http.HttpConstants.LF;
 
 /**
  * Encodes an {@link HttpMessage} or an {@link HttpContent} into
@@ -136,6 +138,7 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
 
                         break;
                     }
+
                     // fall-through!
                 case ST_CONTENT_ALWAYS_EMPTY:
 
@@ -144,8 +147,13 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
                         out.add(buf);
                     } else {
                         // Need to produce some output otherwise an
-                        // IllegalStateException will be thrown
-                        out.add(EMPTY_BUFFER);
+                        // IllegalStateException will be thrown as we did not write anything
+                        // Its ok to just write an EMPTY_BUFFER as if there are reference count issues these will be
+                        // propagated as the caller of the encode(...) method will release the original
+                        // buffer.
+                        // Writing an empty buffer will not actually write anything on the wire, so if there is a user
+                        // error with msg it will not be visible externally
+                        out.add(Unpooled.EMPTY_BUFFER);
                     }
 
                     break;
@@ -202,7 +210,7 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
         } else if (contentLength == 0) {
             // Need to produce some output otherwise an
             // IllegalStateException will be thrown
-            out.add(ReferenceCountUtil.retain(msg));
+            out.add(encodeAndRetain(msg));
         }
     }
 

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

@@ -26,9 +26,7 @@ import java.util.concurrent.ExecutionException;
 
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 /**
  */
@@ -127,7 +125,7 @@ public class HttpRequestEncoderTest {
     }
 
     @Test
-    public void testEmptydBufferShouldPassThrough() throws Exception {
+    public void testEmptyBufferShouldPassThrough() throws Exception {
         HttpRequestEncoder encoder = new HttpRequestEncoder();
         EmbeddedChannel channel = new EmbeddedChannel(encoder);
         ByteBuf buffer = Unpooled.buffer();
@@ -135,4 +133,41 @@ public class HttpRequestEncoderTest {
         channel.finishAndReleaseAll();
         assertEquals(0, buffer.refCnt());
     }
+
+    @Test
+    public void testEmptyContentChunked() throws Exception {
+        testEmptyContent(true);
+    }
+
+    @Test
+    public void testEmptyContentNotChunked() throws Exception {
+        testEmptyContent(false);
+    }
+
+    private void testEmptyContent(boolean chunked) throws Exception {
+        HttpRequestEncoder encoder = new HttpRequestEncoder();
+        EmbeddedChannel channel = new EmbeddedChannel(encoder);
+        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
+        if (chunked) {
+            HttpHeaders.setTransferEncodingChunked(request);
+        }
+        assertTrue(channel.writeOutbound(request));
+
+        ByteBuf contentBuffer = Unpooled.buffer();
+        assertTrue(channel.writeOutbound(new DefaultHttpContent(contentBuffer)));
+
+        ByteBuf lastContentBuffer = Unpooled.buffer();
+        assertTrue(channel.writeOutbound(new DefaultHttpContent(lastContentBuffer)));
+
+        // Ensure we only produce ByteBuf instances.
+        ByteBuf head = (ByteBuf) channel.readOutbound();
+        assertTrue(head.release());
+
+        ByteBuf content = (ByteBuf) channel.readOutbound();
+        content.release();
+
+        ByteBuf lastContent = (ByteBuf) channel.readOutbound();
+        lastContent.release();
+        assertFalse(channel.finish());
+    }
 }