Skip to content

[UNDERTOW-2603] Fix double cookie on quoted value #1790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[UNDERTOW-2603] Fix double cookie on quoted value #1790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 35 additions & 6 deletions core/src/main/java/io/undertow/util/Cookies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,13 @@ public static void parseRequestCookies(int maxCookies, boolean allowEqualInValue
static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator) {
return parseRequestCookies(maxCookies, allowEqualInValue, cookies, commaIsSeperator, LegacyCookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0);
}

static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0) {
static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0, boolean rfc6265ParsingDisabled){
if (cookies == null) {
return new TreeMap<>();
}
final Set<Cookie> parsedCookies = new HashSet<>();
for (String cookie : cookies) {
parseCookie(cookie, parsedCookies, maxCookies, allowEqualInValue, commaIsSeperator, allowHttpSepartorsV0, true);
parseCookie(cookie, parsedCookies, maxCookies, allowEqualInValue, commaIsSeperator, allowHttpSepartorsV0, rfc6265ParsingDisabled);
}

final Map<String, Cookie> retVal = new TreeMap<>();
Expand All @@ -242,6 +241,9 @@ static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqua
}
return retVal;
}
static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0) {
return parseRequestCookies(maxCookies, allowEqualInValue, cookies, commaIsSeperator, allowHttpSepartorsV0, true);
}

static void parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, Set<Cookie> parsedCookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0, boolean rfc6265ParsingDisabled) {
if (cookies != null) {
Expand Down Expand Up @@ -320,13 +322,40 @@ private static void parseCookie(final String cookie, final Set<Cookie> parsedCoo
case 3: {
//extract quoted value
if (c == '"') {
int index = i;
if (!rfc6265ParsingDisabled && inQuotes) {
start = start - 1;
inQuotes = false;
i++;
cookieCount = createCookie(name, containsEscapedQuotes ? unescapeDoubleQuotes(cookie.substring(start, i)) : cookie.substring(start, i), maxCookies, cookieCount, cookies, additional);
}
inQuotes = false;
final boolean existed = cookies.containsKey(name);
cookieCount = createCookie(name, containsEscapedQuotes ? unescapeDoubleQuotes(cookie.substring(start, i)) : cookie.substring(start, i), maxCookies, cookieCount, cookies, additional);
//if there is more, make sure next is separator
if (index + 1 < cookie.length() && (cookie.charAt(index + 1) == ';' // Cookie: key="\"; key2=...
|| (commaIsSeperator && cookie.charAt(index + 1) == ',')) // Cookie: key="\", key2=...
|| index+1 == cookie.length()) { //end of cookie
//adjust position to delimiter and let state == 0 spin again whole thing
i++;
} else {
cookieCount = createCookie(name, containsEscapedQuotes ? unescapeDoubleQuotes(cookie.substring(start, i)) : cookie.substring(start, i), maxCookies, cookieCount, cookies, additional);
if(UndertowLogger.REQUEST_LOGGER.isTraceEnabled()) {
UndertowLogger.REQUEST_LOGGER.trace("Ignoring invalid cookies in header " + cookie);
}
if(!existed) {
//RN we dont add copy, so lets not remove proper cookie that we stored
//prior to this one
cookies.remove(name);
}
additional.remove(name);
//seek next separator
while(i<cookie.length()) {
char seeker = cookie.charAt(i);
if(!(seeker == ';' // Cookie: key="\"; key2=...
|| (commaIsSeperator && seeker == ','))) {
i++;
} else {
break;
}
}
}
state = 0;
start = i + 1;
Expand Down
40 changes: 40 additions & 0 deletions core/src/test/java/io/undertow/util/CookiesTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,46 @@ public void testCommaSeparatedCookies() {
cookie = cookies.get("SHIPPING");
Assert.assertNotNull(cookie);
Assert.assertEquals("FEDEX", cookie.getValue());

cookies = Cookies.parseRequestCookies(5, false, Arrays.asList("CUSTOMER=\"WILE_E_COYOTE\", BAD_CUSTOMER=\"APPLE\" IGNORED=PART, SHIPPING=FEDEX" ), true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this should throw exception or not, test case has similar test: testNoDoubleQuoteTermination

Assert.assertEquals(2, cookies.size());
cookie = cookies.get("CUSTOMER");
Assert.assertNotNull(cookie);
Assert.assertEquals("WILE_E_COYOTE", cookie.getValue());
cookie = cookies.get("SHIPPING");
Assert.assertNotNull(cookie);
Assert.assertEquals("FEDEX", cookie.getValue());
}

@Test
public void testCommaSeparatedCookiesLegacyMode() {
Map<String, Cookie> cookies = Cookies.parseRequestCookies(2, false, Arrays.asList("CUSTOMER=\"WILE_E_COYOTE\", SHIPPING=FEDEX" ), true, LegacyCookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0, false);
Assert.assertEquals(2, cookies.size());
Cookie cookie = cookies.get("CUSTOMER");
Assert.assertNotNull(cookie);
Assert.assertEquals("\"WILE_E_COYOTE\"", cookie.getValue());
cookie = cookies.get("SHIPPING");
Assert.assertNotNull(cookie);
Assert.assertEquals("FEDEX", cookie.getValue());

//also make sure semi colon works as normal
cookies = Cookies.parseRequestCookies(2, false, Arrays.asList("CUSTOMER=\"WILE_E_COYOTE\"; SHIPPING=FEDEX" ), true, LegacyCookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0, false);
Assert.assertEquals(2, cookies.size());
cookie = cookies.get("CUSTOMER");
Assert.assertNotNull(cookie);
Assert.assertEquals("\"WILE_E_COYOTE\"", cookie.getValue());
cookie = cookies.get("SHIPPING");
Assert.assertNotNull(cookie);
Assert.assertEquals("FEDEX", cookie.getValue());

cookies = Cookies.parseRequestCookies(5, false, Arrays.asList("CUSTOMER=\"WILE_E_COYOTE\", BAD_CUSTOMER=\"APPLE\" IGNORED=PART, SHIPPING=FEDEX" ), true, LegacyCookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0, false);
Assert.assertEquals(2, cookies.size());
cookie = cookies.get("CUSTOMER");
Assert.assertNotNull(cookie);
Assert.assertEquals("\"WILE_E_COYOTE\"", cookie.getValue());
cookie = cookies.get("SHIPPING");
Assert.assertNotNull(cookie);
Assert.assertEquals("FEDEX", cookie.getValue());
}

@Test
Expand Down