Skip to content

Commit 449252d

Browse files
committed
[UNDERTOW-2603] Fix double cookie on quoted value
1 parent 40a00c3 commit 449252d

File tree

2 files changed

+75
-6
lines changed

2 files changed

+75
-6
lines changed

core/src/main/java/io/undertow/util/Cookies.java

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,14 +226,13 @@ public static void parseRequestCookies(int maxCookies, boolean allowEqualInValue
226226
static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator) {
227227
return parseRequestCookies(maxCookies, allowEqualInValue, cookies, commaIsSeperator, LegacyCookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0);
228228
}
229-
230-
static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0) {
229+
static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0, boolean rfc6265ParsingDisabled){
231230
if (cookies == null) {
232231
return new TreeMap<>();
233232
}
234233
final Set<Cookie> parsedCookies = new HashSet<>();
235234
for (String cookie : cookies) {
236-
parseCookie(cookie, parsedCookies, maxCookies, allowEqualInValue, commaIsSeperator, allowHttpSepartorsV0, true);
235+
parseCookie(cookie, parsedCookies, maxCookies, allowEqualInValue, commaIsSeperator, allowHttpSepartorsV0, rfc6265ParsingDisabled);
237236
}
238237

239238
final Map<String, Cookie> retVal = new TreeMap<>();
@@ -242,6 +241,9 @@ static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqua
242241
}
243242
return retVal;
244243
}
244+
static Map<String, Cookie> parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0) {
245+
return parseRequestCookies(maxCookies, allowEqualInValue, cookies, commaIsSeperator, allowHttpSepartorsV0, true);
246+
}
245247

246248
static void parseRequestCookies(int maxCookies, boolean allowEqualInValue, List<String> cookies, Set<Cookie> parsedCookies, boolean commaIsSeperator, boolean allowHttpSepartorsV0, boolean rfc6265ParsingDisabled) {
247249
if (cookies != null) {
@@ -320,13 +322,40 @@ private static void parseCookie(final String cookie, final Set<Cookie> parsedCoo
320322
case 3: {
321323
//extract quoted value
322324
if (c == '"') {
325+
int index = i;
323326
if (!rfc6265ParsingDisabled && inQuotes) {
324327
start = start - 1;
325-
inQuotes = false;
326328
i++;
327-
cookieCount = createCookie(name, containsEscapedQuotes ? unescapeDoubleQuotes(cookie.substring(start, i)) : cookie.substring(start, i), maxCookies, cookieCount, cookies, additional);
329+
}
330+
inQuotes = false;
331+
final boolean existed = cookies.containsKey(name);
332+
cookieCount = createCookie(name, containsEscapedQuotes ? unescapeDoubleQuotes(cookie.substring(start, i)) : cookie.substring(start, i), maxCookies, cookieCount, cookies, additional);
333+
//if there is more, make sure next is separator
334+
if (index + 1 < cookie.length() && (cookie.charAt(index + 1) == ';' // Cookie: key="\"; key2=...
335+
|| (commaIsSeperator && cookie.charAt(index + 1) == ',')) // Cookie: key="\", key2=...
336+
|| index+1 == cookie.length()) { //end of cookie
337+
//adjust position to delimiter and let state == 0 spin again whole thing
338+
i++;
328339
} else {
329-
cookieCount = createCookie(name, containsEscapedQuotes ? unescapeDoubleQuotes(cookie.substring(start, i)) : cookie.substring(start, i), maxCookies, cookieCount, cookies, additional);
340+
if(UndertowLogger.REQUEST_LOGGER.isTraceEnabled()) {
341+
UndertowLogger.REQUEST_LOGGER.trace("Ignoring invalid cookies in header " + cookie);
342+
}
343+
if(!existed) {
344+
//RN we dont add copy, so lets not remove proper cookie that we stored
345+
//prior to this one
346+
cookies.remove(name);
347+
}
348+
additional.remove(name);
349+
//seek next separator
350+
while(i<cookie.length()) {
351+
char seeker = cookie.charAt(i);
352+
if(!(seeker == ';' // Cookie: key="\"; key2=...
353+
|| (commaIsSeperator && seeker == ','))) {
354+
i++;
355+
} else {
356+
break;
357+
}
358+
}
330359
}
331360
state = 0;
332361
start = i + 1;

core/src/test/java/io/undertow/util/CookiesTestCase.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,46 @@ public void testCommaSeparatedCookies() {
221221
cookie = cookies.get("SHIPPING");
222222
Assert.assertNotNull(cookie);
223223
Assert.assertEquals("FEDEX", cookie.getValue());
224+
225+
cookies = Cookies.parseRequestCookies(5, false, Arrays.asList("CUSTOMER=\"WILE_E_COYOTE\", BAD_CUSTOMER=\"APPLE\" IGNORED=PART, SHIPPING=FEDEX" ), true);
226+
Assert.assertEquals(2, cookies.size());
227+
cookie = cookies.get("CUSTOMER");
228+
Assert.assertNotNull(cookie);
229+
Assert.assertEquals("WILE_E_COYOTE", cookie.getValue());
230+
cookie = cookies.get("SHIPPING");
231+
Assert.assertNotNull(cookie);
232+
Assert.assertEquals("FEDEX", cookie.getValue());
233+
}
234+
235+
@Test
236+
public void testCommaSeparatedCookiesLegacyMode() {
237+
Map<String, Cookie> cookies = Cookies.parseRequestCookies(2, false, Arrays.asList("CUSTOMER=\"WILE_E_COYOTE\", SHIPPING=FEDEX" ), true, LegacyCookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0, false);
238+
Assert.assertEquals(2, cookies.size());
239+
Cookie cookie = cookies.get("CUSTOMER");
240+
Assert.assertNotNull(cookie);
241+
Assert.assertEquals("\"WILE_E_COYOTE\"", cookie.getValue());
242+
cookie = cookies.get("SHIPPING");
243+
Assert.assertNotNull(cookie);
244+
Assert.assertEquals("FEDEX", cookie.getValue());
245+
246+
//also make sure semi colon works as normal
247+
cookies = Cookies.parseRequestCookies(2, false, Arrays.asList("CUSTOMER=\"WILE_E_COYOTE\"; SHIPPING=FEDEX" ), true, LegacyCookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0, false);
248+
Assert.assertEquals(2, cookies.size());
249+
cookie = cookies.get("CUSTOMER");
250+
Assert.assertNotNull(cookie);
251+
Assert.assertEquals("\"WILE_E_COYOTE\"", cookie.getValue());
252+
cookie = cookies.get("SHIPPING");
253+
Assert.assertNotNull(cookie);
254+
Assert.assertEquals("FEDEX", cookie.getValue());
255+
256+
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);
257+
Assert.assertEquals(2, cookies.size());
258+
cookie = cookies.get("CUSTOMER");
259+
Assert.assertNotNull(cookie);
260+
Assert.assertEquals("\"WILE_E_COYOTE\"", cookie.getValue());
261+
cookie = cookies.get("SHIPPING");
262+
Assert.assertNotNull(cookie);
263+
Assert.assertEquals("FEDEX", cookie.getValue());
224264
}
225265

226266
@Test

0 commit comments

Comments
 (0)