Merge pull request #20 from FusionAuth/degroff/parse-dquote-cookie-value

Fix cookie parsing to account for double-quoted values.
FusionAuth · Mar 29, 2024 · 0f8139d · 0f8139d
2 parents 3484282 + aeb05de
commit 0f8139d
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 34 deletions.
diff --git a/build.savant b/build.savant
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2023, FusionAuth, All Rights Reserved
+ * Copyright (c) 2018-2024, FusionAuth, All Rights Reserved
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,7 +17,7 @@ jackson5Version = "3.0.1"
 restifyVersion = "4.2.0"
 testngVersion = "7.8.0"
 
-project(group: "io.fusionauth", name: "java-http", version: "0.3.0", licenses: ["ApacheV2_0"]) {
+project(group: "io.fusionauth", name: "java-http", version: "0.3.1", licenses: ["ApacheV2_0"]) {
   workflow {
     fetch {
       // Dependency resolution order:

diff --git a/java-http.ipr b/java-http.ipr
@@ -12,6 +12,13 @@
   <component name="InspectionProjectProfileManager">
     <profile version="1.0">
       <option name="myName" value="Project Default" />
+      <inspection_tool class="SizeReplaceableByIsEmpty" enabled="true" level="WARNING" enabled_by_default="true">
+        <option name="ignoredTypes">
+          <set>
+            <option value="java.util.List" />
+          </set>
+        </option>
+      </inspection_tool>
       <inspection_tool class="groupsTestNG" enabled="true" level="WARNING" enabled_by_default="true">
         <option name="groups">
           <value>
@@ -1385,32 +1392,6 @@
   <component name="ProjectRootManager" version="2" languageLevel="JDK_17" default="true" project-jdk-name="Java 17" project-jdk-type="JavaSDK">
     <output url="file://$PROJECT_DIR$/out" />
   </component>
-  <component name="ProjectRunConfigurationManager">
-    <configuration default="true" type="TestNG">
-      <module name="java-http" />
-      <shortenClasspath name="NONE" />
-      <useClassPathOnly />
-      <option name="SUITE_NAME" value="" />
-      <option name="PACKAGE_NAME" value="" />
-      <option name="MAIN_CLASS_NAME" value="" />
-      <option name="GROUP_NAME" value="" />
-      <option name="TEST_OBJECT" value="CLASS" />
-      <option name="VM_PARAMETERS" value="-ea --add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.util=ALL-UNNAMED" />
-      <option name="PARAMETERS" value="" />
-      <option name="OUTPUT_DIRECTORY" value="" />
-      <option name="TEST_SEARCH_SCOPE">
-        <value defaultName="moduleWithDependencies" />
-      </option>
-      <option name="PROPERTIES_FILE" value="" />
-      <properties />
-      <listeners>
-        <listener class="io.fusionauth.http.BaseTest$TestListener" />
-      </listeners>
-      <method v="2">
-        <option name="Make" enabled="true" />
-      </method>
-    </configuration>
-  </component>
   <component name="VcsDirectoryMappings">
     <mapping directory="$PROJECT_DIR$" vcs="Git" />
   </component>

diff --git a/src/main/java/io/fusionauth/http/Cookie.java b/src/main/java/io/fusionauth/http/Cookie.java
@@ -130,6 +130,8 @@ public static List<Cookie> fromRequestHeader(String header) {
    * @return The Cookie.
    */
   public static Cookie fromResponseHeader(String header) {
+    // Note we could use the JDK java.net.HttpCookie.parse(header) instead.
+    // - However, they still don't support SameSite. Super lame.
     Cookie cookie = new Cookie();
     boolean inName = false, inValue = false, inAttributes = false;
     char[] chars = header.toCharArray();
@@ -145,14 +147,19 @@ public static Cookie fromResponseHeader(String header) {
 
       if (c == '=' && inName) {
         name = header.substring(start, i);
-        if (!inAttributes && name.trim().length() == 0) {
+        if (!inAttributes && name.trim().isEmpty()) {
           return null;
         }
 
         value = "";
         inValue = true;
         inName = false;
-        start = i + 1;
+        // Values may be double-quoted
+        // https://www.rfc-editor.org/rfc/rfc6265#section-4.1.1
+        // cookie-value = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
+        start = (i < (header.length() - 1)) && chars[i + 1] == '"'
+            ? i + 2
+            : i + 1;
       } else if (c == ';') {
         if (inName) {
           if (!inAttributes) {
@@ -162,7 +169,11 @@ public static Cookie fromResponseHeader(String header) {
           name = header.substring(start, i);
           value = null;
         } else {
-          value = header.substring(start, i);
+          // Values may be double-quoted
+          int end = chars[i - 1] == '"'
+              ? i - 1
+              : i;
+          value = header.substring(start, end);
         }
 
         if (inAttributes) {
@@ -209,6 +220,10 @@ public static Cookie fromResponseHeader(String header) {
   }
 
   public void addAttribute(String name, String value) {
+    if (name == null) {
+      return;
+    }
+
     switch (name.toLowerCase()) {
       case HTTPValues.CookieAttributes.DomainLower:
         domain = value;

diff --git a/src/test/java/io/fusionauth/http/CookieTest.java b/src/test/java/io/fusionauth/http/CookieTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021-2022, FusionAuth, All Rights Reserved
+ * Copyright (c) 2021-2024, FusionAuth, All Rights Reserved
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -169,6 +169,58 @@ public void fromResponseHeader() {
     assertFalse(cookie.secure);
     assertEquals(cookie.value, "bar");
 
+    // Quoted value, no other attributes
+    // https://www.rfc-editor.org/rfc/rfc6265#section-4.1.1
+    // cookie-value = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
+    cookie = Cookie.fromResponseHeader("foo=\"bar\";");
+    assertNull(cookie.domain);
+    assertNull(cookie.expires);
+    assertFalse(cookie.httpOnly);
+    assertNull(cookie.maxAge);
+    assertEquals(cookie.name, "foo");
+    assertNull(cookie.path);
+    assertNull(cookie.sameSite);
+    assertFalse(cookie.secure);
+    assertEquals(cookie.value, "bar");
+
+    // Quoted value, additional attribute
+    cookie = Cookie.fromResponseHeader("foo=\"bar\"; SameSite=");
+    assertNull(cookie.domain);
+    assertNull(cookie.expires);
+    assertFalse(cookie.httpOnly);
+    assertNull(cookie.maxAge);
+    assertEquals(cookie.name, "foo");
+    assertNull(cookie.path);
+    assertNull(cookie.sameSite);
+    assertFalse(cookie.secure);
+    assertEquals(cookie.value, "bar");
+
+    // Missing closing quote
+    // - This is not a valid value, but we are handling it anyway.
+    cookie = Cookie.fromResponseHeader("foo=\"bar; SameSite=");
+    assertNull(cookie.domain);
+    assertNull(cookie.expires);
+    assertFalse(cookie.httpOnly);
+    assertNull(cookie.maxAge);
+    assertEquals(cookie.name, "foo");
+    assertNull(cookie.path);
+    assertNull(cookie.sameSite);
+    assertFalse(cookie.secure);
+    assertEquals(cookie.value, "bar");
+
+    // Missing opening quote
+    // - This is not a valid value, but we are handling it anyway.
+    cookie = Cookie.fromResponseHeader("foo=bar\"; SameSite=");
+    assertNull(cookie.domain);
+    assertNull(cookie.expires);
+    assertFalse(cookie.httpOnly);
+    assertNull(cookie.maxAge);
+    assertEquals(cookie.name, "foo");
+    assertNull(cookie.path);
+    assertNull(cookie.sameSite);
+    assertFalse(cookie.secure);
+    assertEquals(cookie.value, "bar");
+
     // Broken attributes
     cookie = Cookie.fromResponseHeader("foo=bar;  =fusionauth.io; =Wed, 21 Oct 2015 07:28:00 GMT; =1; =Lax");
     assertNull(cookie.domain);
@@ -206,7 +258,8 @@ public void fromResponseHeader() {
     assertEquals(cookie.value, "");
 
     // Empty values
-    cookie = Cookie.fromResponseHeader("foo=;Domain=;Expires=;Max-Age=;SameSite=");
+    // - Max-Age and Expires should never be empty
+    cookie = Cookie.fromResponseHeader("foo=;Domain=;SameSite=");
     assertEquals(cookie.domain, "");
     assertNull(cookie.expires);
     assertFalse(cookie.httpOnly);
@@ -319,4 +372,4 @@ public void roundTripSingle(String scheme) throws Exception {
   public void toRequestHeader() {
     assertEquals("foo=bar", new Cookie("foo", "bar").toRequestHeader());
   }
-}
+}