fix(cookies): preserve values and parse SameSite strictly

Signed-off-by: Matteo Collina <hello@matteocollina.com>
(cherry picked from commit 5655ea4)

Author: mcollina

docs/docs/api/Cookies.md

@@ -80,6 +80,33 @@ Arguments:
 
 Returns: `Cookie[]`
 
+## `parseCookie(cookie)`
+
+Parses a single `Set-Cookie` header value into a `Cookie` object.
+
+```js
+import { parseCookie } from 'undici'
+
+console.log(parseCookie('undici=getSetCookies; Secure; SameSite=Lax'))
+// {
+//   name: 'undici',
+//   value: 'getSetCookies',
+//   secure: true,
+//   sameSite: 'Lax'
+// }
+```
+
+Notes:
+
+* The cookie value is returned as it appears in the header. Percent-encoded sequences such as `%20` or `%0D%0A` are **not** decoded.
+* `sameSite` is only set for exact case-insensitive matches of `Strict`, `Lax`, or `None`.
+
+Arguments:
+
+* **cookie** `string`
+
+Returns: `Cookie | null`
+
 ## `setCookie(headers, cookie)`
 
 Appends a cookie to the `Set-Cookie` header.

lib/web/cookies/parse.js

@@ -4,7 +4,6 @@ const { collectASequenceOfCodePointsFast } = require('../infra')
 const { maxNameValuePairSize, maxAttributeValueSize } = require('./constants')
 const { isCTLExcludingHtab } = require('./util')
 const assert = require('node:assert')
-const { unescape: qsUnescape } = require('node:querystring')
 
 /**
  * @description Parses the field-value attributes of a set-cookie header string.
@@ -82,7 +81,7 @@ function parseSetCookie (header) {
   // store arbitrary data in a cookie-value SHOULD encode that data, for
   // example, using Base64 [RFC4648].
   return {
-    name, value: qsUnescape(value), ...parseUnparsedAttributes(unparsedAttributes)
+    name, value, ...parseUnparsedAttributes(unparsedAttributes)
   }
 }
 
@@ -280,32 +279,25 @@ function parseUnparsedAttributes (unparsedAttributes, cookieAttributeList = {})
     // If the attribute-name case-insensitively matches the string
     // "SameSite", the user agent MUST process the cookie-av as follows:
 
-    // 1. Let enforcement be "Default".
-    let enforcement = 'Default'
-
     const attributeValueLowercase = attributeValue.toLowerCase()
-    // 2. If cookie-av's attribute-value is a case-insensitive match for
-    //    "None", set enforcement to "None".
-    if (attributeValueLowercase.includes('none')) {
-      enforcement = 'None'
-    }
 
-    // 3. If cookie-av's attribute-value is a case-insensitive match for
-    //    "Strict", set enforcement to "Strict".
-    if (attributeValueLowercase.includes('strict')) {
-      enforcement = 'Strict'
+    // 1. If cookie-av's attribute-value is a case-insensitive match for
+    //    "None", append an attribute to the cookie-attribute-list with an
+    //    attribute-name of "SameSite" and an attribute-value of "None".
+    if (attributeValueLowercase === 'none') {
+      cookieAttributeList.sameSite = 'None'
+    } else if (attributeValueLowercase === 'strict') {
+      // 2. If cookie-av's attribute-value is a case-insensitive match for
+      //    "Strict", append an attribute to the cookie-attribute-list with
+      //    an attribute-name of "SameSite" and an attribute-value of
+      //    "Strict".
+      cookieAttributeList.sameSite = 'Strict'
+    } else if (attributeValueLowercase === 'lax') {
+      // 3. If cookie-av's attribute-value is a case-insensitive match for
+      //    "Lax", append an attribute to the cookie-attribute-list with an
+      //    attribute-name of "SameSite" and an attribute-value of "Lax".
+      cookieAttributeList.sameSite = 'Lax'
     }
-
-    // 4. If cookie-av's attribute-value is a case-insensitive match for
-    //    "Lax", set enforcement to "Lax".
-    if (attributeValueLowercase.includes('lax')) {
-      enforcement = 'Lax'
-    }
-
-    // 5. Append an attribute to the cookie-attribute-list with an
-    //    attribute-name of "SameSite" and an attribute-value of
-    //    enforcement.
-    cookieAttributeList.sameSite = enforcement
   } else {
     cookieAttributeList.unparsed ??= []
 

test/cookie/cookies.js

@@ -28,6 +28,7 @@ const {
   deleteCookie,
   getCookies,
   getSetCookies,
+  parseCookie,
   setCookie,
   Headers
 } = require('../..')
@@ -600,6 +601,41 @@ test('Set-Cookie parser', () => {
   assert.deepEqual(getSetCookies(headers), [])
 })
 
+test('Set-Cookie parser does not percent-decode cookie values', () => {
+  assert.deepEqual(
+    parseCookie(
+      'token=legit%0d%0aSet-Cookie:%20evil=injected%3B%20Path%3D/'
+    ),
+    {
+      name: 'token',
+      value: 'legit%0d%0aSet-Cookie:%20evil=injected%3B%20Path%3D/'
+    }
+  )
+
+  assert.deepEqual(parseCookie('data=prefix%00suffix'), {
+    name: 'data',
+    value: 'prefix%00suffix'
+  })
+})
+
+test('Set-Cookie parser only accepts exact SameSite values', () => {
+  assert.deepEqual(parseCookie('a=b; SameSite=none'), {
+    name: 'a',
+    value: 'b',
+    sameSite: 'None'
+  })
+
+  assert.deepEqual(parseCookie('a=b; SameSite=StrictLax'), {
+    name: 'a',
+    value: 'b'
+  })
+
+  assert.deepEqual(parseCookie('a=b; SameSite=NoneOfYourBusiness'), {
+    name: 'a',
+    value: 'b'
+  })
+})
+
 test('Cookie setCookie throws if headers is not of type Headers', () => {
   class Headers {
     [Symbol.toStringTag] = 'CustomHeaders'

test/cookie/npm-cookie.js

@@ -51,13 +51,16 @@ describe('parseCookie(str)', function () {
     assert.deepStrictEqual(parseCookie('f=;b='), { name: 'f', value: '', unparsed: ['b='] })
   })
 
-  it('should URL-decode values', function () {
+  it('should preserve encoded values', function () {
     assert.deepStrictEqual(parseCookie('foo="bar=123456789&name=Magic+Mouse"'), {
       name: 'foo',
       value: '"bar=123456789&name=Magic+Mouse"'
     })
 
-    assert.deepStrictEqual(parseCookie('email=%20%22%2c%3b%2f'), { name: 'email', value: ' ",;/' })
+    assert.deepStrictEqual(parseCookie('email=%20%22%2c%3b%2f'), {
+      name: 'email',
+      value: '%20%22%2c%3b%2f'
+    })
   })
 
   it('should trim whitespace around key and value', function () {