Improves handling of cookies in server responses (helidon-io#9751)

Improves handling of cookies in server responses. A cookie's identity is defined by its name, domain and path, not just its name. A new method in ServerResponseHeaders enables clearing a cookie by passing all three values (a complete cookie). The equals method in SetCookie has now been overridden as well. Adds additional checks during cookie parsing. New tests.

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

http/http/src/main/java/io/helidon/http/ServerResponseHeaders.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -106,12 +106,24 @@ default ServerResponseHeaders addCookie(String name, String value) {
 
     /**
      * Clears a cookie by adding a {@code Set-Cookie} header with an expiration date in the past.
+     * It is recommended to use {@link #clearCookie(SetCookie)} instead for better handling
+     * of Path and Domain.
      *
      * @param name name of the cookie.
      * @return this instance
      */
     ServerResponseHeaders clearCookie(String name);
 
+    /**
+     * Clears a cookie by adding a {@code Set-Cookie} header with an expiration date in the past.
+     *
+     * @param setCookie the cookie.
+     * @return this instance
+     */
+    default ServerResponseHeaders clearCookie(SetCookie setCookie) {
+        return clearCookie(setCookie.name());
+    }
+
     /**
      * Sets the value of {@link HeaderNames#LAST_MODIFIED} header.
      * <p>

http/http/src/main/java/io/helidon/http/ServerResponseHeadersImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import java.util.List;
+import java.util.function.Predicate;
 
 import io.helidon.common.LazyValue;
 
@@ -39,22 +40,35 @@ public ServerResponseHeaders addCookie(SetCookie cookie) {
         return this;
     }
 
+    @Override
+    public ServerResponseHeaders clearCookie(SetCookie cookie) {
+        clearCookie(cookie, cookie::equals);
+        return this;
+    }
+
     @Override
     public ServerResponseHeaders clearCookie(String name) {
-        SetCookie expiredCookie = SetCookie.builder(name, "deleted")
-                .path("/")
+        clearCookie(SetCookie.builder(name, "").build(), c -> c.name().equals(name));
+        return this;
+    }
+
+    private void clearCookie(SetCookie cookie, Predicate<SetCookie> predicate) {
+        // expiredCookie same as cookie but with different expiration
+        SetCookie expiredCookie = SetCookie.builder(cookie)
                 .expires(START_OF_YEAR_1970.get())
                 .build();
 
+        // update or add new header?
         if (contains(HeaderNames.SET_COOKIE)) {
             remove(HeaderNames.SET_COOKIE, it -> {
                 List<String> currentValues = it.allValues();
                 String[] newValues = new String[currentValues.size()];
                 boolean found = false;
                 for (int i = 0; i < currentValues.size(); i++) {
                     String currentValue = currentValues.get(i);
-                    if (SetCookie.parse(currentValue).name().equals(name)) {
-                        newValues[i] = expiredCookie.text();
+                    SetCookie currentCookie = SetCookie.parse(currentValue);
+                    if (predicate.test(currentCookie)) {
+                        newValues[i] = expiredCookie.text();            // replaces with expired cookie
                         found = true;
                     } else {
                         newValues[i] = currentValue;
@@ -63,15 +77,13 @@ public ServerResponseHeaders clearCookie(String name) {
                 if (!found) {
                     String[] values = new String[newValues.length + 1];
                     System.arraycopy(newValues, 0, values, 0, newValues.length);
-                    values[values.length - 1] = expiredCookie.text();
+                    values[values.length - 1] = expiredCookie.text();   // adds expired cookie
                     newValues = values;
                 }
-
                 set(HeaderValues.create(HeaderNames.SET_COOKIE, newValues));
             });
         } else {
             addCookie(expiredCookie);
         }
-        return this;
     }
 }

http/http/src/main/java/io/helidon/http/SetCookie.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -67,18 +67,32 @@ public static Builder builder(String name, String value) {
         return new Builder(name, value);
     }
 
+    /**
+     * Creates a new fluent API builder using another cookie.
+     *
+     * @param setCookie the other cookie
+     * @return a new fluent API builder
+     */
+    public static Builder builder(SetCookie setCookie) {
+        return new Builder(setCookie);
+    }
+
     /**
      * Parses new instance of {@link SetCookie} from the String representation.
      *
      * @param setCookie string representation
      * @return new instance
      */
     public static SetCookie parse(String setCookie) {
+        Objects.requireNonNull(setCookie);
         String[] cookieParts = setCookie.split(PARAM_SEPARATOR);
         String nameAndValue = cookieParts[0];
         int equalsIndex = nameAndValue.indexOf('=');
-        String name = nameAndValue.substring(0, equalsIndex);
-        String value = nameAndValue.length() == equalsIndex ? null : nameAndValue.substring(equalsIndex + 1);
+        String name = (equalsIndex == -1) ? nameAndValue : nameAndValue.substring(0, equalsIndex);
+        if (name.isEmpty()) {
+            throw new IllegalArgumentException("Cookie name cannot be empty");
+        }
+        String value = (equalsIndex == -1) ? "" : nameAndValue.substring(equalsIndex + 1);
         Builder builder = builder(name, value);
 
         for (int i = 1; i < cookieParts.length; i++) {
@@ -88,7 +102,7 @@ public static SetCookie parse(String setCookie) {
             String partValue;
             if (equalsIndex > -1) {
                 partName = cookiePart.substring(0, equalsIndex);
-                partValue = cookiePart.length() == equalsIndex ? null : cookiePart.substring(equalsIndex + 1);
+                partValue = cookiePart.substring(equalsIndex + 1);
             } else {
                 partName = cookiePart;
                 partValue = null;
@@ -277,6 +291,24 @@ public String toString() {
         return result.toString();
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (!(o instanceof SetCookie setCookie)) {
+            return false;
+        }
+        return Objects.equals(name, setCookie.name)
+                && Objects.equals(domain, setCookie.domain)
+                && Objects.equals(path, setCookie.path);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(name, domain, path);
+    }
+
     private static void hasNoValue(String partName, String partValue) {
         if (partValue != null) {
             throw new IllegalArgumentException("Set-Cookie parameter " + partName + " has to have no value!");
@@ -349,6 +381,19 @@ private Builder(String name, String value) {
             this.value = value;
         }
 
+        private Builder(SetCookie other) {
+            Objects.requireNonNull(other);
+            this.name = other.name;
+            this.value = other.value;
+            this.expires = other.expires;
+            this.maxAge = other.maxAge;
+            this.domain = other.domain;
+            this.path = other.path;
+            this.secure = other.secure;
+            this.httpOnly = other.httpOnly;
+            this.sameSite = other.sameSite;
+        }
+
         @Override
         public SetCookie build() {
             return new SetCookie(this);

http/http/src/test/java/io/helidon/http/SetCookieTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 package io.helidon.http;
 
 import java.time.Duration;
+import java.time.Instant;
 
 import org.junit.jupiter.api.Test;
 
@@ -29,17 +30,18 @@
  */
 public class SetCookieTest {
 
+    private static final String TEMPLATE = "some-cookie=some-cookie-value; "
+            + "Expires=Thu, 22 Oct 2015 07:28:00 GMT; "
+            + "Max-Age=2592000; "
+            + "Domain=domain.value; "
+            + "Path=/; "
+            + "Secure; "
+            + "HttpOnly; "
+            + "SameSite=Lax";
+
     @Test
-    public void testSetCookiesFromString() {
-        String template = "some-cookie=some-cookie-value; "
-                + "Expires=Thu, 22 Oct 2015 07:28:00 GMT; "
-                + "Max-Age=2592000; "
-                + "Domain=domain.value; "
-                + "Path=/; "
-                + "Secure; "
-                + "HttpOnly; "
-                + "SameSite=Lax";
-        SetCookie setCookie = SetCookie.parse(template);
+    void testSetCookiesFromString() {
+        SetCookie setCookie = SetCookie.parse(TEMPLATE);
 
         assertThat(setCookie.name(), is("some-cookie"));
         assertThat(setCookie.value(), is("some-cookie-value"));
@@ -51,16 +53,64 @@ public void testSetCookiesFromString() {
         assertThat(setCookie.httpOnly(), is(true));
         assertThat(setCookie.sameSite(), optionalValue(is(SetCookie.SameSite.LAX)));
 
-        assertThat("Generate same cookie value", setCookie.toString(), is(template));
+        assertThat("Generate same cookie value", setCookie.toString(), is(TEMPLATE));
     }
 
     @Test
-    public void testSetCookiesInvalidValue() {
+    void testSetCookiesInvalidValue() {
         String template = "some-cookie=some-cookie-value; "
                 + "Invalid=value";
         IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
                                                    () -> SetCookie.parse(template));
         assertThat(ex.getMessage(), is("Unexpected Set-Cookie part: Invalid"));
     }
 
+    @Test
+    void testEquals() {
+        SetCookie setCookie1 = SetCookie.parse(TEMPLATE);
+        SetCookie setCookie2 = SetCookie.builder("some-cookie", "").build();
+        SetCookie setCookie3 = SetCookie.builder("some-cookie", "")
+                .path("/")
+                .domain("domain.value")
+                .expires(Instant.now())     // ignored in equals
+                .build();
+
+        assertThat("They match name, path and domain",
+                   setCookie1.equals(setCookie3), is(true));
+        assertThat("They match name, path and domain",
+                   setCookie1.hashCode(), is(setCookie3.hashCode()));
+        assertThat("They do not match path or domain",
+                   setCookie1.equals(setCookie2), is(false));
+    }
+
+    @Test
+    void testCookieBuilder() {
+        SetCookie setCookie1 = SetCookie.parse(TEMPLATE);
+        SetCookie setCookie2 = SetCookie.builder(setCookie1).build();       // from setCookie1
+
+        assertThat(setCookie1.equals(setCookie2), is(true));
+        assertThat(setCookie1.hashCode(), is(setCookie2.hashCode()));
+        assertThat(setCookie2.name(), is("some-cookie"));
+        assertThat(setCookie2.value(), is("some-cookie-value"));
+        assertThat(setCookie2.expires(), optionalValue(is(DateTime.parse("Thu, 22 Oct 2015 07:28:00 GMT"))));
+        assertThat(setCookie2.maxAge(), optionalValue(is(Duration.ofSeconds(2592000))));
+        assertThat(setCookie2.domain(), optionalValue(is("domain.value")));
+        assertThat(setCookie2.path(), optionalValue(is("/")));
+        assertThat(setCookie2.secure(), is(true));
+        assertThat(setCookie2.httpOnly(), is(true));
+        assertThat(setCookie2.sameSite(), optionalValue(is(SetCookie.SameSite.LAX)));
+    }
+
+    @Test
+    void testParse() {
+        assertThrows(NullPointerException.class,() -> SetCookie.parse(null));
+        assertThrows(IllegalArgumentException.class,() -> SetCookie.parse(""));
+        SetCookie setCookie1 = SetCookie.parse("some-cookie");
+        assertThat(setCookie1.name(), is("some-cookie"));
+        assertThat(setCookie1.value(), is(""));
+        SetCookie setCookie2 = SetCookie.parse("some-cookie=");
+        assertThat(setCookie2.name(), is("some-cookie"));
+        assertThat(setCookie2.value(), is(""));
+        assertThat(setCookie1.equals(setCookie2), is(true));
+    }
 }

http/tests/cookie/pom.xml

@@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    Copyright (c) 2025 Oracle and/or its affiliates.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+        http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+  -->
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xmlns="http://maven.apache.org/POM/4.0.0"
+        xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>io.helidon.http.tests</groupId>
+        <artifactId>helidon-http-tests-project</artifactId>
+        <version>4.2.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>helidon-http-tests-cookie</artifactId>
+    <name>Helidon HTTP Tests Cookie</name>
+
+    <dependencies>
+        <dependency>
+            <groupId>io.helidon.webserver.testing.junit5</groupId>
+            <artifactId>helidon-webserver-testing-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-all</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>io.helidon.logging</groupId>
+            <artifactId>helidon-logging-jul</artifactId>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+</project>