From fd810a02ec3ef1da0deb541f7874ed87ad8adc3c Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 20 Jun 2024 18:11:45 +1000 Subject: [PATCH] Fix #11934 Servlet 6.1 Cookies Added compliance mode MAINTAIN_QUOTES to keep the quotes as part of the cookie value. Added mode RFC6265_QUOTED that includes this violation --- .../eclipse/jetty/http/CookieCompliance.java | 28 ++++++++++-- .../jetty/http/RFC6265CookieParser.java | 14 +++++- .../jetty/http/RFC6265CookieParserTest.java | 44 ++++++++++++++++--- 3 files changed, 75 insertions(+), 11 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java index 7e0a26f48a49..62cb56a9a7cf 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java @@ -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; @@ -142,10 +147,24 @@ public String getDescription() Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPACE_IN_VALUES) ); + /** + *

A CookieCompliance mode that enforces RFC 6265 compliance, + * but allows:

+ * + */ + 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 RFC 6265 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)); /** *

A CookieCompliance mode that enforces RFC 6265 compliance, @@ -176,13 +195,14 @@ public String getDescription() *

  • {@link Violation#BAD_QUOTES}
    • *
  • {@link Violation#COMMA_NOT_VALID_OCTET}
    • *
  • {@link Violation#RESERVED_NAMES_NOT_DOLLAR_PREFIXED}
    • + *
  • {@link Violation#MAINTAIN_QUOTES}
    • * */ 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 KNOWN_MODES = Arrays.asList(RFC6265, RFC6265_STRICT, RFC6265_LEGACY, RFC2965, RFC2965_LEGACY); + private static final List 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) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java index 8b54d16b0bc5..8e3cae8a69bf 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java @@ -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; @@ -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 == ';') @@ -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)) diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java index dc84b00087fb..1ba0a014e48e 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java @@ -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 @@ -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)); } /** @@ -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() { @@ -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; @@ -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 expected;