Fixes #10219 - Review HTTP Cookie parsing (#10464)

* Added SetCookieParser interface and RFC6265SetCookieParser implementation to properly parse Set-Cookie values.
* Removed hacky implementation in HttpClient.
* Removed unused methods in HttpCookieUtils.
* Using SetCookieParser for the implementation of newPushBuilder in ee9,ee10.
* Reworked HttpCookieStore.Default implementation.
* Implemented properly cookie path resolution.
* Using URI.getRawPath() to resolve cookie paths.
* Removed secure vs. non-secure scheme distinction when storing cookies.
* Refactored common code in HttpCookieStore.Default to avoid duplications.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java

@@ -13,10 +13,6 @@
 
 package org.eclipse.jetty.client;
 
-import java.io.IOException;
-import java.net.CookieManager;
-import java.net.CookiePolicy;
-import java.net.CookieStore;
 import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.net.SocketAddress;
@@ -25,7 +21,6 @@ import java.nio.channels.SelectionKey;
 import java.nio.channels.SocketChannel;
 import java.time.Duration;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -50,6 +45,7 @@ import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpParser;
 import org.eclipse.jetty.http.HttpScheme;
+import org.eclipse.jetty.http.SetCookieParser;
 import org.eclipse.jetty.io.ArrayByteBufferPool;
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.ClientConnectionFactory;
@@ -113,6 +109,7 @@ public class HttpClient extends ContainerLifeCycle
 {
     public static final String USER_AGENT = "Jetty/" + Jetty.VERSION;
     private static final Logger LOG = LoggerFactory.getLogger(HttpClient.class);
+    private static final SetCookieParser COOKIE_PARSER = SetCookieParser.newInstance();
 
     private final ConcurrentMap<Origin, HttpDestination> destinations = new ConcurrentHashMap<>();
     private final ProtocolHandlers handlers = new ProtocolHandlers();
@@ -123,7 +120,6 @@ public class HttpClient extends ContainerLifeCycle
     private final ClientConnector connector;
     private AuthenticationStore authenticationStore = new HttpAuthenticationStore();
     private HttpCookieStore cookieStore;
-    private HttpCookieParser cookieParser;
     private SocketAddressResolver resolver;
     private HttpField agentField = new HttpField(HttpHeader.USER_AGENT, USER_AGENT);
     private boolean followRedirects = true;
@@ -221,7 +217,6 @@ public class HttpClient extends ContainerLifeCycle
 
         if (cookieStore == null)
             cookieStore = new HttpCookieStore.Default();
-        cookieParser = new HttpCookieParser();
 
         transport.setHttpClient(this);
 
@@ -284,17 +279,9 @@ public class HttpClient extends ContainerLifeCycle
 
     public void putCookie(URI uri, HttpField field)
     {
-        try
+        HttpCookie cookie = COOKIE_PARSER.parse(field.getValue());
-        {
+        if (cookie != null)
-            HttpCookie cookie = cookieParser.parse(uri, field);
+            cookieStore.add(uri, cookie);
-            if (cookie != null)
-                cookieStore.add(uri, cookie);
-        }
-        catch (IOException x)
-        {
-            if (LOG.isDebugEnabled())
-                LOG.debug("Unable to store cookies {} from {}", field, uri, x);
-        }
     }
 
     /**
@@ -1102,71 +1089,4 @@ public class HttpClient extends ContainerLifeCycle
             sslContextFactory = getSslContextFactory();
         return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory);
     }
-
-    private static class HttpCookieParser extends CookieManager
-    {
-        public HttpCookieParser()
-        {
-            super(new Store(), CookiePolicy.ACCEPT_ALL);
-        }
-
-        public HttpCookie parse(URI uri, HttpField field) throws IOException
-        {
-            // TODO: hacky implementation waiting for a real HttpCookie parser.
-            String value = field.getValue();
-            if (value == null)
-                return null;
-            Map<String, List<String>> header = new HashMap<>(1);
-            header.put(field.getHeader().asString(), List.of(value));
-            put(uri, header);
-            Store store = (Store)getCookieStore();
-            HttpCookie cookie = store.cookie;
-            store.cookie = null;
-            return cookie;
-        }
-
-        private static class Store implements CookieStore
-        {
-            private HttpCookie cookie;
-
-            @Override
-            public void add(URI uri, java.net.HttpCookie cookie)
-            {
-                String domain = cookie.getDomain();
-                if ("localhost.local".equals(domain))
-                    cookie.setDomain("localhost");
-                this.cookie = HttpCookie.from(cookie);
-            }
-
-            @Override
-            public List<java.net.HttpCookie> get(URI uri)
-            {
-                throw new UnsupportedOperationException();
-            }
-
-            @Override
-            public List<java.net.HttpCookie> getCookies()
-            {
-                throw new UnsupportedOperationException();
-            }
-
-            @Override
-            public List<URI> getURIs()
-            {
-                throw new UnsupportedOperationException();
-            }
-
-            @Override
-            public boolean remove(URI uri, java.net.HttpCookie cookie)
-            {
-                throw new UnsupportedOperationException();
-            }
-
-            @Override
-            public boolean removeAll()
-            {
-                throw new UnsupportedOperationException();
-            }
-        }
-    }
 }

jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpCookieTest.java

@@ -242,8 +242,8 @@ public class HttpCookieTest extends AbstractHttpClientServerTest
                     List<HttpCookie> cookies = Request.getCookies(request);
                     switch (target)
                     {
-                        case "/", "/foo", "/foobar" -> assertEquals(0, cookies.size(), target);
+                        case "/", "/foobar" -> assertEquals(0, cookies.size(), target);
-                        case "/foo/", "/foo/bar", "/foo/bar/baz" ->
+                        case "/foo", "/foo/", "/foo/bar", "/foo/bar/", "/foo/bar/baz" ->
                         {
                             assertEquals(1, cookies.size(), target);
                             HttpCookie cookie = cookies.get(0);
@@ -263,7 +263,63 @@ public class HttpCookieTest extends AbstractHttpClientServerTest
             .timeout(5, TimeUnit.SECONDS));
         assertEquals(HttpStatus.OK_200, response.getStatus());
 
-        Arrays.asList("/", "/foo", "/foo/", "/foobar", "/foo/bar", "/foo/bar/baz").forEach(path ->
+        Arrays.asList("/", "/foo", "/foo/", "/foobar", "/foo/bar", "/foo/bar/", "/foo/bar/baz").forEach(path ->
+        {
+            ContentResponse r = send(client.newRequest("localhost", connector.getLocalPort())
+                .scheme(scenario.getScheme())
+                .path(path)
+                .headers(headers -> headers.put(headerName, "1"))
+                .timeout(5, TimeUnit.SECONDS));
+            assertEquals(HttpStatus.OK_200, r.getStatus());
+        });
+    }
+
+    @ParameterizedTest
+    @ArgumentsSource(ScenarioProvider.class)
+    public void testSetCookieWithoutPathRequestURIWithTwoSegmentsEndingWithSlash(Scenario scenario) throws Exception
+    {
+        String headerName = "X-Request";
+        String cookieName = "a";
+        String cookieValue = "1";
+        start(scenario, new EmptyServerHandler()
+        {
+            @Override
+            protected void service(Request request, org.eclipse.jetty.server.Response response)
+            {
+                String target = Request.getPathInContext(request);
+                int r = (int)request.getHeaders().getLongField(headerName);
+                if ("/foo/bar/".equals(target) && r == 0)
+                {
+                    HttpCookie cookie = HttpCookie.from(cookieName, cookieValue);
+                    org.eclipse.jetty.server.Response.addCookie(response, cookie);
+                }
+                else
+                {
+                    List<HttpCookie> cookies = Request.getCookies(request);
+                    switch (target)
+                    {
+                        case "/", "/foo", "/foo/", "/foobar" -> assertEquals(0, cookies.size(), target);
+                        case "/foo/bar", "/foo/bar/", "/foo/bar/baz" ->
+                        {
+                            assertEquals(1, cookies.size(), target);
+                            HttpCookie cookie = cookies.get(0);
+                            assertEquals(cookieName, cookie.getName(), target);
+                            assertEquals(cookieValue, cookie.getValue(), target);
+                        }
+                        default -> fail("Unrecognized Target: " + target);
+                    }
+                }
+            }
+        });
+
+        ContentResponse response = send(client.newRequest("localhost", connector.getLocalPort())
+            .scheme(scenario.getScheme())
+            .path("/foo/bar/")
+            .headers(headers -> headers.put(headerName, "0"))
+            .timeout(5, TimeUnit.SECONDS));
+        assertEquals(HttpStatus.OK_200, response.getStatus());
+
+        Arrays.asList("/", "/foo", "/foo/", "/foobar", "/foo/bar", "/foo/bar/", "/foo/bar/baz").forEach(path ->
         {
             ContentResponse r = send(client.newRequest("localhost", connector.getLocalPort())
                 .scheme(scenario.getScheme())

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCache.java

@@ -56,9 +56,12 @@ public class CookieCache implements CookieParser.Handler
         else
         {
             Map<String, String> attributes = new HashMap<>();
-            attributes.put(HttpCookie.DOMAIN_ATTRIBUTE, cookieDomain);
+            if (!StringUtil.isEmpty(cookieDomain))
-            attributes.put(HttpCookie.PATH_ATTRIBUTE, cookiePath);
+                attributes.put(HttpCookie.DOMAIN_ATTRIBUTE, cookieDomain);
-            attributes.put(HttpCookie.COMMENT_ATTRIBUTE, cookieComment);
+            if (!StringUtil.isEmpty(cookiePath))
+                attributes.put(HttpCookie.PATH_ATTRIBUTE, cookiePath);
+            if (!StringUtil.isEmpty(cookieComment))
+                attributes.put(HttpCookie.COMMENT_ATTRIBUTE, cookieComment);
             _cookieList.add(HttpCookie.from(cookieName, cookieValue, cookieVersion, attributes));
         }
     }

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

@@ -174,7 +174,7 @@ public interface HttpCookie
 
         public Wrapper(HttpCookie wrapped)
         {
-            this.wrapped = wrapped;
+            this.wrapped = Objects.requireNonNull(wrapped);
         }
 
         public HttpCookie getWrapped()
@@ -534,10 +534,42 @@ public interface HttpCookie
 
         public Builder attribute(String name, String value)
         {
-            _attributes = lazyAttributePut(_attributes, name, value);
+            if (name == null)
+                return this;
+            // Sanity checks on the values, expensive but necessary to avoid to store garbage.
+            switch (name.toLowerCase(Locale.ENGLISH))
+            {
+                case "expires" -> expires(parseExpires(value));
+                case "httponly" ->
+                {
+                    if (!isTruthy(value))
+                        throw new IllegalArgumentException("Invalid HttpOnly attribute");
+                    httpOnly(true);
+                }
+                case "max-age" -> maxAge(Long.parseLong(value));
+                case "samesite" ->
+                {
+                    SameSite sameSite = SameSite.from(value);
+                    if (sameSite == null)
+                        throw new IllegalArgumentException("Invalid SameSite attribute");
+                    sameSite(sameSite);
+                }
+                case "secure" ->
+                {
+                    if (!isTruthy(value))
+                        throw new IllegalArgumentException("Invalid Secure attribute");
+                    secure(true);
+                }
+                default -> _attributes = lazyAttributePut(_attributes, name, value);
+            }
             return this;
         }
 
+        private boolean isTruthy(String value)
+        {
+            return value != null && (value.isEmpty() || "true".equalsIgnoreCase(value));
+        }
+
         public Builder comment(String comment)
         {
             _attributes = lazyAttributePut(_attributes, COMMENT_ATTRIBUTE, comment);
@@ -818,26 +850,19 @@ public interface HttpCookie
         return obj1.equalsIgnoreCase(obj2);
     }
 
-    /**
-     * <p>Formats this cookie into a string suitable to be used
-     * in {@code Cookie} or {@code Set-Cookie} headers.</p>
-     *
-     * @param httpCookie the cookie to format
-     * @return a header string representation of the cookie
-     */
     private static String asString(HttpCookie httpCookie)
     {
         StringBuilder builder = new StringBuilder();
         builder.append(httpCookie.getName()).append("=").append(httpCookie.getValue());
-        int version = httpCookie.getVersion();
+        Map<String, String> attributes = httpCookie.getAttributes();
-        if (version > 0)
+        if (!attributes.isEmpty())
-            builder.append(";Version=").append(version);
+        {
-        String domain = httpCookie.getDomain();
+            for (Map.Entry<String, String> entry : attributes.entrySet())
-        if (domain != null)
+            {
-            builder.append(";Domain=").append(domain);
+                builder.append("; ");
-        String path = httpCookie.getPath();
+                builder.append(entry.getKey()).append("=").append(entry.getValue());
-        if (path != null)
+            }
-            builder.append(";Path=").append(path);
+        }
         return builder.toString();
     }
 

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookieStore.java

@@ -23,8 +23,11 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
+import java.util.function.Predicate;
 
 import org.eclipse.jetty.util.NanoTime;
+import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.thread.AutoLock;
 
 /**
@@ -121,85 +124,106 @@ public interface HttpCookieStore
     public static class Default implements HttpCookieStore
     {
         private final AutoLock lock = new AutoLock();
-        private final Map<Key, List<HttpCookie>> cookies = new HashMap<>();
+        private final Map<String, List<StoredHttpCookie>> cookies = new HashMap<>();
 
         @Override
         public boolean add(URI uri, HttpCookie cookie)
         {
             // TODO: reject if cookie size is too big?
 
-            boolean secure = HttpScheme.isSecure(uri.getScheme());
+            String resolvedDomain = resolveDomain(uri, cookie);
-            // Do not accept a secure cookie sent over an insecure channel.
+            if (resolvedDomain == null)
-            if (cookie.isSecure() && !secure)
                 return false;
 
-            String cookieDomain = cookie.getDomain();
+            String resolvedPath = resolvePath(uri, cookie);
-            if (cookieDomain != null)
-            {
-                cookieDomain = cookieDomain.toLowerCase(Locale.ENGLISH);
-                if (cookieDomain.startsWith("."))
-                    cookieDomain = cookieDomain.substring(1);
-                // RFC 6265 section 4.1.2.3, ignore Domain if ends with ".".
-                if (cookieDomain.endsWith("."))
-                    cookieDomain = uri.getHost();
-                // Reject top-level domains.
-                // TODO: should also reject "top" domain such as co.uk, gov.au, etc.
-                if (!cookieDomain.contains("."))
-                {
-                    if (!cookieDomain.equals("localhost"))
-                        return false;
-                }
 
-                String domain = uri.getHost();
+            // Cookies are stored under their resolved domain, so that:
-                if (domain != null)
+            // - add(sub.example.com, cookie[Domain]=null) => key=sub.example.com
-                {
+            // - add(sub.example.com, cookie[Domain]=example.com) => key=example.com
-                    domain = domain.toLowerCase(Locale.ENGLISH);
-                    // If uri.host==foo.example.com, only accept
-                    // cookie.domain==(foo.example.com|example.com).
-                    if (!domain.endsWith(cookieDomain))
-                        return false;
-                    int beforeMatch = domain.length() - cookieDomain.length() - 1;
-                    if (beforeMatch >= 0 && domain.charAt(beforeMatch) != '.')
-                        return false;
-                }
-            }
-            else
-            {
-                // No explicit cookie domain, use the origin domain.
-                cookieDomain = uri.getHost();
-            }
-
-            // Cookies are stored under their domain, so that:
-            // - add(sub.example.com, cookie[Domain]=null) => Key[domain=sub.example.com]
-            // - add(sub.example.com, cookie[Domain]=example.com) => Key[domain=example.com]
             // This facilitates the matching algorithm.
-            Key key = new Key(secure, cookieDomain);
+            boolean[] added = new boolean[1];
-            boolean[] result = new boolean[]{true};
+            StoredHttpCookie storedCookie = new StoredHttpCookie(cookie, uri, resolvedDomain, resolvedPath);
             try (AutoLock ignored = lock.lock())
             {
+                String key = resolvedDomain.toLowerCase(Locale.ENGLISH);
                 cookies.compute(key, (k, v) ->
                 {
                     // RFC 6265, section 4.1.2.
                     // Evict an existing cookie with
                     // same name, domain and path.
                     if (v != null)
-                        v.remove(cookie);
+                        v.remove(storedCookie);
 
                     // Add only non-expired cookies.
                     if (cookie.isExpired())
-                    {
-                        result[0] = false;
                         return v == null || v.isEmpty() ? null : v;
-                    }
 
+                    added[0] = true;
                     if (v == null)
                         v = new ArrayList<>();
-                    v.add(new Cookie(cookie));
+                    v.add(storedCookie);
                     return v;
                 });
             }
 
-            return result[0];
+            return added[0];
+        }
+
+        private String resolveDomain(URI uri, HttpCookie cookie)
+        {
+            String uriDomain = uri.getHost();
+            if (uriDomain == null)
+                return null;
+
+            String cookieDomain = cookie.getDomain();
+            // No explicit cookie domain, use the origin domain.
+            if (cookieDomain == null)
+                return uriDomain;
+
+            String resolvedDomain = cookieDomain;
+            if (resolvedDomain.startsWith("."))
+                resolvedDomain = cookieDomain.substring(1);
+            // RFC 6265 section 4.1.2.3, ignore Domain if ends with ".".
+            if (resolvedDomain.endsWith("."))
+                resolvedDomain = uriDomain;
+            // Reject top-level domains.
+            // TODO: should also reject "top" domain such as co.uk, gov.au, etc.
+            if (!resolvedDomain.contains("."))
+            {
+                if (!resolvedDomain.equalsIgnoreCase("localhost"))
+                    return null;
+            }
+
+            // Reject if the resolved domain is not either
+            // the same or a parent domain of the URI domain.
+            if (!isSameOrSubDomain(uriDomain, resolvedDomain))
+                return null;
+
+            return resolvedDomain;
+        }
+
+        private String resolvePath(URI uri, HttpCookie cookie)
+        {
+            // RFC 6265, section 5.1.4 and 5.2.4.
+            // Note that cookies with the Path attribute different from the
+            // URI path are accepted, as specified in sections 8.5 and 8.6.
+            String resolvedPath = cookie.getPath();
+            if (resolvedPath == null || !resolvedPath.startsWith("/"))
+            {
+                String uriPath = uri.getRawPath();
+                if (StringUtil.isBlank(uriPath) || !uriPath.startsWith("/"))
+                {
+                    resolvedPath = "/";
+                }
+                else
+                {
+                    int lastSlash = uriPath.lastIndexOf('/');
+                    resolvedPath = uriPath.substring(0, lastSlash);
+                    if (resolvedPath.isEmpty())
+                        resolvedPath = "/";
+                }
+            }
+            return resolvedPath;
         }
 
         @Override
@@ -209,6 +233,8 @@ public interface HttpCookieStore
             {
                 return cookies.values().stream()
                     .flatMap(Collection::stream)
+                    .filter(Predicate.not(StoredHttpCookie::isExpired))
+                    .map(HttpCookie.class::cast)
                     .toList();
             }
         }
@@ -216,13 +242,17 @@ public interface HttpCookieStore
         @Override
         public List<HttpCookie> match(URI uri)
         {
-            List<HttpCookie> result = new ArrayList<>();
-            boolean secure = HttpScheme.isSecure(uri.getScheme());
             String uriDomain = uri.getHost();
-            String path = uri.getPath();
+            if (uriDomain == null)
-            if (path == null || path.trim().isEmpty())
+                return List.of();
+
+            String path = uri.getRawPath();
+            if (path == null || path.isBlank())
                 path = "/";
 
+            boolean secure = HttpScheme.isSecure(uri.getScheme());
+
+            List<HttpCookie> result = new ArrayList<>();
             try (AutoLock ignored = lock.lock())
             {
                 // Given the way cookies are stored in the Map, the matching
@@ -235,15 +265,14 @@ public interface HttpCookieStore
                 // - Key[domain=example.com]
                 // - chop domain to com
                 //   invalid domain, exit iteration.
-                String domain = uriDomain;
+                String domain = uriDomain.toLowerCase(Locale.ENGLISH);
-                while (true)
+                while (domain != null)
                 {
-                    Key key = new Key(secure, domain);
+                    List<StoredHttpCookie> stored = cookies.get(domain);
-                    List<HttpCookie> stored = cookies.get(key);
+                    Iterator<StoredHttpCookie> iterator = stored == null ? Collections.emptyIterator() : stored.iterator();
-                    Iterator<HttpCookie> iterator = stored == null ? Collections.emptyIterator() : stored.iterator();
                     while (iterator.hasNext())
                     {
-                        HttpCookie cookie = iterator.next();
+                        StoredHttpCookie cookie = iterator.next();
 
                         // Check and remove expired cookies.
                         if (cookie.isExpired())
@@ -257,24 +286,16 @@ public interface HttpCookieStore
                             continue;
 
                         // Match the domain.
-                        if (!domainMatches(uriDomain, key.domain, cookie.getDomain()))
+                        if (!domainMatches(uriDomain, cookie.domain, cookie.getWrapped().getDomain()))
                             continue;
 
                         // Match the path.
-                        if (!pathMatches(path, cookie.getPath()))
+                        if (!pathMatches(path, cookie.path))
                             continue;
 
                         result.add(cookie);
                     }
-
+                    domain = parentDomain(domain);
-                    int dot = domain.indexOf('.');
-                    if (dot < 0)
-                        break;
-                    // Remove one subdomain.
-                    domain = domain.substring(dot + 1);
-                    // Exit if the top-level domain was reached.
-                    if (domain.indexOf('.') < 0)
-                        break;
                 }
             }
 
@@ -283,64 +304,97 @@ public interface HttpCookieStore
 
         private static boolean domainMatches(String uriDomain, String domain, String cookieDomain)
         {
-            if (uriDomain == null)
+            // If the cookie has no domain, or ends with ".", it must only be sent to the origin domain.
-                return true;
-            if (domain != null)
-                domain = domain.toLowerCase(Locale.ENGLISH);
-            uriDomain = uriDomain.toLowerCase(Locale.ENGLISH);
-            if (cookieDomain != null)
-                cookieDomain = cookieDomain.toLowerCase(Locale.ENGLISH);
             if (cookieDomain == null || cookieDomain.endsWith("."))
-            {
+                return uriDomain.equalsIgnoreCase(domain);
-                // RFC 6265, section 4.1.2.3.
+            return isSameOrSubDomain(uriDomain, cookieDomain);
-                // No cookie domain -> cookie sent only to origin server.
-                return uriDomain.equals(domain);
-            }
-            if (cookieDomain.startsWith("."))
-                cookieDomain = cookieDomain.substring(1);
-            if (uriDomain.endsWith(cookieDomain))
-            {
-                // The domain is the same as, or a subdomain of, the cookie domain.
-                int beforeMatch = uriDomain.length() - cookieDomain.length() - 1;
-                // Domains are the same.
-                if (beforeMatch == -1)
-                    return true;
-                // Verify it is a proper subdomain such as bar.foo.com,
-                // not just a suffix of a domain such as bazfoo.com.
-                return uriDomain.charAt(beforeMatch) == '.';
-            }
-            return false;
         }
 
-        private static boolean pathMatches(String path, String cookiePath)
+        private static boolean isSameOrSubDomain(String subDomain, String domain)
+        {
+            int subDomainLength = subDomain.length();
+            int domainLength = domain.length();
+            // Case-insensitive version of subDomain.endsWith(domain).
+            if (!subDomain.regionMatches(true, subDomainLength - domainLength, domain, 0, domainLength))
+                return false;
+            // Make sure it is a subdomain.
+            int beforeMatch = subDomainLength - domainLength - 1;
+            // Domains are the same.
+            if (beforeMatch < 0)
+                return true;
+            // Verify it is a proper subdomain such as bar.foo.com,
+            // not just a suffix of a domain such as bazfoo.com.
+            return subDomain.charAt(beforeMatch) == '.';
+        }
+
+        private static boolean pathMatches(String uriPath, String cookiePath)
         {
             if (cookiePath == null)
                 return true;
             // RFC 6265, section 5.1.4, path matching algorithm.
-            if (path.equals(cookiePath))
+            if (uriPath.equals(cookiePath))
                 return true;
-            if (path.startsWith(cookiePath))
+            if (uriPath.startsWith(cookiePath))
-                return cookiePath.endsWith("/") || path.charAt(cookiePath.length()) == '/';
+                return cookiePath.endsWith("/") || uriPath.charAt(cookiePath.length()) == '/';
             return false;
         }
 
         @Override
         public boolean remove(URI uri, HttpCookie cookie)
         {
-            Key key = new Key(HttpScheme.isSecure(uri.getScheme()), uri.getHost());
+            String uriDomain = uri.getHost();
+            if (uriDomain == null)
+                return false;
+
+            String resolvedPath = resolvePath(uri, cookie);
+
+            boolean[] removed = new boolean[1];
             try (AutoLock ignored = lock.lock())
             {
-                boolean[] result = new boolean[1];
+                String domain = uriDomain.toLowerCase(Locale.ENGLISH);
-                cookies.compute(key, (k, v) ->
+                while (domain != null)
                 {
-                    if (v == null)
+                    cookies.compute(domain, (k, v) ->
-                        return null;
+                    {
-                    boolean removed = v.remove(cookie);
+                        if (v == null)
-                    result[0] = removed;
+                            return null;
-                    return v.isEmpty() ? null : v;
+
-                });
+                        Iterator<StoredHttpCookie> iterator = v.iterator();
-                return result[0];
+                        while (iterator.hasNext())
+                        {
+                            StoredHttpCookie storedCookie = iterator.next();
+                            if (uriDomain.equalsIgnoreCase(storedCookie.uri.getHost()))
+                            {
+                                if (storedCookie.path.equals(resolvedPath))
+                                {
+                                    if (storedCookie.getWrapped().getName().equals(cookie.getName()))
+                                    {
+                                        iterator.remove();
+                                        removed[0] = true;
+                                    }
+                                }
+                            }
+                        }
+
+                        return v.isEmpty() ? null : v;
+                    });
+                    domain = parentDomain(domain);
+                }
             }
+            return removed[0];
+        }
+
+        private String parentDomain(String domain)
+        {
+            int dot = domain.indexOf('.');
+            if (dot < 0)
+                return null;
+            // Remove one subdomain.
+            domain = domain.substring(dot + 1);
+            // Exit if the top-level domain was reached.
+            if (domain.indexOf('.') < 0)
+                return null;
+            return domain;
         }
 
         @Override
@@ -355,22 +409,19 @@ public interface HttpCookieStore
             }
         }
 
-        private record Key(boolean secure, String domain)
+        private static class StoredHttpCookie extends HttpCookie.Wrapper
-        {
-            private Key(boolean secure, String domain)
-            {
-                this.secure = secure;
-                this.domain = domain.toLowerCase(Locale.ENGLISH);
-            }
-        }
-
-        private static class Cookie extends HttpCookie.Wrapper
         {
             private final long creationNanoTime = NanoTime.now();
+            private final URI uri;
+            private final String domain;
+            private final String path;
 
-            public Cookie(HttpCookie wrapped)
+            private StoredHttpCookie(HttpCookie wrapped, URI uri, String domain, String path)
             {
                 super(wrapped);
+                this.uri = Objects.requireNonNull(uri);
+                this.domain = Objects.requireNonNull(domain);
+                this.path = Objects.requireNonNull(path);
             }
 
             @Override
@@ -382,6 +433,24 @@ public interface HttpCookieStore
                 Instant expires = getExpires();
                 return expires != null && Instant.now().isAfter(expires);
             }
+
+            @Override
+            public int hashCode()
+            {
+                return Objects.hash(getWrapped().getName(), domain.toLowerCase(Locale.ENGLISH), path);
+            }
+
+            @Override
+            public boolean equals(Object obj)
+            {
+                if (this == obj)
+                    return true;
+                if (!(obj instanceof StoredHttpCookie that))
+                    return false;
+                return getName().equals(that.getName()) &&
+                       domain.equalsIgnoreCase(that.domain) &&
+                       path.equals(that.path);
+            }
         }
     }
 }

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265SetCookieParser.java

@@ -0,0 +1,215 @@
+//
+// ========================================================================
+// 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 org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * <p>A parser for {@code Set-Cookie} header values following
+ * <a href="https://datatracker.ietf.org/doc/html/rfc6265">RFC 6265</a>.</p>
+ * <p>White spaces around cookie name and value, and around attribute
+ * name and value, are permitted but stripped.
+ * Cookie values and attribute values may be quoted with double-quotes.</p>
+ */
+public class RFC6265SetCookieParser implements SetCookieParser
+{
+    private static final Logger LOG = LoggerFactory.getLogger(RFC6265SetCookieParser.class);
+
+    @Override
+    public HttpCookie parse(String setCookieValue)
+    {
+        // Implementation of the algorithm from RFC 6265, section 5.2.
+
+        // Parser state.
+        State state = State.NAME;
+        String name = null;
+        boolean quoted = false;
+        HttpCookie.Builder cookie = null;
+        int offset = 0;
+        int length = setCookieValue.length();
+
+        // Parse.
+        for (int i = 0; i < length; ++i)
+        {
+            char ch = setCookieValue.charAt(i);
+            switch (state)
+            {
+                case NAME ->
+                {
+                    HttpTokens.Token token = HttpTokens.getToken(ch);
+                    if (token == null)
+                    {
+                        if (LOG.isDebugEnabled())
+                            LOG.debug("invalid character {} at index {} of {}", ch, i, setCookieValue);
+                        return null;
+                    }
+                    if (ch == '=')
+                    {
+                        name = setCookieValue.substring(offset, i).trim();
+                        if (name.isEmpty())
+                        {
+                            if (LOG.isDebugEnabled())
+                                LOG.debug("invalid empty cookie name at index {} of {}", i, setCookieValue);
+                            return null;
+                        }
+                        offset = i + 1;
+                        state = State.VALUE_START;
+                    }
+                }
+                case VALUE_START ->
+                {
+                    if (isWhitespace(ch))
+                        continue;
+                    if (ch == '"')
+                        quoted = true;
+                    else
+                        --i;
+                    offset = i + 1;
+                    state = State.VALUE;
+                }
+                case VALUE ->
+                {
+                    if (quoted && ch == '"')
+                    {
+                        quoted = false;
+                        String value = setCookieValue.substring(offset, i).trim();
+                        cookie = HttpCookie.build(name, value);
+                        offset = i + 1;
+                        state = State.ATTRIBUTE;
+                    }
+                    else
+                    {
+                        if (ch == ';')
+                        {
+                            String value = setCookieValue.substring(offset, i).trim();
+                            cookie = HttpCookie.build(name, value);
+                            offset = i + 1;
+                            state = State.ATTRIBUTE_NAME;
+                        }
+                    }
+                }
+                case ATTRIBUTE ->
+                {
+                    if (isWhitespace(ch))
+                        continue;
+                    if (ch != ';')
+                    {
+                        if (LOG.isDebugEnabled())
+                            LOG.debug("invalid character {} at index {} of {}", ch, i, setCookieValue);
+                        return null;
+                    }
+                    offset = i + 1;
+                    state = State.ATTRIBUTE_NAME;
+                }
+                case ATTRIBUTE_NAME ->
+                {
+                    HttpTokens.Token token = HttpTokens.getToken(ch);
+                    if (token == null || token.getType() == HttpTokens.Type.CNTL)
+                    {
+                        if (LOG.isDebugEnabled())
+                            LOG.debug("invalid character {} at index {} of {}", ch, i, setCookieValue);
+                        return null;
+                    }
+                    if (ch == '=')
+                    {
+                        name = setCookieValue.substring(offset, i).trim();
+                        offset = i + 1;
+                        state = State.ATTRIBUTE_VALUE_START;
+                    }
+                    else if (ch == ';')
+                    {
+                        name = setCookieValue.substring(offset, i).trim();
+                        if (!setAttribute(cookie, name, ""))
+                            return null;
+                        offset = i + 1;
+                        // Stay in the ATTRIBUTE_NAME state.
+                    }
+                }
+                case ATTRIBUTE_VALUE_START ->
+                {
+                    if (isWhitespace(ch))
+                        continue;
+                    if (ch == '"')
+                        quoted = true;
+                    else
+                        --i;
+                    offset = i + 1;
+                    state = State.ATTRIBUTE_VALUE;
+                }
+                case ATTRIBUTE_VALUE ->
+                {
+                    if (quoted && ch == '"')
+                    {
+                        quoted = false;
+                        String value = setCookieValue.substring(offset, i).trim();
+                        if (!setAttribute(cookie, name, value))
+                            return null;
+                        offset = i + 1;
+                        state = State.ATTRIBUTE;
+                    }
+                    else
+                    {
+                        if (ch == ';')
+                        {
+                            String value = setCookieValue.substring(offset, i).trim();
+                            if (!setAttribute(cookie, name, value))
+                                return null;
+                            offset = i + 1;
+                            state = State.ATTRIBUTE_NAME;
+                        }
+                    }
+                }
+                default -> throw new IllegalStateException("invalid state " + state);
+            }
+        }
+
+        return switch (state)
+        {
+            case NAME -> null;
+            case VALUE_START -> HttpCookie.from(name, "");
+            case VALUE -> HttpCookie.from(name, setCookieValue.substring(offset, length).trim());
+            case ATTRIBUTE -> cookie.build();
+            case ATTRIBUTE_NAME -> setAttribute(cookie, setCookieValue.substring(offset, length).trim(), "") ? cookie.build() : null;
+            case ATTRIBUTE_VALUE_START -> setAttribute(cookie, name, "") ? cookie.build() : null;
+            case ATTRIBUTE_VALUE -> setAttribute(cookie, name, setCookieValue.substring(offset, length).trim()) ? cookie.build() : null;
+        };
+    }
+
+    private static boolean isWhitespace(char ch)
+    {
+        // A little more forgiving than RFC 6265.
+        return ch == ' ' || ch == '\t';
+    }
+
+    private boolean setAttribute(HttpCookie.Builder cookie, String name, String value)
+    {
+        try
+        {
+            cookie.attribute(name, value);
+            return true;
+        }
+        catch (Throwable x)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("could not set attribute {}={}", name, value, x);
+            return false;
+        }
+    }
+
+    private enum State
+    {
+        NAME, VALUE_START, VALUE, ATTRIBUTE, ATTRIBUTE_NAME, ATTRIBUTE_VALUE_START, ATTRIBUTE_VALUE
+    }
+}

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/SetCookieParser.java

@@ -0,0 +1,42 @@
+//
+// ========================================================================
+// 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;
+
+/**
+ * <p>A parser for {@code Set-Cookie} header values.</p>
+ * <p>Differently from other HTTP headers, {@code Set-Cookie} cannot be multi-valued
+ * with values separated by commas, because cookies supports the {@code Expires}
+ * attribute whose value is an RFC 1123 date that contains a comma.</p>
+ */
+public interface SetCookieParser
+{
+    /**
+     * <p>Returns an {@link HttpCookie} obtained by parsing the given
+     * {@code Set-Cookie} value.</p>
+     * <p>Returns {@code null} if the {@code Set-Cookie} value cannot
+     * be parsed due to syntax errors.</p>
+     *
+     * @param setCookieValue the {@code Set-Cookie} value to parse
+     * @return the parse {@link HttpCookie} or {@code null}
+     */
+    HttpCookie parse(String setCookieValue);
+
+    /**
+     * @return a new instance of the default {@link SetCookieParser}
+     */
+    static SetCookieParser newInstance()
+    {
+        return new RFC6265SetCookieParser();
+    }
+}

jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieStoreTest.java

@@ -19,6 +19,8 @@ import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -26,6 +28,14 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class HttpCookieStoreTest
 {
+    @Test
+    public void testRejectCookieForNoDomain()
+    {
+        HttpCookieStore store = new HttpCookieStore.Default();
+        URI uri = URI.create("/path");
+        assertFalse(store.add(uri, HttpCookie.from("n", "v")));
+    }
+
     @Test
     public void testRejectCookieForTopDomain()
     {
@@ -60,6 +70,7 @@ public class HttpCookieStoreTest
         URI uri = URI.create("http://sub.example.com");
         assertTrue(store.add(uri, HttpCookie.build("n", "v").domain("sub.example.com").build()));
         assertTrue(store.add(uri, HttpCookie.build("n", "v").domain("example.com").build()));
+        assertEquals(2, store.all().size());
     }
 
     @Test
@@ -78,6 +89,7 @@ public class HttpCookieStoreTest
         assertTrue(store.add(uri, HttpCookie.from("n", "v1")));
         // Replace the cookie with another that has a different value.
         assertTrue(store.add(uri, HttpCookie.from("n", "v2")));
+        assertEquals(1, store.all().size());
         List<HttpCookie> matches = store.match(uri);
         assertEquals(1, matches.size());
         assertEquals("v2", matches.get(0).getValue());
@@ -92,6 +104,7 @@ public class HttpCookieStoreTest
         // Replace the cookie with another that has a different value.
         // Domain comparison must be case-insensitive.
         assertTrue(store.add(uri, HttpCookie.build("n", "v2").domain("EXAMPLE.COM").build()));
+        assertEquals(1, store.all().size());
         List<HttpCookie> matches = store.match(uri);
         assertEquals(1, matches.size());
         assertEquals("v2", matches.get(0).getValue());
@@ -106,13 +119,26 @@ public class HttpCookieStoreTest
         // Replace the cookie with another that has a different value.
         // Path comparison must be case-sensitive.
         assertTrue(store.add(uri, HttpCookie.build("n", "v2").path("/path").build()));
+        assertEquals(1, store.all().size());
         List<HttpCookie> matches = store.match(uri);
         assertEquals(1, matches.size());
         assertEquals("v2", matches.get(0).getValue());
         // Same path but different case should generate another cookie.
         assertTrue(store.add(uri, HttpCookie.build("n", "v3").path("/PATH").build()));
-        matches = store.all();
+        assertEquals(2, store.all().size());
-        assertEquals(2, matches.size());
+    }
+
+    @Test
+    public void testMatchNoDomain()
+    {
+        HttpCookieStore store = new HttpCookieStore.Default();
+        URI cookieURI = URI.create("http://example.com");
+        assertTrue(store.add(cookieURI, HttpCookie.from("n", "v1")));
+
+        // No domain, no match.
+        URI uri = URI.create("/path");
+        List<HttpCookie> matches = store.match(uri);
+        assertEquals(0, matches.size());
     }
 
     @Test
@@ -220,6 +246,46 @@ public class HttpCookieStoreTest
         assertEquals(2, matches.size());
     }
 
+    @Test
+    public void testMatchWithEscapedURIPath()
+    {
+        HttpCookieStore store = new HttpCookieStore.Default();
+        URI cookieURI = URI.create("http://example.com/foo%2Fbar/baz");
+        assertTrue(store.add(cookieURI, HttpCookie.build("n1", "v1").build()));
+
+        URI uri = URI.create("http://example.com/foo");
+        List<HttpCookie> matches = store.match(uri);
+        assertEquals(0, matches.size());
+
+        uri = URI.create("http://example.com/foo/");
+        matches = store.match(uri);
+        assertEquals(0, matches.size());
+
+        uri = URI.create("http://example.com/foo/bar");
+        matches = store.match(uri);
+        assertEquals(0, matches.size());
+
+        uri = URI.create("http://example.com/foo/bar/");
+        matches = store.match(uri);
+        assertEquals(0, matches.size());
+
+        uri = URI.create("http://example.com/foo/bar/baz");
+        matches = store.match(uri);
+        assertEquals(0, matches.size());
+
+        uri = URI.create("http://example.com/foo%2Fbar");
+        matches = store.match(uri);
+        assertEquals(1, matches.size());
+
+        uri = URI.create("http://example.com/foo%2Fbar/");
+        matches = store.match(uri);
+        assertEquals(1, matches.size());
+
+        uri = URI.create("http://example.com/foo%2Fbar/qux");
+        matches = store.match(uri);
+        assertEquals(1, matches.size());
+    }
+
     @Test
     public void testExpiredCookieDoesNotMatch() throws Exception
     {
@@ -237,14 +303,48 @@ public class HttpCookieStoreTest
     @Test
     public void testRemove()
     {
+        // Note that the URI path is "/path/", but the cookie path "/remove" is used.
         HttpCookieStore store = new HttpCookieStore.Default();
-        URI cookieURI = URI.create("http://example.com/path");
+        URI cookieURI = URI.create("http://example.com/path/");
-        assertTrue(store.add(cookieURI, HttpCookie.from("n1", "v1")));
+        assertTrue(store.add(cookieURI, HttpCookie.build("n1", "v1").path("/remove").build()));
+
+        // Path does not match.
+        assertFalse(store.remove(URI.create("http://example.com"), HttpCookie.from("n1", "v2")));
+        assertFalse(store.remove(URI.create("http://example.com/path/"), HttpCookie.from("n1", "v2")));
+        assertFalse(store.remove(URI.create("http://example.com"), HttpCookie.build("n1", "v2").path("/path").build()));
+        assertFalse(store.remove(URI.create("http://example.com/remove/"), HttpCookie.build("n1", "v2").path("/path").build()));
+
+        // Domain does not match.
+        assertFalse(store.remove(URI.create("http://domain.com/remove/"), HttpCookie.build("n1", "v2").build()));
 
-        URI removeURI = URI.create("http://example.com");
         // Cookie value should not matter.
-        assertTrue(store.remove(removeURI, HttpCookie.from("n1", "n2")));
+        // The URI path must be "/remove/" (end with slash) because URI paths
-        assertFalse(store.remove(removeURI, HttpCookie.from("n1", "n2")));
+        // are chopped to the parent directory while cookie paths are not chopped.
+        URI removeURI = URI.create("http://example.com/remove/");
+        assertTrue(store.remove(removeURI, HttpCookie.from("n1", "v2")));
+        // Try again, should not be there.
+        assertFalse(store.remove(removeURI, HttpCookie.from("n1", "v2")));
+    }
+
+    @Test
+    public void testRemoveWithSubDomains()
+    {
+        // Subdomains can set cookies on the parent domain.
+        HttpCookieStore store = new HttpCookieStore.Default();
+        URI cookieURI = URI.create("http://sub.example.com/path");
+        assertTrue(store.add(cookieURI, HttpCookie.build("n1", "v1").domain("example.com").build()));
+
+        // Cannot remove the cookie from the parent domain.
+        assertFalse(store.remove(URI.create("http://example.com/path"), HttpCookie.from("n1", "v2")));
+        assertFalse(store.remove(URI.create("http://example.com/path"), HttpCookie.build("n1", "v2").domain("example.com").build()));
+        assertFalse(store.remove(URI.create("http://example.com/path"), HttpCookie.build("n1", "v2").domain("sub.example.com").build()));
+
+        // Cannot remove the cookie from a sibling domain.
+        assertFalse(store.remove(URI.create("http://foo.example.com/path"), HttpCookie.from("n1", "v2")));
+        assertFalse(store.remove(URI.create("http://foo.example.com/path"), HttpCookie.build("n1", "v2").domain("sub.example.com").build()));
+
+        // Remove the cookie.
+        assertTrue(store.remove(cookieURI, HttpCookie.from("n1", "v2")));
     }
 
     @Test
@@ -252,17 +352,19 @@ public class HttpCookieStoreTest
     {
         HttpCookieStore store = new HttpCookieStore.Default();
         URI uri = URI.create("http://example.com");
-        // Dumb server sending a secure cookie on clear-text scheme.
+        // A secure cookie on clear-text scheme.
-        assertFalse(store.add(uri, HttpCookie.build("n1", "v1").secure(true).build()));
+        assertTrue(store.add(uri, HttpCookie.build("n1", "v1").secure(true).build()));
+
         URI secureURI = URI.create("https://example.com");
         assertTrue(store.add(secureURI, HttpCookie.build("n2", "v2").secure(true).build()));
         assertTrue(store.add(secureURI, HttpCookie.from("n3", "v3")));
 
         List<HttpCookie> matches = store.match(uri);
-        assertEquals(0, matches.size());
+        assertEquals(1, matches.size());
+        assertEquals("n3", matches.get(0).getName());
 
         matches = store.match(secureURI);
-        assertEquals(2, matches.size());
+        assertEquals(3, matches.size());
     }
 
     @Test
@@ -290,9 +392,51 @@ public class HttpCookieStoreTest
         cookieURI = URI.create("wss://example.com");
         assertTrue(store.add(cookieURI, HttpCookie.from("n2", "v2")));
 
+        // Cookie matching does not depend on the scheme,
+        // not even with regard to non-secure vs secure.
         matchURI = URI.create("https://example.com");
         matches = store.match(matchURI);
+        assertEquals(2, matches.size());
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"localhost.", "domain.com."})
+    public void testCookieDomainEndingWithDotIsIgnored(String cookieDomain)
+    {
+        HttpCookieStore store = new HttpCookieStore.Default();
+        URI cookieURI = URI.create("http://example.com");
+        assertTrue(store.add(cookieURI, HttpCookie.build("n1", "v1").domain(cookieDomain).build()));
+
+        List<HttpCookie> matches = store.match(cookieURI);
         assertEquals(1, matches.size());
-        assertEquals("n2", matches.get(0).getName());
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"localhost.", "domain.com."})
+    public void testAddWithURIDomainEndingWithDot(String uriDomain)
+    {
+        HttpCookieStore store = new HttpCookieStore.Default();
+        URI cookieURI = URI.create("http://" + uriDomain);
+        assertTrue(store.add(cookieURI, HttpCookie.from("n1", "v1")));
+
+        List<HttpCookie> matches = store.match(cookieURI);
+        assertEquals(1, matches.size());
+
+        cookieURI = URI.create("http://" + uriDomain.substring(0, uriDomain.length() - 1));
+        matches = store.match(cookieURI);
+        assertEquals(0, matches.size());
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"localhost.", "domain.com."})
+    public void testMatchWithURIDomainEndingWithDot(String uriDomain)
+    {
+        HttpCookieStore store = new HttpCookieStore.Default();
+        URI cookieURI = URI.create("http://" + uriDomain.substring(0, uriDomain.length() - 1));
+        assertTrue(store.add(cookieURI, HttpCookie.from("n1", "v1")));
+
+        cookieURI = URI.create("http://" + uriDomain);
+        List<HttpCookie> matches = store.match(cookieURI);
+        assertEquals(0, matches.size());
     }
 }

jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java

@@ -0,0 +1,108 @@
+//
+// ========================================================================
+// 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.time.Instant;
+import java.time.format.DateTimeParseException;
+import java.util.List;
+
+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;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class HttpCookieTest
+{
+    public static List<Arguments> cookies()
+    {
+        return List.of(
+            Arguments.of("=", null),
+            Arguments.of("=B", null),
+            Arguments.of("=B; a", null),
+            Arguments.of("=B; a=", null),
+            Arguments.of("=B; a=v", null),
+            Arguments.of("A", null),
+            Arguments.of("A=", HttpCookie.build("A", "").build()),
+            Arguments.of("A=; HttpOnly", HttpCookie.build("A", "").httpOnly(true).build()),
+            Arguments.of("A=; a", HttpCookie.build("A", "").attribute("a", "").build()),
+            Arguments.of("A=; a=", HttpCookie.build("A", "").attribute("a", "").build()),
+            Arguments.of("A=; a=v", HttpCookie.build("A", "").attribute("a", "v").build()),
+            Arguments.of("A=B", HttpCookie.build("A", "B").build()),
+            Arguments.of("A= B", HttpCookie.build("A", "B").build()),
+            Arguments.of("A =B", HttpCookie.build("A", "B").build()),
+            Arguments.of(" A=B", HttpCookie.build("A", "B").build()),
+            Arguments.of(" A= B", HttpCookie.build("A", "B").build()),
+            Arguments.of("A=B; Secure", HttpCookie.build("A", "B").secure(true).build()),
+            Arguments.of("A=B; Expires=Thu, 01 Jan 1970 00:00:00 GMT", HttpCookie.build("A", "B").expires(Instant.EPOCH).build()),
+            Arguments.of("A=B; a", HttpCookie.build("A", "B").attribute("a", "").build()),
+            Arguments.of("A=B; a=", HttpCookie.build("A", "B").attribute("a", "").build()),
+            Arguments.of("A=B; a=v", HttpCookie.build("A", "B").attribute("a", "v").build()),
+            Arguments.of("A=B; Secure; Path=/", HttpCookie.build("A", "B").secure(true).path("/").build()),
+            // Quoted cookie.
+            Arguments.of("A=\"1\"", HttpCookie.build("A", "1").build()),
+            Arguments.of("A=\"1\"; HttpOnly", HttpCookie.build("A", "1").httpOnly(true).build()),
+            Arguments.of(" A = \"1\" ; a = v", HttpCookie.build("A", "1").attribute("a", "v").build()),
+            Arguments.of(" A = \"1\" ; a = \"v\"; Secure", HttpCookie.build("A", "1").attribute("a", "v").secure(true).build()),
+            Arguments.of(" A = \"1\" ; Path= \"/\"", HttpCookie.build("A", "1").path("/").build()),
+            Arguments.of(" A = \"1\" ; Expires= \"Thu, 01 Jan 1970 00:00:00 GMT\"", HttpCookie.build("A", "1").expires(Instant.EPOCH).build()),
+            // Invalid cookie.
+            Arguments.of("A=\"1\" Bad", null),
+            Arguments.of("A=1; Expires=blah", null),
+            Arguments.of("A=1; Expires=blah; HttpOnly", null),
+            Arguments.of("A=1; HttpOnly=blah", null),
+            Arguments.of("A=1; Max-Age=blah", null),
+            Arguments.of("A=1; SameSite=blah", null),
+            Arguments.of("A=1; SameSite=blah; Secure", null),
+            Arguments.of("A=1; Secure=blah", null),
+            Arguments.of("A=1; Max-Age=\"blah\"", null),
+            // Weird cookie.
+            Arguments.of("A=1; Domain=example.org; Domain=domain.com", HttpCookie.build("A", "1").domain("domain.com").build()),
+            Arguments.of("A=1; Path=/; Path=/ctx", HttpCookie.build("A", "1").path("/ctx").build())
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("cookies")
+    public void testParseCookies(String setCookieValue, HttpCookie expectedCookie)
+    {
+        SetCookieParser parser = SetCookieParser.newInstance();
+        HttpCookie parsed = parser.parse(setCookieValue);
+        assertEquals(expectedCookie, parsed);
+        if (expectedCookie != null)
+            assertThat(expectedCookie.getAttributes(), is(parsed.getAttributes()));
+    }
+
+    public static List<Arguments> invalidAttributes()
+    {
+        return List.of(
+            Arguments.of("Expires", "blah", DateTimeParseException.class),
+            Arguments.of("HttpOnly", "blah", IllegalArgumentException.class),
+            Arguments.of("Max-Age", "blah", NumberFormatException.class),
+            Arguments.of("SameSite", "blah", IllegalArgumentException.class),
+            Arguments.of("Secure", "blah", IllegalArgumentException.class)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("invalidAttributes")
+    public void testParseInvalidAttributes(String name, String value, Class<? extends Throwable> failure)
+    {
+        assertThrows(failure, () -> HttpCookie.build("A", "1")
+            .attribute(name, value));
+    }
+}

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpCookieUtils.java

@@ -14,18 +14,14 @@
 package org.eclipse.jetty.server;
 
 import java.time.Instant;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.jetty.http.CookieCompliance;
 import org.eclipse.jetty.http.HttpCookie;
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpHeader;
-import org.eclipse.jetty.http.QuotedCSVParser;
 import org.eclipse.jetty.http.Syntax;
 import org.eclipse.jetty.util.Attributes;
 import org.eclipse.jetty.util.Index;
@@ -75,35 +71,6 @@ public final class HttpCookieUtils
         return HttpCookie.from(cookie, HttpCookie.SAME_SITE_ATTRIBUTE, contextDefault.getAttributeValue());
     }
 
-    /**
-     * Extract the bare minimum of info from a Set-Cookie header string.
-     *
-     * <p>
-     * Ideally this method should not be necessary, however as java.net.HttpCookie
-     * does not yet support generic attributes, we have to use it in a minimal
-     * fashion. When it supports attributes, we could look at reverting to a
-     * constructor on o.e.j.h.HttpCookie to take the set-cookie header string.
-     * </p>
-     *
-     * @param setCookieHeader the header as a string
-     * @return a map containing the name, value, domain, path. max-age of the set cookie header
-     */
-    public static Map<String, String> extractBasics(String setCookieHeader)
-    {
-        //Parse the bare minimum
-        List<java.net.HttpCookie> cookies = java.net.HttpCookie.parse(setCookieHeader);
-        if (cookies.size() != 1)
-            return Collections.emptyMap();
-        java.net.HttpCookie cookie = cookies.get(0);
-        Map<String, String> fields = new HashMap<>();
-        fields.put("name", cookie.getName());
-        fields.put("value", cookie.getValue());
-        fields.put("domain", cookie.getDomain());
-        fields.put("path",  cookie.getPath());
-        fields.put("max-age", Long.toString(cookie.getMaxAge()));
-        return fields;
-    }
-
     /**
      * Get the default value for SameSite cookie attribute, if one
      * has been set for the given context.
@@ -406,52 +373,6 @@ public final class HttpCookieUtils
         return oldPath.equals(newPath);
     }
 
-    /**
-     * Get a {@link HttpHeader#SET_COOKIE} field as a {@link HttpCookie}, either
-     * by optimally checking for a {@link SetCookieHttpField} or by parsing
-     * the value with {@link #parseSetCookie(String)}.
-     * @param field The field
-     * @return The field value as a {@link HttpCookie} or null if the field
-     *         is not a {@link HttpHeader#SET_COOKIE} or cannot be parsed.
-     */
-    public static HttpCookie getSetCookie(HttpField field)
-    {
-        if (field == null || field.getHeader() != HttpHeader.SET_COOKIE)
-            return null;
-        if (field instanceof SetCookieHttpField setCookieHttpField)
-            return setCookieHttpField.getHttpCookie();
-        return parseSetCookie(field.getValue());
-    }
-
-    public static HttpCookie parseSetCookie(String value)
-    {
-        AtomicReference<HttpCookie.Builder> builder = new AtomicReference<>();
-        new QuotedCSVParser(false)
-        {
-            @Override
-            protected void parsedParam(StringBuffer buffer, int valueLength, int paramName, int paramValue)
-            {
-                String name = buffer.substring(paramName, paramValue - 1);
-                String value = buffer.substring(paramValue);
-                HttpCookie.Builder b = builder.get();
-                if (b == null)
-                {
-                    b = HttpCookie.build(name, value);
-                    builder.set(b);
-                }
-                else
-                {
-                    b.attribute(name, value);
-                }
-            }
-        }.addValue(value);
-
-        HttpCookie.Builder b = builder.get();
-        if (b == null)
-            return null;
-        return b.build();
-    }
-
     private static void quoteIfNeededAndAppend(String text, StringBuilder builder)
     {
         if (isQuoteNeeded(text))

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java

@@ -23,6 +23,7 @@ import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.http.SetCookieParser;
 import org.eclipse.jetty.io.Content;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.component.LifeCycle;
@@ -396,9 +397,7 @@ public class ResponseTest
                             if (field.getHeader() != HttpHeader.SET_COOKIE)
                                 continue;
 
-                            HttpCookie cookie = HttpCookieUtils.getSetCookie(field);
+                            HttpCookie cookie = SetCookieParser.newInstance().parse(field.getValue());
-                            if (cookie == null)
-                                continue;
 
                             i.set(new HttpCookieUtils.SetCookieHttpField(
                                 HttpCookie.build(cookie)
@@ -411,7 +410,7 @@ public class ResponseTest
                 });
                 response.setStatus(200);
                 Response.addCookie(response, HttpCookie.from("name", "test1"));
-                response.getHeaders().add(HttpHeader.SET_COOKIE, "other=test2; Domain=wrong; SameSite=wrong; Attr=x");
+                response.getHeaders().add(HttpHeader.SET_COOKIE, "other=test2; Domain=original; SameSite=None; Attr=x");
                 Content.Sink.write(response, true, "OK", callback);
                 return true;
             }

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

@@ -96,10 +96,10 @@ public class ServerHttpCookieTest
             Arguments.of(RFC6265, "Cookie: $version=1; name=value", 200, "Version=", List.of("[$version=1]", "[name=value]").toArray(new String[0])),
             Arguments.of(RFC6265, "Cookie: name=value;$path=/path", 200, "Path=", List.of("[name=value]", "[$path=/path]").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(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: 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", 200, null, List.of("name=value; 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;Domain=host;Path=/path").toArray(new String[0])),
+            Arguments.of(RFC2965, "Cookie: $Version=1;name=value;$path=/path;$Domain=host", 200, null, List.of("name=value; Domain=host; Path=/path").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])),

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java

@@ -22,7 +22,6 @@ import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.security.Principal;
 import java.util.AbstractList;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -69,6 +68,7 @@ import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MimeTypes;
+import org.eclipse.jetty.http.SetCookieParser;
 import org.eclipse.jetty.io.QuietException;
 import org.eclipse.jetty.io.RuntimeIOException;
 import org.eclipse.jetty.security.AuthenticationState;
@@ -98,6 +98,8 @@ import org.slf4j.LoggerFactory;
 public class ServletApiRequest implements HttpServletRequest
 {
     private static final Logger LOG = LoggerFactory.getLogger(ServletApiRequest.class);
+    private static final SetCookieParser SET_COOKIE_PARSER = SetCookieParser.newInstance();
+
     private final ServletContextRequest _servletContextRequest;
     private final ServletChannel _servletChannel;
     private AsyncContextState _async;
@@ -615,32 +617,37 @@ public class ServletApiRequest implements HttpServletRequest
             referrer += "?" + query;
         pushHeaders.put(HttpHeader.REFERER, referrer);
 
-        // Any Set-Cookie in the response should be present in the push.
+        StringBuilder cookieBuilder = new StringBuilder();
-        HttpFields.Mutable responseHeaders = _servletChannel.getResponse().getHeaders();
+        Cookie[] cookies = getCookies();
-        List<String> setCookies = new ArrayList<>(responseHeaders.getValuesList(HttpHeader.SET_COOKIE));
+        if (cookies != null)
-        setCookies.addAll(responseHeaders.getValuesList(HttpHeader.SET_COOKIE2));
-        String cookies = pushHeaders.get(HttpHeader.COOKIE);
-        if (!setCookies.isEmpty())
         {
-            StringBuilder pushCookies = new StringBuilder();
+            for (Cookie cookie : cookies)
-            if (cookies != null)
-                pushCookies.append(cookies);
-            for (String setCookie : setCookies)
             {
-                Map<String, String> cookieFields = HttpCookieUtils.extractBasics(setCookie);
+                if (!cookieBuilder.isEmpty())
-                String cookieName = cookieFields.get("name");
+                    cookieBuilder.append("; ");
-                String cookieValue = cookieFields.get("value");
+                cookieBuilder.append(cookie.getName()).append("=").append(cookie.getValue());
-                String cookieMaxAge = cookieFields.get("max-age");
-                long maxAge = cookieMaxAge != null ? Long.parseLong(cookieMaxAge) : -1;
-                if (maxAge > 0)
-                {
-                    if (pushCookies.length() > 0)
-                        pushCookies.append("; ");
-                    pushCookies.append(cookieName).append("=").append(cookieValue);
-                }
             }
-            pushHeaders.put(HttpHeader.COOKIE, pushCookies.toString());
         }
+        // Any Set-Cookie in the response should be present in the push.
+        for (HttpField field : _servletContextRequest.getServletContextResponse().getHeaders())
+        {
+            HttpHeader header = field.getHeader();
+            if (header == HttpHeader.SET_COOKIE || header == HttpHeader.SET_COOKIE2)
+            {
+                HttpCookie httpCookie;
+                if (field instanceof HttpCookieUtils.SetCookieHttpField set)
+                    httpCookie = set.getHttpCookie();
+                else
+                    httpCookie = SET_COOKIE_PARSER.parse(field.getValue());
+                if (httpCookie == null || httpCookie.isExpired())
+                    continue;
+                if (!cookieBuilder.isEmpty())
+                    cookieBuilder.append("; ");
+                cookieBuilder.append(httpCookie.getName()).append("=").append(httpCookie.getValue());
+            }
+        }
+        if (!cookieBuilder.isEmpty())
+            pushHeaders.put(HttpHeader.COOKIE, cookieBuilder.toString());
 
         String sessionId;
         HttpSession httpSession = getSession(false);

jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java

@@ -25,7 +25,6 @@ import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.Map;
 import java.util.function.Consumer;
 import java.util.function.Function;
 
@@ -60,7 +59,6 @@ import org.eclipse.jetty.session.SessionDataStoreFactory;
 import org.eclipse.jetty.toolchain.test.IO;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
-import org.eclipse.jetty.util.URIUtil;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
@@ -413,7 +411,7 @@ public class SessionHandlerTest
             assertThat(response.getContentAsString(), containsString("valid=false"));
 
             //test with a cookie for non-existant session
-            URI uri = URIUtil.toURI(URIUtil.newURI("http", "localhost", port, path, ""));
+            URI uri = URI.create(url);
             HttpCookie cookie = HttpCookie.build(SessionHandler.__DefaultSessionCookie, "123456789").path("/").domain("localhost").build();
             client.getHttpCookieStore().add(uri, cookie);
             response = client.GET(url);

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java

@@ -79,11 +79,11 @@ import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.http.MimeTypes;
+import org.eclipse.jetty.http.SetCookieParser;
 import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.io.RuntimeIOException;
 import org.eclipse.jetty.security.UserIdentity;
 import org.eclipse.jetty.server.HttpCookieUtils;
-import org.eclipse.jetty.server.HttpCookieUtils.SetCookieHttpField;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.Session;
 import org.eclipse.jetty.session.AbstractSessionManager;
@@ -107,11 +107,11 @@ public class Request implements HttpServletRequest
     public static final String __MULTIPART_CONFIG_ELEMENT = "org.eclipse.jetty.multipartConfig";
 
     private static final Logger LOG = LoggerFactory.getLogger(Request.class);
+    private static final SetCookieParser SET_COOKIE_PARSER = SetCookieParser.newInstance();
     private static final Collection<Locale> __defaultLocale = Collections.singleton(Locale.getDefault());
     private static final int INPUT_NONE = 0;
     private static final int INPUT_STREAM = 1;
     private static final int INPUT_READER = 2;
-
     private static final MultiMap<String> NO_PARAMS = new MultiMap<>();
     private static final MultiMap<String> BAD_PARAMS = new MultiMap<>();
 
@@ -289,60 +289,37 @@ public class Request implements HttpServletRequest
             id = getRequestedSessionId();
         }
 
-        Map<String, String> cookies = new HashMap<>();
+        StringBuilder cookieBuilder = new StringBuilder();
-        Cookie[] existingCookies = getCookies();
+        Cookie[] cookies = getCookies();
-        if (existingCookies != null)
+        if (cookies != null)
         {
-            for (Cookie c : getCookies())
+            for (Cookie cookie : cookies)
             {
-                cookies.put(c.getName(), c.getValue());
+                if (!cookieBuilder.isEmpty())
+                    cookieBuilder.append("; ");
+                cookieBuilder.append(cookie.getName()).append("=").append(cookie.getValue());
             }
         }
-
+        // Any Set-Cookie in the response should be present in the push.
-        //Any Set-Cookies that were set on the response must be set as Cookies on the
+        for (HttpField field : getResponse().getHttpFields())
-        //PushBuilder, unless the max-age of the cookie is <= 0
-        HttpFields responseFields = getResponse().getHttpFields();
-        for (HttpField field : responseFields)
         {
             HttpHeader header = field.getHeader();
-            if (header == HttpHeader.SET_COOKIE)
+            if (header == HttpHeader.SET_COOKIE || header == HttpHeader.SET_COOKIE2)
             {
-                String cookieName;
+                HttpCookie httpCookie;
-                String cookieValue;
+                if (field instanceof HttpCookieUtils.SetCookieHttpField set)
-                long cookieMaxAge;
+                    httpCookie = set.getHttpCookie();
-                if (field instanceof SetCookieHttpField)
-                {
-                    HttpCookie cookie =  ((SetCookieHttpField)field).getHttpCookie();
-                    cookieName = cookie.getName();
-                    cookieValue = cookie.getValue();
-                    cookieMaxAge = cookie.getMaxAge();
-                }
                 else
-                {
+                    httpCookie = SET_COOKIE_PARSER.parse(field.getValue());
-                    Map<String, String> cookieFields = HttpCookieUtils.extractBasics(field.getValue());
+                if (httpCookie == null || httpCookie.isExpired())
-                    cookieName = cookieFields.get("name");
+                    continue;
-                    cookieValue = cookieFields.get("value");
+                if (!cookieBuilder.isEmpty())
-                    cookieMaxAge = cookieFields.get("max-age") != null ? Long.parseLong(cookieFields.get("max-age")) : -1;
+                    cookieBuilder.append("; ");
-                }
+                cookieBuilder.append(httpCookie.getName()).append("=").append(httpCookie.getValue());
-
-                if (cookieMaxAge > 0)
-                    cookies.put(cookieName, cookieValue);
-                else
-                    cookies.remove(cookieName);
             }
         }
-
+        if (!cookieBuilder.isEmpty())
-        if (!cookies.isEmpty())
+            fields.put(HttpHeader.COOKIE, cookieBuilder.toString());
-        {
-            StringBuilder buff = new StringBuilder();
-            for (Map.Entry<String, String> entry : cookies.entrySet())
-            {
-                if (buff.length() > 0)
-                    buff.append("; ");
-                buff.append(entry.getKey()).append('=').append(entry.getValue());
-            }
-            fields.add(new HttpField(HttpHeader.COOKIE, buff.toString()));
-        }
 
         String query = getQueryString();
         PushBuilder builder = new PushBuilderImpl(this, fields, getMethod(), query, id);