Skip to content

Commit

Permalink
Fix #11934 Servlet 6.1 Cookies
Browse files Browse the repository at this point in the history 
Added compliance mode MAINTAIN_QUOTES to keep the quotes as part of the cookie value.  Added mode RFC6265_QUOTED that includes this violation
  • Loading branch information
@gregw
gregw committed Jun 20, 2024
1 parent fd263c7 commit fd810a0
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 11 deletions.
28 changes: 24 additions & 4 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ public enum Violation implements ComplianceViolation
/**
* Allow spaces within values without quotes.
*/
SPACE_IN_VALUES("https://www.rfc-editor.org/rfc/rfc6265#section-5.2", "Space in value");
SPACE_IN_VALUES("https://www.rfc-editor.org/rfc/rfc6265#section-5.2", "Space in value"),

/**
* Maintain quotes around values .
*/
MAINTAIN_QUOTES("https://www.rfc-editor.org/rfc/rfc6265#section-4.1.1", "Quotes are part of the cookie value");

private final String url;
private final String description;
Expand Down Expand Up @@ -142,10 +147,24 @@ public String getDescription()
Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPACE_IN_VALUES)
);

/**
* <p>A CookieCompliance mode that enforces <a href="https://tools.ietf.org/html/rfc6265">RFC 6265</a> compliance,
* but allows:</p>
* <ul>
* <li>{@link Violation#INVALID_COOKIES}</li>
* <li>{@link Violation#OPTIONAL_WHITE_SPACE}</li>
* <li>{@link Violation#SPACE_IN_VALUES}</li>
* <li>{@link Violation#MAINTAIN_QUOTES}</li>
* </ul>
*/
public static final CookieCompliance RFC6265_QUOTED = new CookieCompliance("RFC6265_QUOTED", of(
Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPACE_IN_VALUES, Violation.MAINTAIN_QUOTES)
);

/**
* A CookieCompliance mode that enforces <a href="https://tools.ietf.org/html/rfc6265">RFC 6265</a> compliance.
*/
public static final CookieCompliance RFC6265_STRICT = new CookieCompliance("RFC6265_STRICT", noneOf(Violation.class));
public static final CookieCompliance RFC6265_STRICT = new CookieCompliance("RFC6265_STRICT", of(Violation.MAINTAIN_QUOTES));

/**
* <p>A CookieCompliance mode that enforces <a href="https://tools.ietf.org/html/rfc6265">RFC 6265</a> compliance,
Expand Down Expand Up @@ -176,13 +195,14 @@ public String getDescription()
* <li>{@link Violation#BAD_QUOTES}</li>
* <li>{@link Violation#COMMA_NOT_VALID_OCTET}</li>
* <li>{@link Violation#RESERVED_NAMES_NOT_DOLLAR_PREFIXED}</li>
* <li>{@link Violation#MAINTAIN_QUOTES}</li>
* </ul>
*/
public static final CookieCompliance RFC2965 = new CookieCompliance("RFC2965", complementOf(of(
Violation.BAD_QUOTES, Violation.COMMA_NOT_VALID_OCTET, Violation.RESERVED_NAMES_NOT_DOLLAR_PREFIXED)
Violation.BAD_QUOTES, Violation.COMMA_NOT_VALID_OCTET, Violation.RESERVED_NAMES_NOT_DOLLAR_PREFIXED, Violation.MAINTAIN_QUOTES)
));

private static final List<CookieCompliance> KNOWN_MODES = Arrays.asList(RFC6265, RFC6265_STRICT, RFC6265_LEGACY, RFC2965, RFC2965_LEGACY);
private static final List<CookieCompliance> KNOWN_MODES = Arrays.asList(RFC6265, RFC6265_QUOTED, RFC6265_STRICT, RFC6265_LEGACY, RFC2965, RFC2965_LEGACY);
private static final AtomicInteger __custom = new AtomicInteger();

public static CookieCompliance valueOf(String name)
Expand Down
14 changes: 13 additions & 1 deletion jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.eclipse.jetty.http.CookieCompliance.Violation.COMMA_SEPARATOR;
import static org.eclipse.jetty.http.CookieCompliance.Violation.ESCAPE_IN_QUOTES;
import static org.eclipse.jetty.http.CookieCompliance.Violation.INVALID_COOKIES;
import static org.eclipse.jetty.http.CookieCompliance.Violation.MAINTAIN_QUOTES;
import static org.eclipse.jetty.http.CookieCompliance.Violation.OPTIONAL_WHITE_SPACE;
import static org.eclipse.jetty.http.CookieCompliance.Violation.SPACE_IN_VALUES;
import static org.eclipse.jetty.http.CookieCompliance.Violation.SPECIAL_CHARS_IN_QUOTES;
Expand Down Expand Up @@ -194,6 +195,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
string.setLength(0);
if (c == '"')
{
if (_complianceMode.allows(MAINTAIN_QUOTES))
string.append(c);
state = State.IN_QUOTED_VALUE;
}
else if (c == ';')
Expand Down Expand Up @@ -276,7 +279,16 @@ else if (_complianceMode.allows(INVALID_COOKIES))
case IN_QUOTED_VALUE:
if (c == '"')
{
value = string.toString();
if (_complianceMode.allows(MAINTAIN_QUOTES))
{
string.append(c);
value = string.toString();
reportComplianceViolation(MAINTAIN_QUOTES, value);
}
else
{
value = string.toString();
}
state = State.AFTER_QUOTED_VALUE;
}
else if (c == '\\' && _complianceMode.allows(ESCAPE_IN_QUOTES))
Expand Down
44 changes: 38 additions & 6 deletions jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.junit.jupiter.params.provider.MethodSource;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.is;

public class RFC6265CookieParserTest
Expand Down Expand Up @@ -76,12 +77,12 @@ public void testRFC2965Single()
parser = new TestCookieParser(CookieCompliance.RFC6265_STRICT);
cookies = parser.parseFields(rawCookie);
assertThat("Cookies.length", cookies.size(), is(3));
assertCookie("Cookies[0]", cookies.get(0), "$Version", "1", 0, null);
assertCookie("Cookies[1]", cookies.get(1), "Customer", "WILE_E_COYOTE", 0, null);
assertCookie("Cookies[2]", cookies.get(2), "$Path", "/acme", 0, null);
assertCookie("Cookies[0]", cookies.get(0), "$Version", "\"1\"", 0, null);
assertCookie("Cookies[1]", cookies.get(1), "Customer", "\"WILE_E_COYOTE\"", 0, null);
assertCookie("Cookies[2]", cookies.get(2), "$Path", "\"/acme\"", 0, null);

// Normal cookies with attributes, so no violations
assertThat(parser.violations.size(), is(0));
// Three violations for each of the maintained quotes
assertThat(parser.violations.size(), is(3));
}

/**
Expand Down Expand Up @@ -296,6 +297,20 @@ public void testExcessiveSemicolons()
assertCookie("Cookies[1]", cookies[1], "xyz", "pdq", 0, null);
}

@Test
public void testRFC6265QuotesMaintained()
{
String rawCookie = "A=\"quotedValue\"";

Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie);
assertThat("Cookies.length", cookies.length, is(1));
assertCookie("Cookies[0]", cookies[0], "A", "quotedValue", 0, null);

cookies = parseCookieHeaders(CookieCompliance.RFC6265_QUOTED, rawCookie);
assertThat("Cookies.length", cookies.length, is(1));
assertCookie("Cookies[0]", cookies[0], "A", "\"quotedValue\"", 0, null);
}

@Test
public void testRFC2965QuotedEscape()
{
Expand Down Expand Up @@ -360,6 +375,23 @@ public void testRFC6265Cookie(Param param)
}
}

@ParameterizedTest
@MethodSource("parameters")
public void testRFC6265QuotedCookie(Param param)
{
Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265_QUOTED, param.input);

assertThat("Cookies.length", cookies.length, is(param.expected.size()));
boolean quoted = param.input.contains("\"");

for (int i = 0; i < cookies.length; i++)
{
Cookie cookie = cookies[i];
String value = cookie.getValue();
assertThat(param.expected.get(i), anyOf(is(cookie.getName() + "=" + value), is(cookie.getName() + "=" + QuotedCSV.unquote(value))));
}
}

static class Cookie
{
String name;
Expand Down Expand Up @@ -462,7 +494,7 @@ public void addCookie(String cookieName, String cookieValue, int cookieVersion,
}
}

private static class Param
public static class Param
{
private final String input;
private final List<String> expected;
Expand Down

0 comments on commit fd810a0

Please sign in to comment.