diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java index 28557b8800eb..cad836722848 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java @@ -94,7 +94,12 @@ public enum Violation implements ComplianceViolation /** * Whitespace was found around the cookie name and/or around the cookie value. */ - OPTIONAL_WHITE_SPACE("https://www.rfc-editor.org/rfc/rfc6265#section-5.2", "White space around name/value"); + OPTIONAL_WHITE_SPACE("https://www.rfc-editor.org/rfc/rfc6265#section-5.2", "White space around name/value"), + + /** + * Allow spaces within values without quotes. + */ + SPACE_IN_VALUES("https://www.rfc-editor.org/rfc/rfc6265#section-5.2", "Space in value"); private final String url; private final String description; @@ -130,10 +135,11 @@ public String getDescription() * */ public static final CookieCompliance RFC6265 = new CookieCompliance("RFC6265", of( - Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE) + Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPACE_IN_VALUES) ); /** @@ -151,10 +157,11 @@ public String getDescription() *
  • {@link Violation#INVALID_COOKIES}
  • *
  • {@link Violation#OPTIONAL_WHITE_SPACE}
  • *
  • {@link Violation#SPECIAL_CHARS_IN_QUOTES}
  • + *
  • {@link Violation#SPACE_IN_VALUES}
  • * */ public static final CookieCompliance RFC6265_LEGACY = new CookieCompliance("RFC6265_LEGACY", EnumSet.of( - Violation.ATTRIBUTES, Violation.BAD_QUOTES, Violation.ESCAPE_IN_QUOTES, Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPECIAL_CHARS_IN_QUOTES) + Violation.ATTRIBUTES, Violation.BAD_QUOTES, Violation.ESCAPE_IN_QUOTES, Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPECIAL_CHARS_IN_QUOTES, Violation.SPACE_IN_VALUES) ); /** diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java index 55ca55b6f30f..1cae2f74e673 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java @@ -26,6 +26,7 @@ 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.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; /** @@ -53,6 +54,7 @@ private enum State AFTER_NAME, VALUE, IN_VALUE, + SPACE_IN_VALUE, IN_QUOTED_VALUE, ESCAPED_VALUE, AFTER_QUOTED_VALUE, @@ -74,6 +76,7 @@ public void parseField(String field) String cookieComment = null; int cookieVersion = 0; boolean cookieInvalid = false; + int spaces = 0; int length = field.length(); StringBuilder string = new StringBuilder(); @@ -220,7 +223,13 @@ else if (_complianceMode.allows(INVALID_COOKIES)) break; case IN_VALUE: - if (c == ';' || c == ',' || c == ' ' || c == '\t') + if (c == ' ' && _complianceMode.allows(SPACE_IN_VALUES)) + { + reportComplianceViolation(SPACE_IN_VALUES, field); + spaces = 1; + state = State.SPACE_IN_VALUE; + } + else if (c == ' ' || c == ';' || c == ',' || c == '\t') { value = string.toString(); i--; @@ -241,6 +250,33 @@ else if (_complianceMode.allows(INVALID_COOKIES)) } break; + case SPACE_IN_VALUE: + if (c == ' ') + { + spaces++; + } + else if (c == ';' || c == ',' || c == '\t') + { + value = string.toString(); + i--; + state = State.END; + } + else if (token.isRfc6265CookieOctet()) + { + string.append(" ".repeat(spaces)).append(c); + state = State.IN_VALUE; + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + } + else + { + throw new InvalidCookieException("Bad Cookie value"); + } + break; + case IN_QUOTED_VALUE: if (c == '"') { @@ -265,6 +301,11 @@ else if (c == ',' && _complianceMode.allows(COMMA_NOT_VALID_OCTET)) reportComplianceViolation(COMMA_NOT_VALID_OCTET, field); string.append(c); } + else if (c == ' ' && _complianceMode.allows(SPACE_IN_VALUES)) + { + reportComplianceViolation(SPACE_IN_VALUES, field); + string.append(c); + } else if (_complianceMode.allows(INVALID_COOKIES)) { string.append(c); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserLenientTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserLenientTest.java index 2ba3d3d10764..0e8b5e6ed3ac 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserLenientTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserLenientTest.java @@ -22,6 +22,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; /** @@ -34,8 +35,8 @@ public static Stream data() { return Stream.of( // Simple test to verify behavior - Arguments.of("x=y", "x", "y"), - Arguments.of("key=value", "key", "value"), + Arguments.of("x=y", new String[]{"x", "y"}), + Arguments.of("key=value", new String[]{"key", "value"}), // Tests that conform to RFC2109 // RFC2109 - token values @@ -49,75 +50,83 @@ public static Stream data() // | "," | ";" | ":" | "\" | <"> // | "/" | "[" | "]" | "?" | "=" // | "{" | "}" | SP | HT - Arguments.of("abc=xyz", "abc", "xyz"), - Arguments.of("abc=!bar", "abc", "!bar"), - Arguments.of("abc=#hash", "abc", "#hash"), - Arguments.of("abc=#hash", "abc", "#hash"), + Arguments.of("abc=xyz", new String[]{"abc", "xyz"}), + Arguments.of("abc=!bar", new String[]{"abc", "!bar"}), + Arguments.of("abc=#hash", new String[]{"abc", "#hash"}), + Arguments.of("abc=#hash", new String[]{"abc", "#hash"}), // RFC2109 - quoted-string values // quoted-string = ( <"> *(qdtext) <"> ) // qdtext = > // rejected, as name cannot be DQUOTED - Arguments.of("\"a\"=bcd", null, null), - Arguments.of("\"a\"=\"b c d\"", null, null), + Arguments.of("\"a\"=bcd", new String[0]), + Arguments.of("\"a\"=\"b c d\"", new String[0]), // lenient with spaces and EOF - Arguments.of("abc=", "abc", ""), - Arguments.of("abc= ", "abc", ""), - Arguments.of("abc= x", "abc", "x"), - Arguments.of("abc = ", "abc", ""), - Arguments.of("abc = ;", "abc", ""), - Arguments.of("abc = ; ", "abc", ""), - Arguments.of("abc = x ", "abc", "x"), - Arguments.of("abc=\"\"", "abc", ""), - Arguments.of("abc= \"\" ", "abc", ""), - Arguments.of("abc= \"x\" ", "abc", "x"), - Arguments.of("abc= \"x\" ;", "abc", "x"), - Arguments.of("abc= \"x\" ; ", "abc", "x"), + Arguments.of("abc=", new String[]{"abc", ""}), + Arguments.of("abc= ", new String[]{"abc", ""}), + Arguments.of("abc= x", new String[]{"abc", "x"}), + Arguments.of("abc = ", new String[]{"abc", ""}), + Arguments.of("abc = ;", new String[]{"abc", ""}), + Arguments.of("abc = ; ", new String[]{"abc", ""}), + Arguments.of("abc = x ", new String[]{"abc", "x"}), + Arguments.of("abc=\"\"", new String[]{"abc", ""}), + Arguments.of("abc= \"\" ", new String[]{"abc", ""}), + Arguments.of("abc= \"x\" ", new String[]{"abc", "x"}), + Arguments.of("abc= \"x\" ;", new String[]{"abc", "x"}), + Arguments.of("abc= \"x\" ; ", new String[]{"abc", "x"}), + Arguments.of("abc = x y z ", new String[]{"abc", "x y z"}), + Arguments.of("abc = x y z ", new String[]{"abc", "x y z"}), + Arguments.of("abc=a:x b:y c:z", new String[]{"abc", "a:x b:y c:z"}), + Arguments.of("abc=a;x b;y c;z", new String[]{"abc", "a"}), + Arguments.of("abc=a ;b=x", new String[]{"abc", "a", "b", "x"}), + + Arguments.of("abc=x y;def=w z", new String[]{"abc", "x y", "def", "w z"}), + Arguments.of("abc=\"x y\";def=w z", new String[]{"abc", "x y", "def", "w z"}), + Arguments.of("abc=x y;def=\"w z\"", new String[]{"abc", "x y", "def", "w z"}), // The backslash character ("\") may be used as a single-character quoting // mechanism only within quoted-string and comment constructs. // quoted-pair = "\" CHAR - Arguments.of("abc=\"xyz\"", "abc", "xyz"), - Arguments.of("abc=\"!bar\"", "abc", "!bar"), - Arguments.of("abc=\"#hash\"", "abc", "#hash"), + Arguments.of("abc=\"xyz\"", new String[]{"abc", "xyz"}), + Arguments.of("abc=\"!bar\"", new String[]{"abc", "!bar"}), + Arguments.of("abc=\"#hash\"", new String[]{"abc", "#hash"}), // RFC2109 - other valid name types that conform to 'attr'/'token' syntax - Arguments.of("!f!o!o!=wat", "!f!o!o!", "wat"), - Arguments.of("__MyHost=Foo", "__MyHost", "Foo"), - Arguments.of("some-thing-else=to-parse", "some-thing-else", "to-parse"), - Arguments.of("$foo=bar", "$foo", "bar"), + Arguments.of("!f!o!o!=wat", new String[]{"!f!o!o!", "wat"}), + Arguments.of("__MyHost=Foo", new String[]{"__MyHost", "Foo"}), + Arguments.of("some-thing-else=to-parse", new String[]{"some-thing-else", "to-parse"}), + Arguments.of("$foo=bar", new String[]{"$foo", "bar"}), // Tests that conform to RFC6265 - Arguments.of("abc=foobar!", "abc", "foobar!"), - Arguments.of("abc=\"foobar!\"", "abc", "foobar!") - + Arguments.of("abc=foobar!", new String[]{"abc", "foobar!"}), + Arguments.of("abc=\"foobar!\"", new String[]{"abc", "foobar!"}) ); } @ParameterizedTest @MethodSource("data") - public void testLenientBehavior(String rawHeader, String expectedName, String expectedValue) + public void testLenientBehavior(String rawHeader, String... expected) { - TestCutter cutter = new TestCutter(); - cutter.parseField(rawHeader); + TestParser parser = new TestParser(); + parser.parseField(rawHeader); - if (expectedName == null) - assertThat("Cookies.length", cutter.names.size(), is(0)); - else + assertThat(expected.length % 2, is(0)); + assertThat(parser.names.size(), equalTo(expected.length / 2)); + for (int i = 0; i < expected.length; i += 2) { - assertThat("Cookies.length", cutter.names.size(), is(1)); - assertThat("Cookie.name", cutter.names.get(0), is(expectedName)); - assertThat("Cookie.value", cutter.values.get(0), is(expectedValue)); + int cookie = i / 2; + assertThat("Cookie.name " + cookie, parser.names.get(cookie), is(expected[i])); + assertThat("Cookie.value " + cookie, parser.values.get(cookie), is(expected[i + 1])); } } - static class TestCutter implements CookieParser.Handler + static class TestParser implements CookieParser.Handler { CookieParser parser; List names = new ArrayList<>(); List values = new ArrayList<>(); - protected TestCutter() + protected TestParser() { parser = new RFC6265CookieParser(this, CookieCompliance.RFC6265, null); } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java index 5fbf65bd657e..b664a6355319 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java @@ -315,15 +315,15 @@ public static List parameters() new Param("A=1 ; B=2; C=3", "A=1", "B=2", "C=3"), new Param("A=\"1; B=2\"; C=3", "C=3"), new Param("A=\"1; B=2; C=3"), - new Param("A=\"1 B=2\"; C=3", "C=3"), + new Param("A=\"1 B=2\"; C=3", "A=1 B=2", "C=3"), new Param("A=\"\"1; B=2; C=3", "B=2", "C=3"), new Param("A=\"\" ; B=2; C=3", "A=", "B=2", "C=3"), new Param("A=1\"\"; B=2; C=3", "B=2", "C=3"), new Param("A=1\"; B=2; C=3", "B=2", "C=3"), new Param("A=1\"1; B=2; C=3", "B=2", "C=3"), new Param("A= 1; B=2; C=3", "A=1", "B=2", "C=3"), - new Param("A=\" 1\"; B=2; C=3", "B=2", "C=3"), - new Param("A=\"1 \"; B=2; C=3", "B=2", "C=3"), + new Param("A=\" 1\"; B=2; C=3", "A= 1", "B=2", "C=3"), + new Param("A=\"1 \"; B=2; C=3", "A=1 ", "B=2", "C=3"), new Param("A=1,; B=2; C=3", "B=2", "C=3"), new Param("A=\"1,\"; B=2; C=3", "B=2", "C=3"), new Param("A=\\1; B=2; C=3", "B=2", "C=3"),