Resolved issue: NETTY-255 (Make CookieDecoder more robust)

* Modified CookieDecoder not to recognize commas and semicolons as attribute separators if it's impossible to decode the next entry.
* Added a test case for decoding Google Analytics cookie which raised this issue initially

src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java

@@ -46,7 +46,7 @@ import java.util.regex.Pattern;
 public class CookieDecoder {
 
     private final static Pattern PATTERN =
-        Pattern.compile("(?:\\s|[;,])*\\$*([^;=]+)(?:=(?:[\"']((?:\\\\.|[^\"])*)[\"']|([^;,]*)))?\\s*(?:[;,]+|$)");
+        Pattern.compile("(?:\\s|[;,])*\\$*([^;=]+)(?:=(?:[\"']((?:\\\\.|[^\"])*)[\"']|([^;,]*)))?(\\s*(?:[;,]+\\s*|$))");
 
     private final static String COMMA = ",";
 
@@ -63,42 +63,16 @@ public class CookieDecoder {
      * @return the decoded {@link Cookie}s
      */
     public Set<Cookie> decode(String header) {
-        Matcher m = PATTERN.matcher(header);
         List<String> names = new ArrayList<String>(8);
         List<String> values = new ArrayList<String>(8);
-        int pos = 0;
+        extractKeyValuePairs(header, names, values);
-        int version = 0;
-        while (m.find(pos)) {
-            pos = m.end();
-
-            // Extract name and value pair from the match.
-            String name = m.group(1);
-            String value = m.group(3);
-            if (value == null) {
-                value = decodeValue(m.group(2));
-            }
-
-            // An exceptional case:
-            // 'Expires' attribute can contain a comma without surrounded with quotes.
-            if (name.equalsIgnoreCase(CookieHeaderNames.EXPIRES) &&
-                value.length() <= 9) { // Longest day of week is 'Wednesday'.
-                if (m.find(pos)) {
-                    value = value + ", " + m.group(1);
-                    pos = m.end();
-                } else {
-                    continue;
-                }
-            }
-
-            names.add(name);
-            values.add(value);
-        }
 
         if (names.isEmpty()) {
             return Collections.emptySet();
         }
 
         int i;
+        int version = 0;
 
         // $Version is the only attribute that can appear before the actual
         // cookie name-value pair.
@@ -208,6 +182,55 @@ public class CookieDecoder {
         return cookies;
     }
 
+    private void extractKeyValuePairs(
+            String header, List<String> names, List<String> values) {
+        Matcher m = PATTERN.matcher(header);
+        int pos = 0;
+        String name = null;
+        String value = null;
+        String separator = null;
+        while (m.find(pos)) {
+            pos = m.end();
+
+            // Extract name and value pair from the match.
+            String newName = m.group(1);
+            String newValue = m.group(3);
+            if (newValue == null) {
+                newValue = decodeValue(m.group(2));
+            }
+            String newSeparator = m.group(4);
+
+            if (name == null) {
+                name = newName;
+                value = newValue == null? "" : newValue;
+                separator = newSeparator;
+                continue;
+            }
+
+            if (newValue == null &&
+                !CookieHeaderNames.DISCARD.equalsIgnoreCase(newName) &&
+                !CookieHeaderNames.SECURE.equalsIgnoreCase(newName) &&
+                !CookieHeaderNames.HTTPONLY.equalsIgnoreCase(newName)) {
+                value = value + separator + newName;
+                separator = newSeparator;
+                continue;
+            }
+
+            names.add(name);
+            values.add(value);
+
+            name = newName;
+            value = newValue;
+            separator = newSeparator;
+        }
+
+        // The last entry
+        if (name != null) {
+            names.add(name);
+            values.add(value);
+        }
+    }
+
     private String decodeValue(String value) {
         if (value == null) {
             return value;

src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java

@@ -303,4 +303,43 @@ public class CookieDecoderTest {
         assertEquals("g", c.getName());
         assertEquals("\\", c.getValue());
     }
+
+    @Test
+    public void testDecodingGoogleAnalyticsCookie() {
+        String source =
+            "ARPT=LWUKQPSWRTUN04CKKJI; " +
+            "kw-2E343B92-B097-442c-BFA5-BE371E0325A2=unfinished furniture; " +
+            "__utma=48461872.1094088325.1258140131.1258140131.1258140131.1; " +
+            "__utmb=48461872.13.10.1258140131; __utmc=48461872; " +
+            "__utmz=48461872.1258140131.1.1.utmcsr=overstock.com|utmccn=(referral)|utmcmd=referral|utmcct=/Home-Garden/Furniture/Clearance,/clearance,/32/dept.html";
+        Set<Cookie> cookies = new CookieDecoder().decode(source);
+        Iterator<Cookie> it = cookies.iterator();
+        Cookie c;
+
+        c = it.next();
+        assertEquals("__utma", c.getName());
+        assertEquals("48461872.1094088325.1258140131.1258140131.1258140131.1", c.getValue());
+
+        c = it.next();
+        assertEquals("__utmb", c.getName());
+        assertEquals("48461872.13.10.1258140131", c.getValue());
+
+        c = it.next();
+        assertEquals("__utmc", c.getName());
+        assertEquals("48461872", c.getValue());
+
+        c = it.next();
+        assertEquals("__utmz", c.getName());
+        assertEquals("48461872.1258140131.1.1.utmcsr=overstock.com|utmccn=(referral)|utmcmd=referral|utmcct=/Home-Garden/Furniture/Clearance,/clearance,/32/dept.html", c.getValue());
+
+        c = it.next();
+        assertEquals("ARPT", c.getName());
+        assertEquals("LWUKQPSWRTUN04CKKJI", c.getValue());
+
+        c = it.next();
+        assertEquals("kw-2E343B92-B097-442c-BFA5-BE371E0325A2", c.getName());
+        assertEquals("unfinished furniture", c.getValue());
+
+        assertFalse(it.hasNext());
+    }
 }