Fix a bug where HttpObjectAggregator doesn't always produce FullHttpMessage

- Fixes #2182
- Always convert an unfull invalid message to a full message

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

@@ -25,7 +25,6 @@ import io.netty.channel.ChannelPipeline;
 import io.netty.handler.codec.DecoderResult;
 import io.netty.handler.codec.MessageToMessageDecoder;
 import io.netty.handler.codec.TooLongFrameException;
-import io.netty.util.ReferenceCountUtil;
 
 import java.util.List;
 
@@ -136,8 +135,8 @@ public class HttpObjectAggregator extends MessageToMessageDecoder<HttpObject> {
 
             if (!m.getDecoderResult().isSuccess()) {
                 removeTransferEncodingChunked(m);
+                out.add(toFullMessage(m));
                 this.currentMessage = null;
-                out.add(ReferenceCountUtil.retain(m));
                 return;
             }
             if (msg instanceof HttpRequest) {
@@ -247,4 +246,25 @@ public class HttpObjectAggregator extends MessageToMessageDecoder<HttpObject> {
             currentMessage = null;
         }
     }
+
+    private static FullHttpMessage toFullMessage(HttpMessage msg) {
+        if (msg instanceof FullHttpMessage) {
+            return ((FullHttpMessage) msg).retain();
+        }
+
+        FullHttpMessage fullMsg;
+        if (msg instanceof HttpRequest) {
+            HttpRequest req = (HttpRequest) msg;
+            fullMsg = new DefaultFullHttpRequest(
+                    req.getProtocolVersion(), req.getMethod(), req.getUri(), Unpooled.EMPTY_BUFFER, false);
+        } else if (msg instanceof HttpResponse) {
+            HttpResponse res = (HttpResponse) msg;
+            fullMsg = new DefaultFullHttpResponse(
+                    res.getProtocolVersion(), res.getStatus(), Unpooled.EMPTY_BUFFER, false);
+        } else {
+            throw new IllegalStateException();
+        }
+
+        return fullMsg;
+    }
 }

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

@@ -27,7 +27,8 @@ import org.junit.Test;
 
 import java.util.List;
 
-import static io.netty.util.ReferenceCountUtil.releaseLater;
+import static io.netty.util.ReferenceCountUtil.*;
+import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 
 public class HttpObjectAggregatorTest {
@@ -186,4 +187,22 @@ public class HttpObjectAggregatorTest {
         checkContentBuffer(aggratedMessage);
         assertNull(embedder.readInbound());
     }
+
+    @Test
+    public void testBadRequest() {
+        EmbeddedChannel ch = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(1024 * 1024));
+        ch.writeInbound(Unpooled.copiedBuffer("GET / HTTP/1.0 with extra\r\n", CharsetUtil.UTF_8));
+        assertThat(ch.readInbound(), is(instanceOf(FullHttpRequest.class)));
+        assertNull(ch.readInbound());
+        ch.finish();
+    }
+
+    @Test
+    public void testBadResponse() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new HttpResponseDecoder(), new HttpObjectAggregator(1024 * 1024));
+        ch.writeInbound(Unpooled.copiedBuffer("HTTP/1.0 BAD_CODE Bad Server\r\n", CharsetUtil.UTF_8));
+        assertThat(ch.readInbound(), is(instanceOf(FullHttpResponse.class)));
+        assertNull(ch.readInbound());
+        ch.finish();
+    }
 }