From 7209feacbd287444fb2b9076820396c4ab3e0486 Mon Sep 17 00:00:00 2001
From: Santiago Pericas-Geertsen <santiago.pericasgeertsen@oracle.com>
Date: Tue, 11 Feb 2025 09:50:34 -0500
Subject: [PATCH] Adds additional checks during cookie parsing. Fixes comment.

Signed-off-by: Santiago Pericas-Geertsen <santiago.pericasgeertsen@oracle.com>
---
 .../http/ServerResponseHeadersImpl.java       |  4 ++--
 .../main/java/io/helidon/http/SetCookie.java  |  8 +++++--
 .../java/io/helidon/http/SetCookieTest.java   | 23 +++++++++++++++----
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/http/http/src/main/java/io/helidon/http/ServerResponseHeadersImpl.java b/http/http/src/main/java/io/helidon/http/ServerResponseHeadersImpl.java
index 2b6ab8ba982..a59831abcb0 100644
--- a/http/http/src/main/java/io/helidon/http/ServerResponseHeadersImpl.java
+++ b/http/http/src/main/java/io/helidon/http/ServerResponseHeadersImpl.java
@@ -68,7 +68,7 @@ private void clearCookie(SetCookie cookie, Predicate<SetCookie> predicate) {
                     String currentValue = currentValues.get(i);
                     SetCookie currentCookie = SetCookie.parse(currentValue);
                     if (predicate.test(currentCookie)) {
-                        newValues[i] = expiredCookie.text();            // replace with expired
+                        newValues[i] = expiredCookie.text();            // replaces with expired cookie
                         found = true;
                     } else {
                         newValues[i] = currentValue;
@@ -77,7 +77,7 @@ private void clearCookie(SetCookie cookie, Predicate<SetCookie> predicate) {
                 if (!found) {
                     String[] values = new String[newValues.length + 1];
                     System.arraycopy(newValues, 0, values, 0, newValues.length);
-                    values[values.length - 1] = expiredCookie.text();   // replace with expired
+                    values[values.length - 1] = expiredCookie.text();   // adds expired cookie
                     newValues = values;
                 }
                 set(HeaderValues.create(HeaderNames.SET_COOKIE, newValues));
diff --git a/http/http/src/main/java/io/helidon/http/SetCookie.java b/http/http/src/main/java/io/helidon/http/SetCookie.java
index 3ae26b82300..2cd2e9bfb28 100644
--- a/http/http/src/main/java/io/helidon/http/SetCookie.java
+++ b/http/http/src/main/java/io/helidon/http/SetCookie.java
@@ -84,11 +84,15 @@ public static Builder builder(SetCookie setCookie) {
      * @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.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++) {
diff --git a/http/http/src/test/java/io/helidon/http/SetCookieTest.java b/http/http/src/test/java/io/helidon/http/SetCookieTest.java
index efeaa6c1d77..28a59ff97da 100644
--- a/http/http/src/test/java/io/helidon/http/SetCookieTest.java
+++ b/http/http/src/test/java/io/helidon/http/SetCookieTest.java
@@ -40,7 +40,7 @@ public class SetCookieTest {
             + "SameSite=Lax";
 
     @Test
-    public void testSetCookiesFromString() {
+    void testSetCookiesFromString() {
         SetCookie setCookie = SetCookie.parse(TEMPLATE);
 
         assertThat(setCookie.name(), is("some-cookie"));
@@ -57,7 +57,7 @@ public void testSetCookiesFromString() {
     }
 
     @Test
-    public void testSetCookiesInvalidValue() {
+    void testSetCookiesInvalidValue() {
         String template = "some-cookie=some-cookie-value; "
                 + "Invalid=value";
         IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
@@ -66,13 +66,13 @@ public void testSetCookiesInvalidValue() {
     }
 
     @Test
-    public void testEmptyValue() {
+    void testEmptyValue() {
         SetCookie setCookie = SetCookie.parse("some-cookie=");
         assertThat(setCookie.value(), is(""));
     }
 
     @Test
-    public void testEquals() {
+    void testEquals() {
         SetCookie setCookie1 = SetCookie.parse(TEMPLATE);
         SetCookie setCookie2 = SetCookie.builder("some-cookie", "").build();
         SetCookie setCookie3 = SetCookie.builder("some-cookie", "")
@@ -90,7 +90,7 @@ public void testEquals() {
     }
 
     @Test
-    public void testCookieBuilder() {
+    void testCookieBuilder() {
         SetCookie setCookie1 = SetCookie.parse(TEMPLATE);
         SetCookie setCookie2 = SetCookie.builder(setCookie1).build();       // from setCookie1
 
@@ -106,4 +106,17 @@ public void testCookieBuilder() {
         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));
+    }
 }