From 4a1e0ceb4dc7a3eb49222b048665ab734bd4ed6f Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Fri, 10 Jun 2016 11:45:14 +0200
Subject: [PATCH] [5382] HttpContentEncoder should not set chunked
 transfer-encoding for HTTP/1.0

Motivation:

When using HttpContentCompressor and the HttpResponse is protocol version 1.0, HttpContentEncoder.encode() should not set the transfer-encoding header to chunked. Chunked transfer-encoding is not valid for HTTP 1.0 - this causes ERR_CONTENT_DECODING_FAILED errors in chrome and similar failures in IE.

Modifications:

Skip HTTP/1.0 messages

Result:

Be able to serve HTTP/1.0 as well when HttpContentEncoder is in the pipeline.
---
 .../codec/http/HttpContentEncoder.java        | 11 +++++---
 .../codec/http/HttpContentEncoderTest.java    | 28 +++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

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 a20a3fd789..ec1856673c 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
@@ -120,9 +120,11 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
                  * The HEAD method is identical to GET except that the server MUST NOT return a message-body
                  * in the response.
                  *
-                 * This code is now inline with HttpClientDecoder.Decoder
+                 * Also we should pass through HTTP/1.0 as transfer-encoding: chunked is not supported.
+                 *
+                 * See https://github.com/netty/netty/issues/5382
                  */
-                if (isPassthru(code, acceptEncoding)) {
+                if (isPassthru(res.protocolVersion(), code, acceptEncoding)) {
                     if (isFull) {
                         out.add(ReferenceCountUtil.retain(res));
                     } else {
@@ -203,9 +205,10 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
         }
     }
 
-    private static boolean isPassthru(int code, CharSequence httpMethod) {
+    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));
+               (httpMethod == ZERO_LENGTH_HEAD || (httpMethod == ZERO_LENGTH_CONNECT && code == 200)) ||
+                version == HttpVersion.HTTP_1_0;
     }
 
     private static void ensureHeaders(HttpObject msg) {
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 dd77426756..660df3406e 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
@@ -30,7 +30,10 @@ import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 public class HttpContentEncoderTest {
 
@@ -332,6 +335,31 @@ public class HttpContentEncoderTest {
         assertThat(ch.readOutbound(), is(nullValue()));
     }
 
+    @Test
+    public void testHttp1_0() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_0, HttpMethod.GET, "/");
+        assertTrue(ch.writeInbound(req));
+
+        HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_0, HttpResponseStatus.OK);
+        res.headers().set(HttpHeaderNames.CONTENT_LENGTH, HttpHeaderValues.ZERO);
+        assertTrue(ch.writeOutbound(res));
+        assertTrue(ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT));
+        assertTrue(ch.finish());
+
+        FullHttpRequest request = ch.readInbound();
+        assertTrue(request.release());
+        assertNull(ch.readInbound());
+
+        HttpResponse response = ch.readOutbound();
+        assertSame(res, response);
+
+        LastHttpContent content = ch.readOutbound();
+        assertSame(LastHttpContent.EMPTY_LAST_CONTENT, content);
+        content.release();
+        assertNull(ch.readOutbound());
+    }
+
     private static void assertEmptyResponse(EmbeddedChannel ch) {
         Object o = ch.readOutbound();
         assertThat(o, is(instanceOf(HttpResponse.class)));