HttpConversionUtil does not account for COOKIE compression

Motivation:
The HTTP/2 RFC allows for COOKIE values to be split into individual header elements to get more benefit from compression (https://tools.ietf.org/html/rfc7540#section-8.1.2.5). HttpConversionUtil was not accounting for this behavior.

Modifications:
- Modify HttpConversionUtil to support compressing and decompressing the COOKIE values

Result:
HttpConversionUtil is compatible with https://tools.ietf.org/html/rfc7540#section-8.1.2.5)
Fixes https://github.com/netty/netty/issues/4457

codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java

@@ -14,17 +14,16 @@
  */
 package io.netty.handler.codec.http2;
 
+import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
+import static io.netty.handler.codec.http2.Http2Exception.connectionError;
+import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER;
+import static io.netty.util.AsciiString.isUpperCase;
 import io.netty.handler.codec.CharSequenceValueConverter;
 import io.netty.handler.codec.DefaultHeaders;
 import io.netty.util.AsciiString;
 import io.netty.util.ByteProcessor;
 import io.netty.util.internal.PlatformDependent;
 
-import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
-import static io.netty.handler.codec.http2.Http2Exception.connectionError;
-import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER;
-import static io.netty.util.AsciiString.isUpperCase;
-
 public class DefaultHttp2Headers
         extends DefaultHeaders<CharSequence, CharSequence, Http2Headers> implements Http2Headers {
     private static final ByteProcessor HTTP2_NAME_VALIDATOR_PROCESSOR = new ByteProcessor() {
@@ -113,6 +112,20 @@ public class DefaultHttp2Headers
         return super.clear();
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (!(o instanceof Http2Headers)) {
+            return false;
+        }
+
+        return equals((Http2Headers) o, CASE_SENSITIVE_HASHER);
+    }
+
+    @Override
+    public int hashCode() {
+        return hashCode(CASE_SENSITIVE_HASHER);
+    }
+
     @Override
     public Http2Headers method(CharSequence value) {
         set(PseudoHeaderName.METHOD.value(), value);

codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java

@@ -14,6 +14,18 @@
  */
 package io.netty.handler.codec.http2;
 
+import static io.netty.handler.codec.http.HttpScheme.HTTP;
+import static io.netty.handler.codec.http.HttpScheme.HTTPS;
+import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm;
+import static io.netty.handler.codec.http.HttpUtil.isOriginForm;
+import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
+import static io.netty.handler.codec.http2.Http2Exception.connectionError;
+import static io.netty.handler.codec.http2.Http2Exception.streamError;
+import static io.netty.util.AsciiString.EMPTY_STRING;
+import static io.netty.util.ByteProcessor.FIND_SEMI_COLON;
+import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.StringUtil.isNullOrEmpty;
+import static io.netty.util.internal.StringUtil.length;
 import io.netty.handler.codec.http.DefaultFullHttpRequest;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.FullHttpMessage;
@@ -34,18 +46,6 @@ import io.netty.util.AsciiString;
 import java.net.URI;
 import java.util.Map.Entry;
 
-import static io.netty.handler.codec.http.HttpScheme.HTTP;
-import static io.netty.handler.codec.http.HttpScheme.HTTPS;
-import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm;
-import static io.netty.handler.codec.http.HttpUtil.isOriginForm;
-import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
-import static io.netty.handler.codec.http2.Http2Exception.connectionError;
-import static io.netty.handler.codec.http2.Http2Exception.streamError;
-import static io.netty.util.AsciiString.EMPTY_STRING;
-import static io.netty.util.internal.ObjectUtil.checkNotNull;
-import static io.netty.util.internal.StringUtil.isNullOrEmpty;
-import static io.netty.util.internal.StringUtil.length;
-
 /**
  * Provides utility methods and constants for the HTTP/2 to HTTP conversion
  */
@@ -327,9 +327,33 @@ public final class HttpConversionUtil {
             final AsciiString aName = AsciiString.of(entry.getKey()).toLowerCase();
             if (!HTTP_TO_HTTP2_HEADER_BLACKLIST.contains(aName)) {
                 // https://tools.ietf.org/html/rfc7540#section-8.1.2.2 makes a special exception for TE
-                if (!aName.contentEqualsIgnoreCase(HttpHeaderNames.TE) ||
+                if (aName.contentEqualsIgnoreCase(HttpHeaderNames.TE) &&
-                    AsciiString.contentEqualsIgnoreCase(entry.getValue(), HttpHeaderValues.TRAILERS)) {
+                    !AsciiString.contentEqualsIgnoreCase(entry.getValue(), HttpHeaderValues.TRAILERS)) {
-                    out.add(aName, AsciiString.of(entry.getValue()));
+                    throw new IllegalArgumentException("Invalid value for " + HttpHeaderNames.TE + ": " +
+                                                        entry.getValue());
+                }
+                if (aName.contentEqualsIgnoreCase(HttpHeaderNames.COOKIE)) {
+                    AsciiString value = AsciiString.of(entry.getValue());
+                    // split up cookies to allow for better compression
+                    // https://tools.ietf.org/html/rfc7540#section-8.1.2.5
+                    int index = value.forEachByte(FIND_SEMI_COLON);
+                    if (index != -1) {
+                        int start = 0;
+                        do {
+                            out.add(HttpHeaderNames.COOKIE, value.subSequence(start, index, false));
+                            // skip 2 characters "; " (see https://tools.ietf.org/html/rfc6265#section-4.2.1)
+                            start = index + 2;
+                        } while (start < value.length() &&
+                                (index = value.forEachByte(start, value.length() - start, FIND_SEMI_COLON)) != -1);
+                        if (start >= value.length()) {
+                            throw new IllegalArgumentException("cookie value is of unexpected format: " + value);
+                        }
+                        out.add(HttpHeaderNames.COOKIE, value.subSequence(start, value.length(), false));
+                    } else {
+                        out.add(HttpHeaderNames.COOKIE, value);
+                    }
+                } else {
+                    out.add(aName, entry.getValue());
                 }
             }
         }
@@ -446,7 +470,15 @@ public final class HttpConversionUtil {
                     throw streamError(streamId, PROTOCOL_ERROR,
                             "Invalid HTTP/2 header '%s' encountered in translation to HTTP/1.x", name);
                 }
-                output.add(AsciiString.of(name), AsciiString.of(value));
+                if (HttpHeaderNames.COOKIE.equals(name)) {
+                    // combine the cookie values into 1 header entry.
+                    // https://tools.ietf.org/html/rfc7540#section-8.1.2.5
+                    String existingCookie = output.getAsString(HttpHeaderNames.COOKIE);
+                    output.set(HttpHeaderNames.COOKIE,
+                               (existingCookie != null) ? (existingCookie + "; " + value) : value);
+                } else {
+                    output.add(name, value);
+                }
             }
         }
     }

codec-http2/src/test/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerTest.java

@@ -140,6 +140,27 @@ public class HttpToHttp2ConnectionHandlerTest {
         verifyHeadersOnly(http2Headers, writePromise, clientChannel.writeAndFlush(request, writePromise));
     }
 
+    @Test
+    public void testMultipleCookieEntriesAreCombined() throws Exception {
+        bootstrapEnv(2, 1, 0);
+        final FullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, GET,
+                "http://my-user_name@www.example.org:5555/example");
+        final HttpHeaders httpHeaders = request.headers();
+        httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 5);
+        httpHeaders.set(HttpHeaderNames.HOST, "my-user_name@www.example.org:5555");
+        httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "http");
+        httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
+        final Http2Headers http2Headers =
+                new DefaultHttp2Headers().method(new AsciiString("GET")).path(new AsciiString("/example"))
+                .authority(new AsciiString("www.example.org:5555")).scheme(new AsciiString("http"))
+                .add(HttpHeaderNames.COOKIE, "a=b")
+                .add(HttpHeaderNames.COOKIE, "c=d")
+                .add(HttpHeaderNames.COOKIE, "e=f");
+
+        ChannelPromise writePromise = newPromise();
+        verifyHeadersOnly(http2Headers, writePromise, clientChannel.writeAndFlush(request, writePromise));
+    }
+
     @Test
     public void testOriginFormRequestTargetHandled() throws Exception {
         bootstrapEnv(2, 1, 0);

codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java

@@ -157,6 +157,75 @@ public class InboundHttp2ToHttpAdapterTest {
         }
     }
 
+    @Test
+    public void clientRequestSingleHeaderCookieSplitIntoMultipleEntries() throws Exception {
+        boostrapEnv(1, 1, 1);
+        final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
+                "/some/path/resource2", true);
+        try {
+            HttpHeaders httpHeaders = request.headers();
+            httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "https");
+            httpHeaders.set(HttpHeaderNames.HOST, "example.org");
+            httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3);
+            httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, 0);
+            httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
+            final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).
+                    scheme(new AsciiString("https")).authority(new AsciiString("example.org"))
+                    .path(new AsciiString("/some/path/resource2"))
+                    .add(HttpHeaderNames.COOKIE, "a=b")
+                    .add(HttpHeaderNames.COOKIE, "c=d")
+                    .add(HttpHeaderNames.COOKIE, "e=f");
+            runInChannel(clientChannel, new Http2Runnable() {
+                @Override
+                public void run() {
+                    frameWriter.writeHeaders(ctxClient(), 3, http2Headers, 0, true, newPromiseClient());
+                    ctxClient().flush();
+                }
+            });
+            awaitRequests();
+            ArgumentCaptor<FullHttpMessage> requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class);
+            verify(serverListener).messageReceived(requestCaptor.capture());
+            capturedRequests = requestCaptor.getAllValues();
+            assertEquals(request, capturedRequests.get(0));
+        } finally {
+            request.release();
+        }
+    }
+
+    @Test
+    public void clientRequestSingleHeaderCookieSplitIntoMultipleEntries2() throws Exception {
+        boostrapEnv(1, 1, 1);
+        final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
+                "/some/path/resource2", true);
+        try {
+            HttpHeaders httpHeaders = request.headers();
+            httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "https");
+            httpHeaders.set(HttpHeaderNames.HOST, "example.org");
+            httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3);
+            httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, 0);
+            httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
+            final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).
+                    scheme(new AsciiString("https")).authority(new AsciiString("example.org"))
+                    .path(new AsciiString("/some/path/resource2"))
+                    .add(HttpHeaderNames.COOKIE, "a=b; c=d")
+                    .add(HttpHeaderNames.COOKIE, "e=f");
+            runInChannel(clientChannel, new Http2Runnable() {
+                @Override
+                public void run() {
+                    frameWriter.writeHeaders(ctxClient(), 3, http2Headers, 0, true, newPromiseClient());
+                    ctxClient().flush();
+                }
+            });
+            awaitRequests();
+            ArgumentCaptor<FullHttpMessage> requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class);
+            verify(serverListener).messageReceived(requestCaptor.capture());
+            capturedRequests = requestCaptor.getAllValues();
+            assertEquals(request, capturedRequests.get(0));
+        } finally {
+            request.release();
+        }
+    }
+
     @Test
     public void clientRequestSingleHeaderNonAsciiShouldThrow() throws Exception {
         boostrapEnv(1, 1, 1);

common/src/main/java/io/netty/util/AsciiString.java

@@ -276,7 +276,7 @@ public final class AsciiString implements CharSequence, Comparable<CharSequence>
     }
 
     private int forEachByte0(int index, int length, ByteProcessor visitor) throws Exception {
-        final int len = offset + length;
+        final int len = offset + index + length;
         for (int i = offset + index; i < len; ++i) {
             if (!visitor.process(value[i])) {
                 return i - offset;

common/src/main/java/io/netty/util/ByteProcessor.java

@@ -120,6 +120,16 @@ public interface ByteProcessor {
         }
     };
 
+    /**
+     * Aborts on a {@code CR (';')}.
+     */
+    ByteProcessor FIND_SEMI_COLON = new ByteProcessor() {
+        @Override
+        public boolean process(byte value) throws Exception {
+            return value != ';';
+        }
+    };
+
     /**
      * @return {@code true} if the processor wants to continue the loop and handle the next byte in the buffer.
      *         {@code false} if the processor wants to stop handling bytes and abort the loop.

common/src/test/java/io/netty/util/AsciiStringMemoryTest.java

@@ -17,6 +17,7 @@ package io.netty.util;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import io.netty.util.ByteProcessor.IndexOfProcessor;
 
 import java.util.Random;
 import java.util.concurrent.atomic.AtomicReference;
@@ -102,6 +103,18 @@ public class AsciiStringMemoryTest {
         assertEquals(bAsciiString.length(), bCount.get().intValue());
     }
 
+    @Test
+    public void forEachWithIndexEndTest() throws Exception {
+        assertNotEquals(-1, aAsciiString.forEachByte(aAsciiString.length() - 1,
+                1, new IndexOfProcessor(aAsciiString.byteAt(aAsciiString.length() - 1))));
+    }
+
+    @Test
+    public void forEachWithIndexBeginTest() throws Exception {
+        assertNotEquals(-1, aAsciiString.forEachByte(0,
+                1, new IndexOfProcessor(aAsciiString.byteAt(0))));
+    }
+
     @Test
     public void forEachDescTest() throws Exception {
         final AtomicReference<Integer> aCount = new AtomicReference<Integer>(0);
@@ -128,6 +141,18 @@ public class AsciiStringMemoryTest {
         assertEquals(bAsciiString.length(), bCount.get().intValue());
     }
 
+    @Test
+    public void forEachDescWithIndexEndTest() throws Exception {
+        assertNotEquals(-1, bAsciiString.forEachByteDesc(bAsciiString.length() - 1,
+                1, new IndexOfProcessor(bAsciiString.byteAt(bAsciiString.length() - 1))));
+    }
+
+    @Test
+    public void forEachDescWithIndexBeginTest() throws Exception {
+        assertNotEquals(-1, bAsciiString.forEachByteDesc(0,
+                1, new IndexOfProcessor(bAsciiString.byteAt(0))));
+    }
+
     @Test
     public void subSequenceTest() {
         final int start = 12;