From abe2a88d60bf1d11151dc646dd86e1bb113e61a6 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 19 Nov 2009 09:46:30 +0000 Subject: [PATCH] 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 --- .../handler/codec/http/CookieDecoder.java | 81 ++++++++++++------- .../handler/codec/http/CookieDecoderTest.java | 39 +++++++++ 2 files changed, 91 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java b/src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java index aa6db8595b..ab046009b5 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java +++ b/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 decode(String header) { - Matcher m = PATTERN.matcher(header); List names = new ArrayList(8); List values = new ArrayList(8); - int pos = 0; - 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); - } + extractKeyValuePairs(header, names, values); 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 names, List 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; diff --git a/src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java b/src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java index 0dde19894b..5877cbc17d 100644 --- a/src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java +++ b/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 cookies = new CookieDecoder().decode(source); + Iterator 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()); + } }