From 789e323b79d642ea2c0a024cb1c839654b7b8fad Mon Sep 17 00:00:00 2001
From: Trustin Lee <t@motd.kr>
Date: Wed, 22 Oct 2014 14:39:31 +0900
Subject: [PATCH] Handle an empty ByteBuf specially in HttpObjectEncoder

Related: #2983

Motivation:

It is a well known idiom to write an empty buffer and add a listener to
its future to close a channel when the last byte has been written out:

  ChannelFuture f = channel.writeAndFlush(Unpooled.EMPTY_BUFFER);
  f.addListener(ChannelFutureListener.CLOSE);

When HttpObjectEncoder is in the pipeline, this still works, but it
silently raises an IllegalStateException, because HttpObjectEncoder does
not allow writing a ByteBuf when it is expecting an HttpMessage.

Modifications:

- Handle an empty ByteBuf specially in HttpObjectEncoder, so that
  writing an empty buffer does not fail even if the pipeline contains an
  HttpObjectEncoder
- Add a test

Result:

An exception is not triggered anymore by HttpObjectEncoder, when a user
attempts to write an empty buffer.
---
 .../handler/codec/http/HttpObjectEncoder.java | 13 ++++++++++
 .../codec/http/HttpResponseEncoderTest.java   | 26 +++++++++++++++++++
 2 files changed, 39 insertions(+)

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 e08ef0cf24..d5e52d25c2 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
@@ -73,7 +73,20 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
             buf.writeBytes(CRLF);
             state = 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:
+        //
+        //     ch.write(Unpooled.EMPTY_BUFFER).addListener(ChannelFutureListener.CLOSE);
+        //
+        // See https://github.com/netty/netty/issues/2983 for more information.
+
+        if (msg instanceof ByteBuf && !((ByteBuf) msg).isReadable()) {
+            out.add(EMPTY_BUFFER);
+            return;
+        }
+
         if (msg instanceof HttpContent || msg instanceof ByteBuf || msg instanceof FileRegion) {
+
             if (state == ST_INIT) {
                 throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
             }
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java
index ff2b26065a..16df5e71bc 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseEncoderTest.java
@@ -16,6 +16,7 @@
 package io.netty.handler.codec.http;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import io.netty.channel.FileRegion;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.util.CharsetUtil;
@@ -24,6 +25,7 @@ import org.junit.Test;
 import java.io.IOException;
 import java.nio.channels.WritableByteChannel;
 
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 
 public class HttpResponseEncoderTest {
@@ -118,4 +120,28 @@ public class HttpResponseEncoderTest {
             return false;
         }
     }
+
+    @Test
+    public void testEmptyBufferBypass() throws Exception {
+        EmbeddedChannel channel = new EmbeddedChannel(new HttpResponseEncoder());
+
+        // Test writing an empty buffer works when the encoder is at ST_INIT.
+        channel.writeOutbound(Unpooled.EMPTY_BUFFER);
+        ByteBuf buffer = channel.readOutbound();
+        assertThat(buffer, is(sameInstance(Unpooled.EMPTY_BUFFER)));
+
+        // Leave the ST_INIT state.
+        HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
+        assertTrue(channel.writeOutbound(response));
+        buffer = channel.readOutbound();
+        assertEquals("HTTP/1.1 200 OK\r\n\r\n", buffer.toString(CharsetUtil.US_ASCII));
+        buffer.release();
+
+        // Test writing an empty buffer works when the encoder is not at ST_INIT.
+        channel.writeOutbound(Unpooled.EMPTY_BUFFER);
+        buffer = channel.readOutbound();
+        assertThat(buffer, is(sameInstance(Unpooled.EMPTY_BUFFER)));
+
+        assertFalse(channel.finish());
+    }
 }