From d747438366cffbbfaec80ac114a5b7fad2e9cf2c Mon Sep 17 00:00:00 2001
From: Stephane Landelle <slandelle@gatling.io>
Date: Fri, 18 Mar 2016 09:23:24 +0100
Subject: [PATCH] Add ! to allowed cookie value chars

Motivation:

! is missing from allowed cookie value chars, as per https://tools.ietf.org/html/rfc6265#section-4.1.1.
Issue was originally reported on Play!, see https://github.com/playframework/playframework/issues/4460#issuecomment-198177302.

Modifications:

Stick to RFC6265 ranges.

Result:

RFC6265 compliance, ! is supported
---
 .../handler/codec/http/cookie/CookieUtil.java | 60 +++++++++----------
 .../http/cookie/ServerCookieEncoderTest.java  | 30 ++++++++++
 2 files changed, 59 insertions(+), 31 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cookie/CookieUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/cookie/CookieUtil.java
index 9bc15f58a3..699587c858 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/cookie/CookieUtil.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/cookie/CookieUtil.java
@@ -24,45 +24,43 @@ final class CookieUtil {
 
     private static final BitSet VALID_COOKIE_VALUE_OCTETS = validCookieValueOctets();
 
-    private static final BitSet VALID_COOKIE_NAME_OCTETS = validCookieNameOctets(VALID_COOKIE_VALUE_OCTETS);
+    private static final BitSet VALID_COOKIE_NAME_OCTETS = validCookieNameOctets();
 
+    // cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
     // US-ASCII characters excluding CTLs, whitespace, DQUOTE, comma, semicolon, and backslash
     private static BitSet validCookieValueOctets() {
-        BitSet bits = new BitSet(8);
-        for (int i = 35; i < 127; i++) {
-            // US-ASCII characters excluding CTLs (%x00-1F / %x7F)
+        BitSet bits = new BitSet();
+        bits.set(0x21);
+        for (int i = 0x23; i <= 0x2B; i++) {
+            bits.set(i);
+        }
+        for (int i = 0x2D; i <= 0x3A; i++) {
+            bits.set(i);
+        }
+        for (int i = 0x3C; i <= 0x5B; i++) {
+            bits.set(i);
+        }
+        for (int i = 0x5D; i <= 0x7E; i++) {
             bits.set(i);
         }
-        bits.set('"', false);  // exclude DQUOTE = %x22
-        bits.set(',', false);  // exclude comma = %x2C
-        bits.set(';', false);  // exclude semicolon = %x3B
-        bits.set('\\', false); // exclude backslash = %x5C
         return bits;
     }
 
-    //    token          = 1*<any CHAR except CTLs or separators>
-    //    separators     = "(" | ")" | "<" | ">" | "@"
-    //                   | "," | ";" | ":" | "\" | <">
-    //                   | "/" | "[" | "]" | "?" | "="
-    //                   | "{" | "}" | SP | HT
-    private static BitSet validCookieNameOctets(BitSet validCookieValueOctets) {
-        BitSet bits = new BitSet(8);
-        bits.or(validCookieValueOctets);
-        bits.set('(', false);
-        bits.set(')', false);
-        bits.set('<', false);
-        bits.set('>', false);
-        bits.set('@', false);
-        bits.set(':', false);
-        bits.set('/', false);
-        bits.set('[', false);
-        bits.set(']', false);
-        bits.set('?', false);
-        bits.set('=', false);
-        bits.set('{', false);
-        bits.set('}', false);
-        bits.set(' ', false);
-        bits.set('\t', false);
+    // token = 1*<any CHAR except CTLs or separators>
+    // separators = "(" | ")" | "<" | ">" | "@"
+    // | "," | ";" | ":" | "\" | <">
+    // | "/" | "[" | "]" | "?" | "="
+    // | "{" | "}" | SP | HT
+    private static BitSet validCookieNameOctets() {
+        BitSet bits = new BitSet();
+        for (int i = 32; i < 127; i++) {
+            bits.set(i);
+        }
+        int[] separators = new int[]
+                { '(', ')', '<', '>', '@', ',', ';', ':', '\\', '"', '/', '[', ']', '?', '=', '{', '}', ' ', '\t' };
+        for (int separator : separators) {
+            bits.set(separator, false);
+        }
         return bits;
     }
 
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/cookie/ServerCookieEncoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/cookie/ServerCookieEncoderTest.java
index 58ca662d15..0ff4ed73be 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/cookie/ServerCookieEncoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/cookie/ServerCookieEncoderTest.java
@@ -24,7 +24,9 @@ import io.netty.handler.codec.http.HttpHeaderDateFormat;
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -76,6 +78,34 @@ public class ServerCookieEncoderTest {
         assertEquals(result, encodedCookies);
     }
 
+    @Test
+    public void illegalCharInCookieValueMakesStrictEncoderThrowsException() {
+
+        Set<Character> illegalChars = new HashSet<Character>();
+        // CTLs
+        for (int i = 0x00; i <= 0x1F; i++) {
+            illegalChars.add((char) i);
+        }
+        illegalChars.add((char) 0x7F);
+        // whitespace, DQUOTE, comma, semicolon, and backslash
+        for (char c : new char[] { ' ', '"', ',', ';', '\\' }) {
+            illegalChars.add(c);
+        }
+
+        int exceptions = 0;
+
+        for (char c : illegalChars) {
+            Cookie cookie = new DefaultCookie("name", "value" + c);
+            try {
+                ServerCookieEncoder.STRICT.encode(cookie);
+            } catch (IllegalArgumentException e) {
+                exceptions++;
+            }
+        }
+
+        assertEquals(illegalChars.size(), exceptions);
+    }
+
     @Test
     public void testEncodingMultipleCookiesLax() {
         List<String> result = new ArrayList<String>();