From 8bcea35d2ea949f4fc2b09fea06c20e5a0adb07d Mon Sep 17 00:00:00 2001
From: Bartek Kowalczyk <bkowalczyyk@gmail.com>
Date: Mon, 21 Jan 2019 07:45:03 +0100
Subject: [PATCH] Set result for decoded request and add test for #8721 (#8721)

Motivation:
I want to fix bug in vert.x project (eclipse-vertx/vert.x#2562) caused by ComposedLastHttpContent result being null. I don't know if it is intentional that this last decoded chuck in the issue returns null, but if not - I am providing fix for that.

Modification:
* Added new constructor in ComposedLastHttpContent allowing to pass DecoderResult
* set DecoderResult.SUCCESS for created ComposedLastHttpContent in HttpContentEncoder
* set DecoderResult.SUCCESS for created ComposedLastHttpContent in HttpContentDecoder

Result:
Fixes eclipse-vertx/vert.x#2562
---
 .../codec/http/ComposedLastHttpContent.java   |  5 +++
 .../codec/http/HttpContentDecoder.java        |  3 +-
 .../codec/http/HttpContentEncoder.java        |  3 +-
 .../codec/http/HttpContentCompressorTest.java |  4 ++
 .../codec/http/HttpContentDecoderTest.java    | 42 +++++++++++++++++++
 .../codec/http/HttpContentEncoderTest.java    |  3 ++
 6 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/ComposedLastHttpContent.java b/codec-http/src/main/java/io/netty/handler/codec/http/ComposedLastHttpContent.java
index b8b7dc967f..1ef39e56ff 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/ComposedLastHttpContent.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/ComposedLastHttpContent.java
@@ -28,6 +28,11 @@ final class ComposedLastHttpContent implements LastHttpContent {
         this.trailingHeaders = trailingHeaders;
     }
 
+    ComposedLastHttpContent(HttpHeaders trailingHeaders, DecoderResult result) {
+        this(trailingHeaders);
+        this.result = result;
+    }
+
     @Override
     public HttpHeaders trailingHeaders() {
         return trailingHeaders;
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java
index e85adaaa37..3eb7ad3e33 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java
@@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.handler.codec.CodecException;
+import io.netty.handler.codec.DecoderResult;
 import io.netty.handler.codec.MessageToMessageDecoder;
 import io.netty.util.ReferenceCountUtil;
 
@@ -164,7 +165,7 @@ public abstract class HttpContentDecoder extends MessageToMessageDecoder<HttpObj
             if (headers.isEmpty()) {
                 out.add(LastHttpContent.EMPTY_LAST_CONTENT);
             } else {
-                out.add(new ComposedLastHttpContent(headers));
+                out.add(new ComposedLastHttpContent(headers, DecoderResult.SUCCESS));
             }
         }
     }
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java
index 0078edc421..c486a67ed8 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java
@@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufHolder;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.embedded.EmbeddedChannel;
+import io.netty.handler.codec.DecoderResult;
 import io.netty.handler.codec.MessageToMessageCodec;
 import io.netty.util.ReferenceCountUtil;
 
@@ -264,7 +265,7 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
             if (headers.isEmpty()) {
                 out.add(LastHttpContent.EMPTY_LAST_CONTENT);
             } else {
-                out.add(new ComposedLastHttpContent(headers));
+                out.add(new ComposedLastHttpContent(headers, DecoderResult.SUCCESS));
             }
             return true;
         }
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentCompressorTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentCompressorTest.java
index 6ab2ab88c6..508eb5ae54 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentCompressorTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentCompressorTest.java
@@ -18,6 +18,7 @@ package io.netty.handler.codec.http;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.embedded.EmbeddedChannel;
+import io.netty.handler.codec.DecoderResult;
 import io.netty.handler.codec.EncoderException;
 import io.netty.handler.codec.compression.ZlibWrapper;
 import io.netty.util.CharsetUtil;
@@ -188,6 +189,7 @@ public class HttpContentCompressorTest {
         assertThat(chunk.content().isReadable(), is(false));
         assertThat(chunk, is(instanceOf(LastHttpContent.class)));
         assertEquals("Netty", ((LastHttpContent) chunk).trailingHeaders().get(of("X-Test")));
+        assertEquals(DecoderResult.SUCCESS, chunk.decoderResult());
         chunk.release();
 
         assertThat(ch.readOutbound(), is(nullValue()));
@@ -331,6 +333,7 @@ public class HttpContentCompressorTest {
         assertThat(res.content().readableBytes(), is(0));
         assertThat(res.content().toString(CharsetUtil.US_ASCII), is(""));
         assertEquals("Netty", res.trailingHeaders().get(of("X-Test")));
+        assertEquals(DecoderResult.SUCCESS, res.decoderResult());
         assertThat(ch.readOutbound(), is(nullValue()));
     }
 
@@ -370,6 +373,7 @@ public class HttpContentCompressorTest {
         assertThat(res.content().readableBytes(), is(0));
         assertThat(res.content().toString(CharsetUtil.US_ASCII), is(""));
         assertEquals("Netty", res.trailingHeaders().get(of("X-Test")));
+        assertEquals(DecoderResult.SUCCESS, res.decoderResult());
         assertThat(ch.readOutbound(), is(nullValue()));
     }
 
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java
index b7ea6df628..8646558c9f 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentDecoderTest.java
@@ -89,6 +89,48 @@ public class HttpContentDecoderTest {
         assertFalse(channel.finish()); // assert that no messages are left in channel
     }
 
+    @Test
+    public void testChunkedRequestDecompression() {
+        HttpResponseDecoder decoder = new HttpResponseDecoder();
+        HttpContentDecoder decompressor = new HttpContentDecompressor();
+
+        EmbeddedChannel channel = new EmbeddedChannel(decoder, decompressor, null);
+
+        String headers = "HTTP/1.1 200 OK\r\n"
+                + "Transfer-Encoding: chunked\r\n"
+                + "Trailer: My-Trailer\r\n"
+                + "Content-Encoding: gzip\r\n\r\n";
+
+        channel.writeInbound(Unpooled.copiedBuffer(headers.getBytes(CharsetUtil.US_ASCII)));
+
+        String chunkLength = Integer.toHexString(GZ_HELLO_WORLD.length);
+        assertTrue(channel.writeInbound(Unpooled.copiedBuffer(chunkLength + "\r\n", CharsetUtil.US_ASCII)));
+        assertTrue(channel.writeInbound(Unpooled.copiedBuffer(GZ_HELLO_WORLD)));
+        assertTrue(channel.writeInbound(Unpooled.copiedBuffer("\r\n".getBytes(CharsetUtil.US_ASCII))));
+        assertTrue(channel.writeInbound(Unpooled.copiedBuffer("0\r\n", CharsetUtil.US_ASCII)));
+        assertTrue(channel.writeInbound(Unpooled.copiedBuffer("My-Trailer: 42\r\n\r\n\r\n", CharsetUtil.US_ASCII)));
+
+        Object ob1 = channel.readInbound();
+        assertThat(ob1, is(instanceOf(DefaultHttpResponse.class)));
+
+        Object ob2 = channel.readInbound();
+        assertThat(ob1, is(instanceOf(DefaultHttpResponse.class)));
+        HttpContent content = (HttpContent) ob2;
+        assertEquals(HELLO_WORLD, content.content().toString(CharsetUtil.US_ASCII));
+        content.release();
+
+        Object ob3 = channel.readInbound();
+        assertThat(ob1, is(instanceOf(DefaultHttpResponse.class)));
+        LastHttpContent lastContent = (LastHttpContent) ob3;
+        assertNotNull(lastContent.decoderResult());
+        assertTrue(lastContent.decoderResult().isSuccess());
+        assertFalse(lastContent.trailingHeaders().isEmpty());
+        assertEquals("42", lastContent.trailingHeaders().get("My-Trailer"));
+        assertHasInboundMessages(channel, false);
+        assertHasOutboundMessages(channel, false);
+        assertFalse(channel.finish());
+    }
+
     @Test
     public void testResponseDecompression() {
         // baseline test: response decoder, content decompressor && request aggregator work as expected
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java
index 9242cf8728..6301ee8c0b 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java
@@ -22,6 +22,7 @@ import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.handler.codec.CodecException;
+import io.netty.handler.codec.DecoderResult;
 import io.netty.handler.codec.EncoderException;
 import io.netty.handler.codec.MessageToByteEncoder;
 import io.netty.util.CharsetUtil;
@@ -156,6 +157,7 @@ public class HttpContentEncoderTest {
         assertThat(chunk.content().isReadable(), is(false));
         assertThat(chunk, is(instanceOf(LastHttpContent.class)));
         assertEquals("Netty", ((LastHttpContent) chunk).trailingHeaders().get(of("X-Test")));
+        assertEquals(DecoderResult.SUCCESS, res.decoderResult());
         chunk.release();
 
         assertThat(ch.readOutbound(), is(nullValue()));
@@ -285,6 +287,7 @@ public class HttpContentEncoderTest {
         assertThat(res.content().readableBytes(), is(0));
         assertThat(res.content().toString(CharsetUtil.US_ASCII), is(""));
         assertEquals("Netty", res.trailingHeaders().get(of("X-Test")));
+        assertEquals(DecoderResult.SUCCESS, res.decoderResult());
         assertThat(ch.readOutbound(), is(nullValue()));
     }