Fix #9334 Cookie Compliance (#9402)

Fix incorrect change to RFC6265 to not support dollars in cookie names.

Signed-off-by: gregw <gregw@webtide.com>

jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java

@@ -128,13 +128,12 @@ public class CookieCompliance implements ComplianceViolation.Mode
      * <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#ATTRIBUTES}</li>
      * <li>{@link Violation#INVALID_COOKIES}</li>
      * <li>{@link Violation#OPTIONAL_WHITE_SPACE}</li>
      * </ul>
      */
     public static final CookieCompliance RFC6265 = new CookieCompliance("RFC6265", of(
-        Violation.ATTRIBUTES, Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE)
+        Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE)
     );
 
     /**
@@ -146,6 +145,7 @@ public class CookieCompliance implements ComplianceViolation.Mode
      * <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#ATTRIBUTES}</li>
      * <li>{@link Violation#BAD_QUOTES}</li>
      * <li>{@link Violation#ESCAPE_IN_QUOTES}</li>
      * <li>{@link Violation#INVALID_COOKIES}</li>
@@ -153,8 +153,8 @@ public class CookieCompliance implements ComplianceViolation.Mode
      * <li>{@link Violation#SPECIAL_CHARS_IN_QUOTES}</li>
      * </ul>
      */
-    public static final CookieCompliance RFC6265_LEGACY = new CookieCompliance("RFC6265_LEGACY", of(
+    public static final CookieCompliance RFC6265_LEGACY = new CookieCompliance("RFC6265_LEGACY", EnumSet.of(
-        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)
     );
 
     /**
@@ -214,40 +214,45 @@ public class CookieCompliance implements ComplianceViolation.Mode
      */
     public static CookieCompliance from(String spec)
     {
-        Set<Violation> violations;
+        CookieCompliance compliance = valueOf(spec);
-        String[] elements = spec.split("\\s*,\\s*");
+        if (compliance == null)
-        switch (elements[0])
         {
-            case "0":
+            String[] elements = spec.split("\\s*,\\s*");
-                violations = noneOf(Violation.class);
+            Set<Violation> violations;
-                break;
+            switch (elements[0])
-
-            case "*":
-                violations = allOf(Violation.class);
-                break;
-
-            default:
             {
-                CookieCompliance mode = valueOf(elements[0]);
+                case "0" :
-                violations = (mode == null) ? noneOf(Violation.class) : copyOf(mode.getAllowed());
+                    violations = noneOf(Violation.class);
-                break;
+                    break;
+
+                case "*" :
+                    violations = allOf(Violation.class);
+                    break;
+
+                default :
+                {
+                    CookieCompliance mode = valueOf(elements[0]);
+                    if (mode == null)
+                        throw new IllegalArgumentException("Unknown base mode: " + elements[0]);
+                    violations = (mode.getAllowed().isEmpty())  ? noneOf(Violation.class) : copyOf(mode.getAllowed());
+                }
             }
-        }
 
-        for (int i = 1; i < elements.length; i++)
+            for (int i = 1; i < elements.length; i++)
-        {
+            {
-            String element = elements[i];
+                String element = elements[i];
-            boolean exclude = element.startsWith("-");
+                boolean exclude = element.startsWith("-");
-            if (exclude)
+                if (exclude)
-                element = element.substring(1);
+                    element = element.substring(1);
-            Violation section = Violation.valueOf(element);
+                Violation section = Violation.valueOf(element);
-            if (exclude)
+                if (exclude)
-                violations.remove(section);
+                    violations.remove(section);
-            else
+                else
-                violations.add(section);
+                    violations.add(section);
-        }
+            }
 
-        CookieCompliance compliance = new CookieCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
+            compliance = new CookieCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
+        }
         if (LOG.isDebugEnabled())
             LOG.debug("CookieCompliance from {}->{}", spec, compliance);
         return compliance;
@@ -290,4 +295,10 @@ public class CookieCompliance implements ComplianceViolation.Mode
     {
         return this == mode || getAllowed().containsAll(mode.getAllowed());
     }
+
+    @Override
+    public String toString()
+    {
+        return String.format("%s@%x%s", _name, hashCode(), _violations);
+    }
 }

jetty-http/src/main/java/org/eclipse/jetty/http/CookieParser.java

@@ -41,9 +41,9 @@ public interface CookieParser
         return new RFC6265CookieParser(handler, compliance, complianceListener);
     }
 
-    void parseField(String field);
+    void parseField(String field) throws InvalidCookieException;
 
-    default void parseFields(List<String> rawFields)
+    default void parseFields(List<String> rawFields) throws InvalidCookieException
     {
         // For each cookie field
         for (String field : rawFields)
@@ -57,4 +57,30 @@ public interface CookieParser
     {
         void addCookie(String name, String value, int version, String domain, String path, String comment);
     }
+
+    /**
+     * <p>The exception thrown when a cookie cannot be parsed and {@link CookieCompliance.Violation#INVALID_COOKIES} is not allowed.</p>
+     */
+    class InvalidCookieException extends IllegalArgumentException
+    {
+        public InvalidCookieException()
+        {
+            super();
+        }
+
+        public InvalidCookieException(String s)
+        {
+            super(s);
+        }
+
+        public InvalidCookieException(String message, Throwable cause)
+        {
+            super(message, cause);
+        }
+
+        public InvalidCookieException(Throwable cause)
+        {
+            super(cause);
+        }
+    }
 }

jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java

@@ -281,7 +281,7 @@ public class HttpCookie
 
     public String getSetCookie(CookieCompliance compliance)
     {
-        if (CookieCompliance.RFC6265.compliesWith(compliance))
+        if (compliance == null || CookieCompliance.RFC6265_LEGACY.compliesWith(compliance))
             return getRFC6265SetCookie();
         return getRFC2965SetCookie();
     }

jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java

@@ -85,7 +85,7 @@ public class RFC6265CookieParser implements CookieParser
             if (token == null)
             {
                 if (!_complianceMode.allows(INVALID_COOKIES))
-                     throw new IllegalArgumentException("Invalid Cookie character");
+                     throw new InvalidCookieException("Invalid Cookie character");
                 state = State.INVALID_COOKIE;
                 continue;
             }
@@ -100,7 +100,7 @@ public class RFC6265CookieParser implements CookieParser
 
                     if (token.isRfc2616Token())
                     {
-                        if (!StringUtil.isBlank(cookieName) && c != '$')
+                        if (!StringUtil.isBlank(cookieName) && !(c == '$' && (_complianceMode.allows(ATTRIBUTES) || _complianceMode.allows(ATTRIBUTE_VALUES))))
                         {
                             _handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment);
                             cookieName = null;
@@ -120,7 +120,7 @@ public class RFC6265CookieParser implements CookieParser
                     }
                     else
                     {
-                        throw new IllegalArgumentException("Bad Cookie name");
+                        throw new InvalidCookieException("Bad Cookie name");
                     }
 
                     break;
@@ -158,7 +158,7 @@ public class RFC6265CookieParser implements CookieParser
                     }
                     else
                     {
-                        throw new IllegalArgumentException("Bad Cookie name");
+                        throw new InvalidCookieException("Bad Cookie name");
                     }
                     break;
 
@@ -181,7 +181,7 @@ public class RFC6265CookieParser implements CookieParser
                     }
                     else
                     {
-                        throw new IllegalArgumentException("Bad Cookie");
+                        throw new InvalidCookieException("Bad Cookie");
                     }
                     break;
 
@@ -215,7 +215,7 @@ public class RFC6265CookieParser implements CookieParser
                     }
                     else
                     {
-                        throw new IllegalArgumentException("Bad Cookie value");
+                        throw new InvalidCookieException("Bad Cookie value");
                     }
                     break;
 
@@ -237,7 +237,7 @@ public class RFC6265CookieParser implements CookieParser
                     }
                     else
                     {
-                        throw new IllegalArgumentException("Bad Cookie value");
+                        throw new InvalidCookieException("Bad Cookie value");
                     }
                     break;
 
@@ -277,7 +277,7 @@ public class RFC6265CookieParser implements CookieParser
                     }
                     else
                     {
-                        throw new IllegalArgumentException("Bad Cookie quoted value");
+                        throw new InvalidCookieException("Bad Cookie quoted value");
                     }
                     break;
 
@@ -299,7 +299,7 @@ public class RFC6265CookieParser implements CookieParser
                     }
                     else
                     {
-                        throw new IllegalArgumentException("Bad Cookie quoted value");
+                        throw new InvalidCookieException("Bad Cookie quoted value");
                     }
                     break;
 
@@ -323,7 +323,7 @@ public class RFC6265CookieParser implements CookieParser
                         }
                         else
                         {
-                            throw new IllegalStateException("Comma cookie separator");
+                            throw new InvalidCookieException("Comma cookie separator");
                         }
                     }
                     else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE))
@@ -332,7 +332,6 @@ public class RFC6265CookieParser implements CookieParser
                         continue;
                     }
 
-                    boolean knownAttribute = true;
                     if (StringUtil.isBlank(attributeName))
                     {
                         cookieValue = value;
@@ -358,7 +357,10 @@ public class RFC6265CookieParser implements CookieParser
                                     cookieVersion = Integer.parseInt(value);
                                     break;
                                 default:
-                                    knownAttribute = false;
+                                    if (!_complianceMode.allows(INVALID_COOKIES))
+                                        throw new IllegalArgumentException("Invalid Cookie attribute");
+                                    reportComplianceViolation(INVALID_COOKIES, field);
+                                    state = State.INVALID_COOKIE;
                                     break;
                             }
                         }
@@ -366,31 +368,17 @@ public class RFC6265CookieParser implements CookieParser
                         {
                             reportComplianceViolation(ATTRIBUTES, field);
                         }
-                        else if (_complianceMode.allows(INVALID_COOKIES))
-                        {
-                            reportComplianceViolation(INVALID_COOKIES, field);
-                            state = State.INVALID_COOKIE;
-                            continue;
-                        }
                         else
                         {
-                            throw new IllegalArgumentException("Invalid Cookie with attributes");
+                            cookieName = attributeName;
+                            cookieValue = value;
                         }
                         attributeName = null;
                     }
                     value = null;
 
-                    if (!knownAttribute)
-                    {
-                        if (!_complianceMode.allows(INVALID_COOKIES))
-                            throw new IllegalArgumentException("Invalid Cookie attribute");
-                        reportComplianceViolation(INVALID_COOKIES, field);
-                        state = State.INVALID_COOKIE;
-                        continue;
-                    }
-
                     if (state == State.END)
-                        throw new IllegalStateException("Invalid cookie");
+                        throw new InvalidCookieException("Invalid cookie");
                     break;
 
                 case INVALID_COOKIE:

jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserLenientTest.java

@@ -0,0 +1,137 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.http;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+/**
+ * Tests of poor various name=value scenarios and expectations of results
+ * due to our efforts at being lenient with what we receive.
+ */
+public class RFC6265CookieParserLenientTest
+{
+    public static Stream<Arguments> data()
+    {
+        return Stream.of(
+            // Simple test to verify behavior
+            Arguments.of("x=y", "x", "y"),
+            Arguments.of("key=value", "key", "value"),
+
+            // Tests that conform to RFC2109
+            // RFC2109 - token values
+            //   token          = 1*<any CHAR except CTLs or tspecials>
+            //   CHAR           = <any US-ASCII character (octets 0 - 127)>
+            //   CTL            = <any US-ASCII control character
+            //                    (octets 0 - 31) and DEL (127)>
+            //   SP             = <US-ASCII SP, space (32)>
+            //   HT             = <US-ASCII HT, horizontal-tab (9)>
+            //   tspecials      = "(" | ")" | "<" | ">" | "@"
+            //                  | "," | ";" | ":" | "\" | <">
+            //                  | "/" | "[" | "]" | "?" | "="
+            //                  | "{" | "}" | 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"),
+            // RFC2109 - quoted-string values
+            //   quoted-string  = ( <"> *(qdtext) <"> )
+            //   qdtext         = <any TEXT except <">>
+
+            // rejected, as name cannot be DQUOTED
+            Arguments.of("\"a\"=bcd", null, null),
+            Arguments.of("\"a\"=\"b c d\"", null, null),
+
+            // 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"),
+
+            // 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"),
+            // 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"),
+
+            // Tests that conform to RFC6265
+            Arguments.of("abc=foobar!", "abc", "foobar!"),
+            Arguments.of("abc=\"foobar!\"", "abc", "foobar!")
+
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("data")
+    public void testLenientBehavior(String rawHeader, String expectedName, String expectedValue)
+    {
+        TestCutter cutter = new TestCutter();
+        cutter.parseField(rawHeader);
+
+        if (expectedName == null)
+            assertThat("Cookies.length", cutter.names.size(), is(0));
+        else
+        {
+            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));
+        }
+    }
+
+    static class TestCutter implements CookieParser.Handler
+    {
+        CookieParser parser;
+        List<String> names = new ArrayList<>();
+        List<String> values = new ArrayList<>();
+
+        protected TestCutter()
+        {
+            parser = new RFC6265CookieParser(this, CookieCompliance.RFC6265, null);
+        }
+
+        public void parseField(String field)
+        {
+            parser.parseField(field);
+        }
+
+        @Override
+        public void addCookie(String cookieName, String cookieValue, int cookieVersion, String cookieDomain, String cookiePath, String cookieComment)
+        {
+            names.add(cookieName);
+            values.add(cookieValue);
+        }
+    }
+}

jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java

@@ -23,7 +23,6 @@ import org.junit.jupiter.params.provider.MethodSource;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class RFC6265CookieParserTest
 {
@@ -36,7 +35,7 @@ public class RFC6265CookieParserTest
         String rawCookie = "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"";
 
         // Test with RFC 2965.
-        TestCookieParser parser = new TestCookieParser(CookieCompliance.RFC2965_LEGACY);
+        TestCookieParser parser = new TestCookieParser(CookieCompliance.RFC2965);
         List<Cookie> cookies = parser.parseFields(rawCookie);
 
         assertThat("Cookies.length", cookies.size(), is(1));
@@ -44,18 +43,45 @@ public class RFC6265CookieParserTest
         // There are 2 attributes, so 2 violations.
         assertThat(parser.violations.size(), is(2));
 
-        // Same test with RFC 6265.
+        // Same test with RFC6265.
         parser = new TestCookieParser(CookieCompliance.RFC6265);
         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);
 
+        // There attributes are seen as just normal cookies, so no violations
+        assertThat(parser.violations.size(), is(0));
+
+        // Same again, but allow attributes which are ignored
+        parser = new TestCookieParser(CookieCompliance.from("RFC6265,ATTRIBUTES"));
+        cookies = parser.parseFields(rawCookie);
         assertThat("Cookies.length", cookies.size(), is(1));
         assertCookie("Cookies[0]", cookies.get(0), "Customer", "WILE_E_COYOTE", 0, null);
-        // There are 2 attributes, so 2 violations.
+
+        // There attributes are seen as just normal cookies, so no violations
+        assertThat(parser.violations.size(), is(2));
+
+        // Same again, but allow attributes which are not ignored
+        parser = new TestCookieParser(CookieCompliance.from("RFC6265,ATTRIBUTE_VALUES"));
+        cookies = parser.parseFields(rawCookie);
+        assertThat("Cookies.length", cookies.size(), is(1));
+        assertCookie("Cookies[0]", cookies.get(0), "Customer", "WILE_E_COYOTE", 1, "/acme");
+
+        // There attributes are seen as just normal cookies, so no violations
         assertThat(parser.violations.size(), is(2));
 
         // Same test with RFC 6265 strict should throw.
-        TestCookieParser strictParser = new TestCookieParser(CookieCompliance.RFC6265_STRICT);
+        parser = new TestCookieParser(CookieCompliance.RFC6265_STRICT);
-        assertThrows(IllegalArgumentException.class, () -> strictParser.parseFields(rawCookie));
+        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);
+
+        // There attributes are seen as just normal cookies, so no violations
+        assertThat(parser.violations.size(), is(0));
     }
 
     /**
@@ -68,11 +94,29 @@ public class RFC6265CookieParserTest
             "Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; " +
             "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"";
 
-        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie);
+        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
 
         assertThat("Cookies.length", cookies.length, is(2));
         assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme");
         assertCookie("Cookies[1]", cookies[1], "Part_Number", "Rocket_Launcher_0001", 1, "/acme");
+
+        cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie);
+        assertThat("Cookies.length", cookies.length, is(5));
+        assertCookie("Cookies[0]", cookies[0], "$Version", "1", 0, null);
+        assertCookie("Cookies[1]", cookies[1], "Customer", "WILE_E_COYOTE", 0, null);
+        assertCookie("Cookies[2]", cookies[2], "$Path", "/acme", 0, null);
+        assertCookie("Cookies[3]", cookies[3], "Part_Number", "Rocket_Launcher_0001", 0, null);
+        assertCookie("Cookies[4]", cookies[4], "$Path", "/acme", 0, null);
+
+        cookies = parseCookieHeaders(CookieCompliance.from("RFC6265,ATTRIBUTES"), rawCookie);
+        assertThat("Cookies.length", cookies.length, is(2));
+        assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 0, null);
+        assertCookie("Cookies[1]", cookies[1], "Part_Number", "Rocket_Launcher_0001", 0, null);
+
+        cookies = parseCookieHeaders(CookieCompliance.from("RFC6265,ATTRIBUTE_VALUES"), rawCookie);
+        assertThat("Cookies.length", cookies.length, is(2));
+        assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme");
+        assertCookie("Cookies[1]", cookies[1], "Part_Number", "Rocket_Launcher_0001", 1, "/acme");
     }
 
     /**
@@ -86,7 +130,7 @@ public class RFC6265CookieParserTest
             "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"; " +
             "Shipping=\"FedEx\"; $Path=\"/acme\"";
 
-        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie);
+        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
 
         assertThat("Cookies.length", cookies.length, is(3));
         assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme");
@@ -104,7 +148,7 @@ public class RFC6265CookieParserTest
             "Part_Number=\"Riding_Rocket_0023\"; $Path=\"/acme/ammo\"; " +
             "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"";
 
-        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie);
+        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
 
         assertThat("Cookies.length", cookies.length, is(2));
         assertCookie("Cookies[0]", cookies[0], "Part_Number", "Riding_Rocket_0023", 1, "/acme/ammo");
@@ -121,7 +165,7 @@ public class RFC6265CookieParserTest
             "session_id=\"1234\"; " +
             "session_id=\"1111\"; $Domain=\".cracker.edu\"";
 
-        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie);
+        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
 
         assertThat("Cookies.length", cookies.length, is(2));
         assertCookie("Cookies[0]", cookies[0], "session_id", "1234", 1, null);
@@ -137,15 +181,25 @@ public class RFC6265CookieParserTest
         String rawCookie = "$Version=\"1\"; session_id=\"1234\", " +
             "$Version=\"1\"; session_id=\"1111\"; $Domain=\".cracker.edu\"";
 
-        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie);
+        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
 
         assertThat("Cookies.length", cookies.length, is(2));
         assertCookie("Cookies[0]", cookies[0], "session_id", "1234", 1, null);
         assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 1, null);
 
         cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie);
-        assertThat("Cookies.length", cookies.length, is(1));
+        assertThat("Cookies.length", cookies.length, is(3));
-        assertCookie("Cookies[1]", cookies[0], "session_id", "1111", 0, null);
+        assertCookie("Cookies[0]", cookies[0], "$Version", "1", 0, null);
+        assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 0, null);
+        assertCookie("Cookies[2]", cookies[2], "$Domain", ".cracker.edu", 0, null);
+
+        cookies = parseCookieHeaders(CookieCompliance.from("RFC6265,COMMA_SEPARATOR"), rawCookie);
+        assertThat("Cookies.length", cookies.length, is(5));
+        assertCookie("Cookies[0]", cookies[0], "$Version", "1", 0, null);
+        assertCookie("Cookies[1]", cookies[1], "session_id", "1234", 0, null);
+        assertCookie("Cookies[3]", cookies[2], "$Version", "1", 0, null);
+        assertCookie("Cookies[3]", cookies[3], "session_id", "1111", 0, null);
+        assertCookie("Cookies[4]", cookies[4], "$Domain", ".cracker.edu", 0, null);
     }
 
     /**
@@ -201,7 +255,8 @@ public class RFC6265CookieParserTest
 
         Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie);
 
-        assertThat("Cookies.length", cookies.length, is(0));
+        assertThat("Cookies.length", cookies.length, is(1));
+        assertCookie("Cookies[0]", cookies[0], "$key", "value", 0, null);
     }
 
     @Test
@@ -236,7 +291,7 @@ public class RFC6265CookieParserTest
     public void testRFC2965QuotedEscape()
     {
         String rawCookie = "A=\"double\\\"quote\"";
-        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie);
+        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
 
         assertThat("Cookies.length", cookies.length, is(1));
         assertCookie("Cookies[0]", cookies[0], "A", "double\"quote", 0, null);
@@ -246,16 +301,12 @@ public class RFC6265CookieParserTest
     public void testRFC2965QuotedSpecial()
     {
         String rawCookie = "A=\", ;\"";
-        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie);
+        Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
 
         assertThat("Cookies.length", cookies.length, is(1));
         assertCookie("Cookies[0]", cookies[0], "A", ", ;", 0, null);
     }
 
-    // TODO:
-    //  $X; N=V
-    //  $X=Y; N=V
-
     public static List<Param> parameters()
     {
         return Arrays.asList(
@@ -281,8 +332,8 @@ public class RFC6265CookieParserTest
             new Param("A=\"1\u0007\"; B=2; C=3", "B=2", "C=3"),
             new Param("€"),
             new Param("@={}"),
-            new Param("$X=Y; N=V", "N=V"),
+            new Param("$X=Y; N=V", "$X=Y", "N=V"),
-            new Param("N=V; $X=Y", "N=V")
+            new Param("N=V; $X=Y", "N=V", "$X=Y")
         );
     }
 

jetty-server/src/main/config/etc/jetty.xml

@@ -73,8 +73,8 @@
       <Set name="persistentConnectionsEnabled" property="jetty.httpConfig.persistentConnectionsEnabled"/>
       <Set name="httpCompliance"><Call class="org.eclipse.jetty.http.HttpCompliance" name="from"><Arg><Property name="jetty.httpConfig.compliance" deprecated="jetty.http.compliance" default="RFC7230"/></Arg></Call></Set>
       <Set name="uriCompliance"><Call class="org.eclipse.jetty.http.UriCompliance" name="from"><Arg><Property name="jetty.httpConfig.uriCompliance" default="SAFE"/></Arg></Call></Set>
-      <Set name="requestCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.requestCookieCompliance" default="RFC6265"/></Arg></Call></Set>
+      <Set name="requestCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="from"><Arg><Property name="jetty.httpConfig.requestCookieCompliance" default="RFC6265"/></Arg></Call></Set>
-      <Set name="responseCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.responseCookieCompliance" default="RFC6265"/></Arg></Call></Set>
+      <Set name="responseCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="from"><Arg><Property name="jetty.httpConfig.responseCookieCompliance" default="RFC6265"/></Arg></Call></Set>
       <Set name="multiPartFormDataCompliance"><Call class="org.eclipse.jetty.server.MultiPartFormDataCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.multiPartFormDataCompliance" default="RFC7578"/></Arg></Call></Set>
       <Set name="relativeRedirectAllowed"><Property name="jetty.httpConfig.relativeRedirectAllowed" default="false"/></Set>
       <Set name="useInputDirectByteBuffers" property="jetty.httpConfig.useInputDirectByteBuffers"/>

jetty-server/src/main/config/modules/server.mod

@@ -75,7 +75,7 @@ etc/jetty.xml
 ## URI Compliance: DEFAULT, LEGACY, RFC3986, RFC3986_UNAMBIGUOUS, UNSAFE
 # jetty.httpConfig.uriCompliance=DEFAULT
 
-## Cookie compliance mode for parsing request Cookie headers: RFC2965, RFC6265
+## Cookie compliance mode for parsing request Cookie headers: RFC6265_STRICT, RFC6265, RFC6265_LEGACY, RFC2965, RFC2965_LEGACY
 # jetty.httpConfig.requestCookieCompliance=RFC6265
 
 ## Cookie compliance mode for generating response Set-Cookie: RFC2965, RFC6265

jetty-server/src/main/java/org/eclipse/jetty/server/Cookies.java

@@ -17,9 +17,11 @@ import java.util.ArrayList;
 import java.util.List;
 import javax.servlet.http.Cookie;
 
+import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.ComplianceViolation;
 import org.eclipse.jetty.http.CookieCompliance;
 import org.eclipse.jetty.http.CookieParser;
+import org.eclipse.jetty.http.HttpStatus;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -95,7 +97,14 @@ public class Cookies implements CookieParser.Handler
         if (_parsed)
             return _cookies;
 
-        _parser.parseFields(_rawFields);
+        try
+        {
+            _parser.parseFields(_rawFields);
+        }
+        catch (CookieParser.InvalidCookieException invalidCookieException)
+        {
+            throw new BadMessageException(HttpStatus.BAD_REQUEST_400, invalidCookieException.getMessage(), invalidCookieException);
+        }
         _cookies = (Cookie[])_cookieList.toArray(new Cookie[_cookieList.size()]);
         _cookieList.clear();
         _parsed = true;

jetty-server/src/test/java/org/eclipse/jetty/server/ServerHttpCookieTest.java

@@ -0,0 +1,212 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.server;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.stream.Stream;
+import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.http.CookieCompliance;
+import org.eclipse.jetty.http.CookieParser;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.eclipse.jetty.util.UrlEncoded;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.eclipse.jetty.http.CookieCompliance.RFC2965;
+import static org.eclipse.jetty.http.CookieCompliance.RFC2965_LEGACY;
+import static org.eclipse.jetty.http.CookieCompliance.RFC6265;
+import static org.eclipse.jetty.http.CookieCompliance.RFC6265_LEGACY;
+import static org.eclipse.jetty.http.CookieCompliance.RFC6265_STRICT;
+import static org.eclipse.jetty.http.CookieCompliance.from;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
+
+public class ServerHttpCookieTest
+{
+    private Server _server;
+    private LocalConnector _connector;
+    private HttpConfiguration _httpConfiguration;
+
+    @BeforeEach
+    public void beforeEach() throws Exception
+    {
+        _server = new Server();
+        _connector = new LocalConnector(_server);
+        _server.addConnector(_connector);
+        _server.setHandler(new AbstractHandler()
+        {
+            @Override
+            public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+            {
+                HttpConfiguration config = baseRequest.getHttpChannel().getHttpConfiguration();
+                baseRequest.setHandled(true);
+                String setCookie = baseRequest.getParameter("SetCookie");
+                if (setCookie != null)
+                {
+                    CookieParser parser = CookieParser.newParser((name, value, version, domain, path, comment) ->
+                    {
+                        Cookie cookie = new Cookie(name, value);
+                        if (version > 0)
+                            cookie.setVersion(version);
+                        if (domain != null)
+                            cookie.setDomain(domain);
+                        if (path != null)
+                            cookie.setPath(path);
+                        if (comment != null)
+                            cookie.setComment(comment);
+                        response.addCookie(cookie);
+                    }, RFC2965, null);
+                    parser.parseField(setCookie);
+                }
+
+                Cookie[] cookies = request.getCookies();
+                StringBuilder out = new StringBuilder();
+                if (cookies != null)
+                {
+                    for (Cookie cookie : cookies)
+                    {
+                        out
+                            .append("[")
+                            .append(cookie.getName())
+                            .append('=')
+                            .append(cookie.getValue());
+
+                        if (cookie.getVersion() > 0)
+                            out.append(";Version=").append(cookie.getVersion());
+                        if (cookie.getPath() != null)
+                            out.append(";Path=").append(cookie.getPath());
+                        if (cookie.getDomain() != null)
+                            out.append(";Domain=").append(cookie.getDomain());
+                        out.append("]\n");
+                    }
+                }
+                response.getWriter().println(out);
+            }
+        });
+        _httpConfiguration = _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration();
+        _server.start();
+    }
+
+    public static Stream<Arguments> requestCases()
+    {
+        return Stream.of(
+            Arguments.of(RFC6265_STRICT, "Cookie: name=value", 200, "Version=", List.of("[name=value]").toArray(new String[0])),
+
+
+            // Attribute tests
+            // TODO $name attributes are ignored because servlet 5.0 Cookie class rejects them.  They are not ignored in servlet 6.0
+            Arguments.of(RFC6265_STRICT, "Cookie:  $version=1; name=value", 200, "Version=", List.of("[name=value]").toArray(new String[0])),
+            Arguments.of(RFC6265, "Cookie: $version=1; name=value", 200, "Version=", List.of("[name=value]").toArray(new String[0])),
+            Arguments.of(RFC6265, "Cookie: name=value;$path=/path", 200, "Path=", List.of("[name=value]").toArray(new String[0])),
+            Arguments.of(from("RFC6265,ATTRIBUTES"), "Cookie: name=value;$path=/path", 200, "/path", List.of("name=value").toArray(new String[0])),
+            Arguments.of(from("RFC6265_STRICT,ATTRIBUTE_VALUES"), "Cookie: name=value;$path=/path", 200, null, List.of("name=value;Path=/path").toArray(new String[0])),
+            Arguments.of(RFC2965, "Cookie: name=value;$path=/path", 200, null, List.of("name=value;Path=/path").toArray(new String[0])),
+            Arguments.of(RFC2965, "Cookie: $Version=1;name=value;$path=/path", 200, null, List.of("name=value;Version=1;Path=/path").toArray(new String[0])),
+            Arguments.of(RFC2965, "Cookie: $Version=1;name=value;$path=/path;$Domain=host", 200, null, List.of("name=value;Version=1;Path=/path;Domain=host").toArray(new String[0])),
+
+            // multiple cookie tests
+            Arguments.of(RFC6265_STRICT, "Cookie: name=value; other=extra", 200, "Version=", List.of("[name=value]", "[other=extra]").toArray(new String[0])),
+            Arguments.of(RFC6265_STRICT, "Cookie: name=value, other=extra", 400, null, List.of("BadMessageException", "Comma cookie separator").toArray(new String[0])),
+            Arguments.of(from("RFC6265_STRICT,COMMA_SEPARATOR,"), "Cookie: name=value, other=extra", 200, "Version=", List.of("[name=value]", "[other=extra]").toArray(new String[0])),
+            Arguments.of(RFC6265, "Cookie: name=value, other=extra", 200, "name=value", null),
+            Arguments.of(RFC6265_LEGACY, "Cookie: name=value, other=extra", 200, null, List.of("[name=value, other=extra]").toArray(new String[0])),
+            Arguments.of(RFC6265_LEGACY, "Cookie: name=value; other=extra", 200, null, List.of("[name=value]", "other=extra]").toArray(new String[0])),
+            Arguments.of(RFC2965, "Cookie: name=value, other=extra", 200, "Version=", List.of("[name=value]", "[other=extra]").toArray(new String[0])),
+            Arguments.of(RFC2965_LEGACY, "Cookie: name=value, other=extra", 200, "Version=", List.of("[name=value]", "[other=extra]").toArray(new String[0])),
+
+            // white space
+            Arguments.of(RFC6265_STRICT, "Cookie: name =value", 400, null, List.of("BadMessageException", "Bad Cookie name").toArray(new String[0])),
+            Arguments.of(from("RFC6265,OPTIONAL_WHITE_SPACE"), "Cookie: name =value", 200, null, List.of("name=value").toArray(new String[0])),
+
+            // bad characters
+            Arguments.of(RFC6265_STRICT, "Cookie: name=va\\ue", 400, null, List.of("BadMessageException", "Bad Cookie value").toArray(new String[0])),
+            Arguments.of(RFC6265_STRICT, "Cookie: name=\"value\"", 200, "Version=", List.of("[name=value]").toArray(new String[0])),
+            Arguments.of(RFC6265_STRICT, "Cookie: name=\"value;other=extra\"", 400, null, List.of("BadMessageException", "Bad Cookie quoted value").toArray(new String[0])),
+            Arguments.of(RFC6265, "Cookie: name=\"value;other=extra\"", 200, "name=value", null),
+            Arguments.of(RFC6265_LEGACY, "Cookie: name=\"value;other=extra\"", 200, null, List.of("[name=value;other=extra]").toArray(new String[0])),
+            Arguments.of(RFC2965, "Cookie: name=\"value;other=extra\"", 200, null, List.of("[name=value;other=extra]").toArray(new String[0])),
+            Arguments.of(RFC2965, "Cookie: name=\"value;other=extra", 200, "name=value", null),
+            Arguments.of(RFC2965_LEGACY, "Cookie: name=\"value;other=extra\"", 200, null, List.of("[name=value;other=extra]").toArray(new String[0])),
+            Arguments.of(RFC2965_LEGACY, "Cookie: name=\"value;other=extra", 200, null, List.of("[name=\"value;other=extra]").toArray(new String[0])),
+
+            // TCK check
+            Arguments.of(RFC6265, "Cookie: $Version=1; name1=value1; $Domain=hostname; $Path=/servlet_jsh_cookie_web", 200, null, List.of("name1=value1").toArray(new String[0]))
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("requestCases")
+    public void testRequestCookies(CookieCompliance compliance, String cookie, int status, String unexpected, String... expectations) throws Exception
+    {
+        _httpConfiguration.setRequestCookieCompliance(compliance);
+
+        HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse("GET / HTTP/1.0\r\n" + cookie + "\r\n\r\n"));
+
+        assertThat(response.getStatus(), equalTo(status));
+
+        if (unexpected != null)
+            assertThat(response.getContent(), not(containsString(unexpected)));
+
+        if (expectations != null)
+            for (String expected : expectations)
+                assertThat(response.getContent(), containsString(expected));
+    }
+
+    public static Stream<Arguments> responseCases()
+    {
+        return Stream.of(
+            Arguments.of(RFC6265_STRICT, "name=value", "name=value"),
+            Arguments.of(RFC6265, "name=value", "name=value"),
+            Arguments.of(RFC6265_LEGACY, "name=value", "name=value"),
+            Arguments.of(RFC2965, "name=value", "name=value"),
+            Arguments.of(RFC2965_LEGACY, "name=value", "name=value"),
+
+            Arguments.of(RFC6265_STRICT, "name=value;$version=1;$path=/path;$domain=domain", "name=value; Path=/path; Domain=domain"),
+            Arguments.of(RFC6265, "name=value;$version=1;$path=/path;$domain=domain", "name=value; Path=/path; Domain=domain"),
+            Arguments.of(RFC6265_LEGACY, "name=value;$version=1;$path=/path;$domain=domain", "name=value; Path=/path; Domain=domain"),
+            Arguments.of(RFC2965, "name=value;$version=1;$path=/path;$domain=domain", "name=value;Version=1;Path=/path;Domain=domain"),
+            Arguments.of(RFC2965_LEGACY, "name=value;$version=1;$path=/path;$domain=domain", "name=value;Version=1;Path=/path;Domain=domain"),
+
+            Arguments.of(RFC6265, "name=value", "name=value")
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("responseCases")
+    public void testResponseCookies(CookieCompliance compliance, String cookie, String expected) throws Exception
+    {
+        _httpConfiguration.setResponseCookieCompliance(compliance);
+
+        HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse("GET /?SetCookie=" + UrlEncoded.encodeString(cookie) + " HTTP/1.0\r\n\r\n"));
+        assertThat(response.getStatus(), equalTo(200));
+
+        String setCookie = response.get(HttpHeader.SET_COOKIE);
+        if (expected == null)
+            assertThat(setCookie, nullValue());
+        else
+            assertThat(setCookie, equalTo(expected));
+    }
+}