Allows commas to separate cookies in RFC2965 compliance mode (#3045)

* Allows commas to separate cookies in RFC2965 compliance mode

* cleanup after review

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* revert accidental change

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2018-11-01 11:43:11 +01:00 committed by GitHub
parent 392260a232
commit 8dcd7e44d8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 125 additions and 150 deletions

View File

@ -129,7 +129,6 @@ public class CookieCutter
{ {
// Parse the header // Parse the header
String name = null; String name = null;
String value = null;
Cookie cookie = null; Cookie cookie = null;
@ -139,11 +138,11 @@ public class CookieCutter
boolean escaped=false; boolean escaped=false;
int tokenstart=-1; int tokenstart=-1;
int tokenend=-1; int tokenend=-1;
for (int i = 0, length = hdr.length(), last=length-1; i < length; i++) for (int i = 0, length = hdr.length(); i <= length; i++)
{ {
char c = hdr.charAt(i); char c = i==length?0:hdr.charAt(i);
// System.err.printf("i=%d c=%s v=%b q=%b e=%b u=%s s=%d e=%d%n" ,i,""+c,invalue,inQuoted,escaped,unquoted,tokenstart,tokenend); // System.err.printf("i=%d/%d c=%s v=%b q=%b/%b e=%b u=%s s=%d e=%d \t%s=%s%n" ,i,length,c==0?"|":(""+c),invalue,inQuoted,quoted,escaped,unquoted,tokenstart,tokenend,name,value);
// Handle quoted values for name or value // Handle quoted values for name or value
if (inQuoted) if (inQuoted)
@ -151,52 +150,39 @@ public class CookieCutter
if (escaped) if (escaped)
{ {
escaped=false; escaped=false;
unquoted.append(c); if (c>0)
unquoted.append(c);
else
{
unquoted.setLength(0);
inQuoted = false;
i--;
}
continue; continue;
} }
switch (c) switch (c)
{ {
case '"': case '"':
inQuoted=false; inQuoted = false;
if (i==last) quoted = true;
{ tokenstart = i;
value = unquoted.toString(); tokenend = -1;
unquoted.setLength(0);
}
else
{
quoted=true;
tokenstart=i;
tokenend=-1;
}
break; break;
case '\\': case '\\':
if (i==last) escaped = true;
{ continue;
unquoted.setLength(0);
inQuoted = false; case 0:
i--; // unterminated quote, let's ignore quotes
} unquoted.setLength(0);
else inQuoted = false;
{ i--;
escaped=true;
}
continue; continue;
default: default:
if (i==last) unquoted.append(c);
{
// unterminated quote, let's ignore quotes
unquoted.setLength(0);
inQuoted = false;
i--;
}
else
{
unquoted.append(c);
}
continue; continue;
} }
} }
@ -211,22 +197,88 @@ public class CookieCutter
case ' ': case ' ':
case '\t': case '\t':
break; break;
case ',':
if (_compliance!=CookieCompliance.RFC2965)
{
if (quoted)
{
// must have been a bad internal quote. let's fix as best we can
unquoted.append(hdr,tokenstart,i--);
inQuoted = true;
quoted = false;
continue;
}
if (tokenstart<0)
tokenstart = i;
tokenend=i;
continue;
}
// fall through
case 0:
case ';': case ';':
{
String value;
if (quoted) if (quoted)
{ {
value = unquoted.toString(); value = unquoted.toString();
unquoted.setLength(0); unquoted.setLength(0);
quoted = false; quoted = false;
} }
else if(tokenstart>=0 && tokenend>=0) else if(tokenstart>=0)
value = hdr.substring(tokenstart, tokenend+1); value = tokenend>=tokenstart?hdr.substring(tokenstart, tokenend+1):hdr.substring(tokenstart);
else else
value = ""; value = "";
try
{
if (name.startsWith("$"))
{
if (_compliance==CookieCompliance.RFC2965)
{
String lowercaseName = name.toLowerCase(Locale.ENGLISH);
switch(lowercaseName)
{
case "$path":
if (cookie!=null)
cookie.setPath(value);
break;
case "$domain":
if (cookie!=null)
cookie.setDomain(value);
break;
case "$port":
if (cookie!=null)
cookie.setComment("$port="+value);
break;
case "$version":
version = Integer.parseInt(value);
break;
default:
break;
}
}
}
else
{
cookie = new Cookie(name, value);
if (version > 0)
cookie.setVersion(version);
cookies.add(cookie);
}
}
catch (Exception e)
{
LOG.debug(e);
}
name = null;
tokenstart = -1; tokenstart = -1;
invalue=false; invalue=false;
break; break;
}
case '"': case '"':
if (tokenstart<0) if (tokenstart<0)
@ -243,20 +295,14 @@ public class CookieCutter
if (quoted) if (quoted)
{ {
// must have been a bad internal quote. let's fix as best we can // must have been a bad internal quote. let's fix as best we can
unquoted.append(hdr.substring(tokenstart,i)); unquoted.append(hdr,tokenstart,i--);
inQuoted = true; inQuoted = true;
quoted = false; quoted = false;
i--;
continue; continue;
} }
if (tokenstart<0) if (tokenstart<0)
tokenstart=i; tokenstart = i;
tokenend=i; tokenend=i;
if (i==last)
{
value = hdr.substring(tokenstart, tokenend+1);
break;
}
continue; continue;
} }
} }
@ -269,21 +315,6 @@ public class CookieCutter
case '\t': case '\t':
continue; continue;
case ';':
if (quoted)
{
name = unquoted.toString();
unquoted.setLength(0);
quoted = false;
}
else if(tokenstart>=0 && tokenend>=0)
{
name = hdr.substring(tokenstart, tokenend+1);
}
tokenstart = -1;
break;
case '=': case '=':
if (quoted) if (quoted)
{ {
@ -291,98 +322,29 @@ public class CookieCutter
unquoted.setLength(0); unquoted.setLength(0);
quoted = false; quoted = false;
} }
else if(tokenstart>=0 && tokenend>=0) else if(tokenstart>=0)
{ name = tokenend>=tokenstart?hdr.substring(tokenstart, tokenend+1):hdr.substring(tokenstart);
name = hdr.substring(tokenstart, tokenend+1);
}
tokenstart = -1; tokenstart = -1;
invalue=true; invalue = true;
break; break;
default: default:
if (quoted) if (quoted)
{ {
// must have been a bad internal quote. let's fix as best we can // must have been a bad internal quote. let's fix as best we can
unquoted.append(hdr.substring(tokenstart,i)); unquoted.append(hdr,tokenstart,i--);
inQuoted = true; inQuoted = true;
quoted = false; quoted = false;
i--;
continue; continue;
} }
if (tokenstart<0) if (tokenstart<0)
tokenstart=i; tokenstart=i;
tokenend=i; tokenend=i;
if (i==last)
break;
continue; continue;
} }
} }
} }
if (invalue && i==last && value==null)
{
if (quoted)
{
value = unquoted.toString();
unquoted.setLength(0);
quoted = false;
}
else if(tokenstart>=0 && tokenend>=0)
{
value = hdr.substring(tokenstart, tokenend+1);
}
else
value = "";
}
// If after processing the current character we have a value and a name, then it is a cookie
if (name!=null && value!=null)
{
try
{
if (name.startsWith("$"))
{
String lowercaseName = name.toLowerCase(Locale.ENGLISH);
if (_compliance==CookieCompliance.RFC6265)
{
// Ignore
}
else if ("$path".equals(lowercaseName))
{
if (cookie!=null)
cookie.setPath(value);
}
else if ("$domain".equals(lowercaseName))
{
if (cookie!=null)
cookie.setDomain(value);
}
else if ("$port".equals(lowercaseName))
{
if (cookie!=null)
cookie.setComment("$port="+value);
}
else if ("$version".equals(lowercaseName))
{
version = Integer.parseInt(value);
}
}
else
{
cookie = new Cookie(name, value);
if (version > 0)
cookie.setVersion(version);
cookies.add(cookie);
}
}
catch (Exception e)
{
LOG.debug(e);
}
name = null;
value = null;
}
} }
} }

View File

@ -764,12 +764,15 @@ public class Request implements HttpServletRequest
} }
_cookiesExtracted = true; _cookiesExtracted = true;
for (String c : metadata.getFields().getValuesList(HttpHeader.COOKIE)) for (HttpField field : metadata.getFields())
{ {
if (_cookies == null) if (field.getHeader()==HttpHeader.COOKIE)
_cookies = new CookieCutter(getHttpChannel().getHttpConfiguration().getCookieCompliance()); {
_cookies.addCookieField(c); if (_cookies==null)
_cookies = new CookieCutter(getHttpChannel().getHttpConfiguration().getCookieCompliance());
_cookies.addCookieField(field.getValue());
}
} }
//Javadoc for Request.getCookies() stipulates null for no cookies //Javadoc for Request.getCookies() stipulates null for no cookies

View File

@ -139,17 +139,22 @@ public class CookieCutterTest
* Example from RFC2965 * Example from RFC2965
*/ */
@Test @Test
@Disabled("comma separation no longer supported by new RFC6265")
public void testRFC2965_CookieSpoofingExample() public void testRFC2965_CookieSpoofingExample()
{ {
String rawCookie = "$Version=\"1\"; session_id=\"1234\", " + String rawCookie = "$Version=\"1\"; session_id=\"1234\", " +
"$Version=\"1\"; session_id=\"1111\"; $Domain=\".cracker.edu\""; "$Version=\"1\"; session_id=\"1111\"; $Domain=\".cracker.edu\"";
Cookie cookies[] = parseCookieHeaders(CookieCompliance.RFC6265,rawCookie);
Cookie cookies[] = parseCookieHeaders(CookieCompliance.RFC2965,rawCookie);
assertThat("Cookies.length", cookies.length, is(2)); assertThat("Cookies.length", cookies.length, is(2));
assertCookie("Cookies[0]", cookies[0], "session_id", "1234", 1, null); assertCookie("Cookies[0]", cookies[0], "session_id", "1234", 1, null);
assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 1, null); assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 1, null);
cookies = parseCookieHeaders(CookieCompliance.RFC6265,rawCookie);
assertThat("Cookies.length", cookies.length, is(2));
assertCookie("Cookies[0]", cookies[0], "session_id", "1234\", $Version=\"1", 0, null);
assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 0, null);
} }
/** /**

View File

@ -144,7 +144,12 @@ public class CookieCutter_LenientTest
Arguments.of("GAPS=1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-", "GAPS", "1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-"), Arguments.of("GAPS=1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-", "GAPS", "1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-"),
// Strong abuse of cookie spec (lots of tspecials) // Strong abuse of cookie spec (lots of tspecials)
Arguments.of("$Version=0; rToken=F_TOKEN''!--\"</a>=&{()}", "rToken", "F_TOKEN''!--\"</a>=&{()}") Arguments.of("$Version=0; rToken=F_TOKEN''!--\"</a>=&{()}", "rToken", "F_TOKEN''!--\"</a>=&{()}"),
// Commas that were not commas
Arguments.of("name=foo,bar","name","foo,bar"),
Arguments.of("name=foo , bar","name","foo , bar"),
Arguments.of("name=foo , bar, bob","name","foo , bar, bob")
); );
} }