From 217f8ce1fd26d7c125612ad6aa4f53819b403d12 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Sun, 24 Jun 2012 18:07:47 +0900 Subject: [PATCH] Fix #218: CookieDecoder.decode() throws StackOverflowError - Rewrote key-value decoder not using a regular expression --- .../handler/codec/http/CookieDecoder.java | 164 ++++++++++++------ .../handler/codec/http/CookieDecoderTest.java | 63 ++++++- 2 files changed, 174 insertions(+), 53 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/CookieDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/CookieDecoder.java index 85a9b412ef..1c1f4c4c47 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/CookieDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/CookieDecoder.java @@ -21,8 +21,6 @@ import java.util.Collections; import java.util.List; import java.util.Set; import java.util.TreeSet; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * Decodes an HTTP header value into {@link Cookie}s. This decoder can decode @@ -41,12 +39,6 @@ import java.util.regex.Pattern; */ public class CookieDecoder { - private static final Pattern PATTERN = Pattern.compile( - // See: https://github.com/netty/netty/pull/96 - //"(?:\\s|[;,])*\\$*([^;=]+)(?:=(?:[\"']((?:\\\\.|[^\"])*)[\"']|([^;,]*)))?(\\s*(?:[;,]+\\s*|$))" - "(?:\\s|[;,])*\\$*([^;=]+)(?:=(?:[\"']((?:\\\\.|[^\"])*)[\"']|([^;]*)))?(\\s*(?:[;,]+\\s*|$))" - ); - private static final String COMMA = ","; /** @@ -175,58 +167,126 @@ public class CookieDecoder { } private static 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(); + final String header, final List names, final List values) { - // 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); + final int headerLen = header.length(); + loop: for (int i = 0;;) { - if (name == null) { - name = newName; - value = newValue == null? "" : newValue; - separator = newSeparator; - continue; + // Skip spaces and separators. + for (;;) { + if (i == headerLen) { + break loop; + } + switch (header.charAt(i)) { + case '\t': case '\n': case 0x0b: case '\f': case '\r': + case ' ': case ',': case ';': + i ++; + continue; + } + break; } - if (newValue == null && - !CookieHeaderNames.DISCARD.equalsIgnoreCase(newName) && - !CookieHeaderNames.SECURE.equalsIgnoreCase(newName) && - !CookieHeaderNames.HTTPONLY.equalsIgnoreCase(newName)) { - value = value + separator + newName; - separator = newSeparator; - continue; + // Skip '$'. + for (;;) { + if (i == headerLen) { + break loop; + } + if (header.charAt(i) == '$') { + i ++; + continue; + } + break; + } + + String name; + String value; + + if (i == headerLen) { + name = null; + value = null; + } else { + int newNameStart = i; + keyValLoop: for (;;) { + switch (header.charAt(i)) { + case ';': + // NAME; (no value till ';') + name = header.substring(newNameStart, i); + value = null; + break keyValLoop; + case '=': + // NAME=VALUE + name = header.substring(newNameStart, i); + i ++; + if (i == headerLen) { + // NAME= (empty value, i.e. nothing after '=') + value = ""; + break keyValLoop; + } + + int newValueStart = i; + char c = header.charAt(i); + if (c == '"' || c == '\'') { + // NAME="VALUE" or NAME='VALUE' + StringBuilder newValueBuf = new StringBuilder(header.length() - i); + final char q = c; + boolean hadBackslash = false; + i ++; + for (;;) { + if (i == headerLen) { + value = newValueBuf.toString(); + break keyValLoop; + } + if (hadBackslash) { + hadBackslash = false; + c = header.charAt(i ++); + switch (c) { + case '\\': case '"': case '\'': + // Escape last backslash. + newValueBuf.setCharAt(newValueBuf.length() - 1, c); + break; + default: + // Do not escape last backslash. + newValueBuf.append(c); + } + } else { + c = header.charAt(i ++); + if (c == q) { + value = newValueBuf.toString(); + break keyValLoop; + } + newValueBuf.append(c); + if (c == '\\') { + hadBackslash = true; + } + } + } + } else { + // NAME=VALUE; + int semiPos = header.indexOf(';', i); + if (semiPos > 0) { + value = header.substring(newValueStart, semiPos); + i = semiPos; + } else { + value = header.substring(newValueStart); + i = headerLen; + } + } + break keyValLoop; + default: + i ++; + } + + if (i == headerLen) { + // NAME (no value till the end of string) + name = header.substring(newNameStart); + value = null; + break; + } + } } 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 static String decodeValue(String value) { - if (value == null) { - return value; - } - return value.replace("\\\"", "\"").replace("\\\\", "\\"); } } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/CookieDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/CookieDecoderTest.java index 94e4ac047c..11982f1199 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/CookieDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/CookieDecoderTest.java @@ -271,7 +271,9 @@ public class CookieDecoderTest { "d=\"1\\\"2\\\"3\"," + "e=\"\\\"\\\"\"," + "f=\"1\\\"\\\"2\"," + - "g=\"\\\\\""; + "g=\"\\\\\"," + + "h=\"';,\\x\""; + Set cookies = new CookieDecoder().decode(source); Iterator it = cookies.iterator(); @@ -305,6 +307,10 @@ public class CookieDecoderTest { assertEquals("g", c.getName()); assertEquals("\\", c.getValue()); + c = it.next(); + assertEquals("h", c.getName()); + assertEquals("';,\\x", c.getValue()); + assertFalse(it.hasNext()); } @@ -390,4 +396,59 @@ public class CookieDecoderTest { assertEquals("HTTPOnly", c.getName()); assertEquals("", c.getValue()); } + + @Test + public void testDecodingLongValue() { + String longValue = + "b!!!$Q!!$ha!!!!!!" + + "%=J^wI!!3iD!!!!$=HbQW!!3iF!!!!#=J^wI!!3iH!!!!%=J^wI!!3iM!!!!%=J^wI!!3iS!!!!" + + "#=J^wI!!3iU!!!!%=J^wI!!3iZ!!!!#=J^wI!!3i]!!!!%=J^wI!!3ig!!!!%=J^wI!!3ij!!!!" + + "%=J^wI!!3ik!!!!#=J^wI!!3il!!!!$=HbQW!!3in!!!!%=J^wI!!3ip!!!!$=HbQW!!3iq!!!!" + + "$=HbQW!!3it!!!!%=J^wI!!3ix!!!!#=J^wI!!3j!!!!!$=HbQW!!3j%!!!!$=HbQW!!3j'!!!!" + + "%=J^wI!!3j(!!!!%=J^wI!!9mJ!!!!'=KqtH!!=SE!!M!!!!" + + "'=KqtH!!s1X!!!!$=MMyc!!s1_!!!!#=MN#O!!ypn!!!!'=KqtH!!ypr!!!!'=KqtH!#%h!!!!!" + + "%=KqtH!#%o!!!!!'=KqtH!#)H6!!!!!!'=KqtH!#]9R!!!!$=H/Lt!#]I6!!!!#=KqtH!#]Z#!!!!%=KqtH!#^*N!!!!" + + "#=KqtH!#^:m!!!!#=KqtH!#_*_!!!!%=J^wI!#`-7!!!!#=KqtH!#`T>!!!!'=KqtH!#`T?!!!!" + + "'=KqtH!#`TA!!!!'=KqtH!#`TB!!!!'=KqtH!#`TG!!!!'=KqtH!#`TP!!!!#=KqtH!#`U,!!!!" + + "'=KqtH!#`U/!!!!'=KqtH!#`U0!!!!#=KqtH!#`U9!!!!'=KqtH!#aEQ!!!!%=KqtH!#b<)!!!!" + + "'=KqtH!#c9-!!!!%=KqtH!#dxC!!!!%=KqtH!#dxE!!!!%=KqtH!#ev$!!!!'=KqtH!#fBi!!!!" + + "#=KqtH!#fBj!!!!'=KqtH!#fG)!!!!'=KqtH!#fG+!!!!'=KqtH!#g*B!!!!'=KqtH!$>hD!!!!+=J^x0!$?lW!!!!'=KqtH!$?ll!!!!'=KqtH!$?lm!!!!" + + "%=KqtH!$?mi!!!!'=KqtH!$?mx!!!!'=KqtH!$D7]!!!!#=J_#p!$D@T!!!!#=J_#p!$V cookies = new CookieDecoder().decode("bh=\"" + longValue + "\";"); + assertEquals(1, cookies.size()); + Cookie c = cookies.iterator().next(); + assertEquals("bh", c.getName()); + assertEquals(longValue, c.getValue()); + } }