Repository: netty/netty
Issue: HttpContentCompressor/HttpContentEncoder should reset the content-length header for FullHttpMessages
Labels: improvement

### Expected behavior
When the `HttpContentEncoder` is tasked with compressing a full `HttpMessage` that already contains a `Content-Length` header it should perform the compression and _adjust_ the `Content-Length`.

### Actual behavior
All messages compressed result in `Transfer-Encoding: Chunked` and don't have a `Content-Length` header.

### Minimal yet complete reproducer code (or URL to code)
```java
@Test
    public void testAmendContentLength() throws Exception {
        EmbeddedChannel ch = new EmbeddedChannel(new HttpContentCompressor());
        ch.writeInbound(newRequest());

        ByteBuf content = Unpooled.copiedBuffer("Hello, world!", CharsetUtil.UTF_8);
        HttpMessage res = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK, content);
        HttpUtil.setContentLength(res, content.readableBytes());

        ch.writeOneOutbound(res);

        HttpMessage compressedResponse = ch.readOutbound();
        assertThat(res.headers().get(HttpHeaderNames.CONTENT_ENCODING), is("gzip"));
        assertTrue(HttpUtil.isContentLengthSet(compressedResponse));
    }
```

### Netty version 4.1 branch at SHA 24f801c7d1d1 (should affect most versions)

Issue comments:
This appears to be a very deliberate behavior, but I don't see a reason other than for implementation convenience. I'm happy to take a look at this if you're willing to accept a patch.
IIUC it was done out of convenience (not required ... chunked should be "semantically correct" in either case so no need for a specialization). However a PR to adjust `content-length` if full object sgtm.
@Scottmitch thanks for the quick reply. I've opened a PR, #6809, that preserves the `content-length` header for full responses.
fixed by https://github.com/netty/netty/pull/6809

Pull request review comments:
in runtime an `assert` block is ignoring if `-ea` option has not specified.
We don't want this to be an `assert`, right? `assert` statements can be disabled via command line and optimized away by JIT.
consider just directly casting to the object you expect like the other tests ... a class cast exception will be thrown if the expected type is not read.
Removed.
Done.

Patch that resolved the issue:
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 578deb25999..c04d5386819 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
@@ -164,18 +164,21 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
                 // so that the message looks like a decoded message.
                 res.headers().set(HttpHeaderNames.CONTENT_ENCODING, result.targetContentEncoding());
 
-                // Make the response chunked to simplify content transformation.
-                res.headers().remove(HttpHeaderNames.CONTENT_LENGTH);
-                res.headers().set(HttpHeaderNames.TRANSFER_ENCODING, HttpHeaderValues.CHUNKED);
-
                 // Output the rewritten response.
                 if (isFull) {
                     // Convert full message into unfull one.
                     HttpResponse newRes = new DefaultHttpResponse(res.protocolVersion(), res.status());
                     newRes.headers().set(res.headers());
                     out.add(newRes);
-                    // Fall through to encode the content of the full response.
+
+                    ensureContent(res);
+                    encodeFullResponse(newRes, (HttpContent) res, out);
+                    break;
                 } else {
+                    // Make the response chunked to simplify content transformation.
+                    res.headers().remove(HttpHeaderNames.CONTENT_LENGTH);
+                    res.headers().set(HttpHeaderNames.TRANSFER_ENCODING, HttpHeaderValues.CHUNKED);
+
                     out.add(res);
                     state = State.AWAIT_CONTENT;
                     if (!(msg instanceof HttpContent)) {
@@ -205,6 +208,25 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
         }
     }
 
+    private void encodeFullResponse(HttpResponse newRes, HttpContent content, List<Object> out) {
+        int existingMessages = out.size();
+        encodeContent(content, out);
+
+        if (HttpUtil.isContentLengthSet(newRes)) {
+            // adjust the content-length header
+            int messageSize = 0;
+            for (int i = existingMessages; i < out.size(); i++) {
+                Object item = out.get(i);
+                if (item instanceof HttpContent) {
+                    messageSize += ((HttpContent) item).content().readableBytes();
+                }
+            }
+            HttpUtil.setContentLength(newRes, messageSize);
+        } else {
+            newRes.headers().set(HttpHeaderNames.TRANSFER_ENCODING, HttpHeaderValues.CHUNKED);
+        }
+    }
+
     private static boolean isPassthru(HttpVersion version, int code, CharSequence httpMethod) {
         return code < 200 || code == 204 || code == 304 ||
                (httpMethod == ZERO_LENGTH_HEAD || (httpMethod == ZERO_LENGTH_CONNECT && code == 200)) ||


Test patch:
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 94b1aa6cd68..c6ff3bd6bc0 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
@@ -198,14 +198,51 @@ public void testChunkedContentWithTrailingHeader() throws Exception {
     }
 
     @Test
-    public void testFullContent() throws Exception {
+    public void testFullContentWithContentLength() throws Exception {
         EmbeddedChannel ch = new EmbeddedChannel(new HttpContentCompressor());
         ch.writeInbound(newRequest());
 
-        FullHttpResponse res = new DefaultFullHttpResponse(
+        FullHttpResponse fullRes = new DefaultFullHttpResponse(
                 HttpVersion.HTTP_1_1, HttpResponseStatus.OK,
                 Unpooled.copiedBuffer("Hello, World", CharsetUtil.US_ASCII));
-        res.headers().set(HttpHeaderNames.CONTENT_LENGTH, res.content().readableBytes());
+        fullRes.headers().set(HttpHeaderNames.CONTENT_LENGTH, fullRes.content().readableBytes());
+        ch.writeOutbound(fullRes);
+
+        HttpResponse res = ch.readOutbound();
+        assertThat(res, is(not(instanceOf(HttpContent.class))));
+
+        assertThat(res.headers().get(HttpHeaderNames.TRANSFER_ENCODING), is(nullValue()));
+        assertThat(res.headers().get(HttpHeaderNames.CONTENT_ENCODING), is("gzip"));
+
+        long contentLengthHeaderValue = HttpUtil.getContentLength(res);
+        long observedLength = 0;
+
+        HttpContent c = ch.readOutbound();
+        observedLength += c.content().readableBytes();
+        assertThat(ByteBufUtil.hexDump(c.content()), is("1f8b0800000000000000f248cdc9c9d75108cf2fca4901000000ffff"));
+        c.release();
+
+        c = ch.readOutbound();
+        observedLength += c.content().readableBytes();
+        assertThat(ByteBufUtil.hexDump(c.content()), is("0300c6865b260c000000"));
+        c.release();
+
+        LastHttpContent last = ch.readOutbound();
+        assertThat(last.content().readableBytes(), is(0));
+        last.release();
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertEquals(contentLengthHeaderValue, observedLength);
+    }
+
+    @Test
+    public void testFullContent() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new HttpContentCompressor());
+        ch.writeInbound(newRequest());
+
+        FullHttpResponse res = new DefaultFullHttpResponse(
+            HttpVersion.HTTP_1_1, HttpResponseStatus.OK,
+            Unpooled.copiedBuffer("Hello, World", CharsetUtil.US_ASCII));
         ch.writeOutbound(res);
 
         assertEncodedResponse(ch);
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 660df3406e5..d4bb1af78ba 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
@@ -160,14 +160,41 @@ public void testChunkedContentWithTrailingHeader() throws Exception {
         assertThat(ch.readOutbound(), is(nullValue()));
     }
 
+    @Test
+    public void testFullContentWithContentLength() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
+        ch.writeInbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"));
+
+        FullHttpResponse fullRes = new DefaultFullHttpResponse(
+                HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.wrappedBuffer(new byte[42]));
+        fullRes.headers().set(HttpHeaderNames.CONTENT_LENGTH, 42);
+        ch.writeOutbound(fullRes);
+
+        HttpResponse res = ch.readOutbound();
+        assertThat(res, is(not(instanceOf(HttpContent.class))));
+        assertThat(res.headers().get(HttpHeaderNames.TRANSFER_ENCODING), is(nullValue()));
+        assertThat(res.headers().get(HttpHeaderNames.CONTENT_LENGTH), is("2"));
+        assertThat(res.headers().get(HttpHeaderNames.CONTENT_ENCODING), is("test"));
+
+        HttpContent c = ch.readOutbound();
+        assertThat(c.content().readableBytes(), is(2));
+        assertThat(c.content().toString(CharsetUtil.US_ASCII), is("42"));
+        c.release();
+
+        LastHttpContent last = ch.readOutbound();
+        assertThat(last.content().readableBytes(), is(0));
+        last.release();
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+    }
+
     @Test
     public void testFullContent() throws Exception {
         EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
         ch.writeInbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"));
 
         FullHttpResponse res = new DefaultFullHttpResponse(
-                HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.wrappedBuffer(new byte[42]));
-        res.headers().set(HttpHeaderNames.CONTENT_LENGTH, 42);
+            HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.wrappedBuffer(new byte[42]));
         ch.writeOutbound(res);
 
         assertEncodedResponse(ch);