Adds additional checks during cookie parsing. Fixes comment.

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>
spericas · Feb 11, 2025 · 93b401a · 93b401a
1 parent 24a0501
commit 93b401a
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 14 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
@@ -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
@@ -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
@@ -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,7 @@ public void testSetCookiesInvalidValue() {
     }
 
     @Test
-    public 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 +84,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 +100,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));
+    }
 }