chore: migrate to getSetCookie (#586)

.changeset/tricky-badgers-flow.md

@@ -0,0 +1,8 @@
+---
+'@edge-runtime/cookies': major
+'@edge-runtime/node-utils': patch
+'@edge-runtime/primitives': major
+'edge-runtime': patch
+---
+
+Simplify `set-cookie` handling

packages/cookies/src/index.ts

@@ -1,9 +1,4 @@
 export type { CookieListItem, RequestCookie, ResponseCookie } from './types'
 export { RequestCookies } from './request-cookies'
 export { ResponseCookies } from './response-cookies'
-export {
-  stringifyCookie,
-  parseCookie,
-  parseSetCookie,
-  splitCookiesString,
-} from './serialize'
+export { stringifyCookie, parseCookie, parseSetCookie } from './serialize'

packages/cookies/src/response-cookies.ts

@@ -1,9 +1,5 @@
 import type { ResponseCookie } from './types'
-import {
-  splitCookiesString,
-  parseSetCookie,
-  stringifyCookie,
-} from './serialize'
+import { parseSetCookie, stringifyCookie } from './serialize'
 
 /**
  * A class for manipulating {@link Response} cookies (`Set-Cookie` header).
@@ -19,14 +15,7 @@ export class ResponseCookies {
   constructor(responseHeaders: Headers) {
     this._headers = responseHeaders
 
-    const setCookie = responseHeaders.getSetCookie?.()
-    responseHeaders.get('set-cookie') ?? []
-
-    const cookieStrings = Array.isArray(setCookie)
-      ? setCookie
-      : // TODO: remove splitCookiesString when `getSetCookie` adoption is high enough in Node.js
-        // https://developer.mozilla.org/en-US/docs/Web/API/Headers/getSetCookie#browser_compatibility
-        splitCookiesString(setCookie)
+    const cookieStrings = responseHeaders.getSetCookie()
 
     for (const cookieString of cookieStrings) {
       const parsed = parseSetCookie(cookieString)

packages/cookies/src/serialize.ts

@@ -91,82 +91,3 @@ function parseSameSite(string: string): ResponseCookie['sameSite'] {
     ? (string as ResponseCookie['sameSite'])
     : undefined
 }
-
-/**
- * @source https://github.com/nfriedly/set-cookie-parser/blob/master/lib/set-cookie.js
- *
- * Set-Cookie header field-values are sometimes comma joined in one string. This splits them without choking on commas
- * that are within a single set-cookie field-value, such as in the Expires portion.
- * This is uncommon, but explicitly allowed - see https://tools.ietf.org/html/rfc2616#section-4.2
- * Node.js does this for every header *except* set-cookie - see https://github.com/nodejs/node/blob/d5e363b77ebaf1caf67cd7528224b651c86815c1/lib/_http_incoming.js#L128
- * React Native's fetch does this for *every* header, including set-cookie.
- *
- * Based on: https://github.com/google/j2objc/commit/16820fdbc8f76ca0c33472810ce0cb03d20efe25
- * Credits to: https://github.com/tomball for original and https://github.com/chrusart for JavaScript implementation
- */
-export function splitCookiesString(cookiesString: string) {
-  if (!cookiesString) return []
-  var cookiesStrings = []
-  var pos = 0
-  var start
-  var ch
-  var lastComma
-  var nextStart
-  var cookiesSeparatorFound
-
-  function skipWhitespace() {
-    while (pos < cookiesString.length && /\s/.test(cookiesString.charAt(pos))) {
-      pos += 1
-    }
-    return pos < cookiesString.length
-  }
-
-  function notSpecialChar() {
-    ch = cookiesString.charAt(pos)
-
-    return ch !== '=' && ch !== ';' && ch !== ','
-  }
-
-  while (pos < cookiesString.length) {
-    start = pos
-    cookiesSeparatorFound = false
-
-    while (skipWhitespace()) {
-      ch = cookiesString.charAt(pos)
-      if (ch === ',') {
-        // ',' is a cookie separator if we have later first '=', not ';' or ','
-        lastComma = pos
-        pos += 1
-
-        skipWhitespace()
-        nextStart = pos
-
-        while (pos < cookiesString.length && notSpecialChar()) {
-          pos += 1
-        }
-
-        // currently special character
-        if (pos < cookiesString.length && cookiesString.charAt(pos) === '=') {
-          // we found cookies separator
-          cookiesSeparatorFound = true
-          // pos is inside the next cookie, so back up and return it.
-          pos = nextStart
-          cookiesStrings.push(cookiesString.substring(start, lastComma))
-          start = pos
-        } else {
-          // in param ',' or param separator ';',
-          // we continue from that comma
-          pos = lastComma + 1
-        }
-      } else {
-        pos += 1
-      }
-    }
-
-    if (!cookiesSeparatorFound || pos >= cookiesString.length) {
-      cookiesStrings.push(cookiesString.substring(start, cookiesString.length))
-    }
-  }
-
-  return cookiesStrings
-}

packages/cookies/test/index.test.ts

@@ -1,9 +1,4 @@
-import {
-  stringifyCookie,
-  parseCookie,
-  parseSetCookie,
-  splitCookiesString,
-} from '../src'
+import { stringifyCookie, parseCookie, parseSetCookie } from '../src'
 
 test('.stringifyCookie is exported', async () => {
   expect(typeof stringifyCookie).toBe('function')
@@ -16,7 +11,3 @@ test('.parseCookie is exported', async () => {
 test('.parseSetCookie is exported', async () => {
   expect(typeof parseSetCookie).toBe('function')
 })
-
-test('.splitCookiesString is exported', async () => {
-  expect(typeof splitCookiesString).toBe('function')
-})

packages/integration-tests/tests/response.test.ts

@@ -10,10 +10,8 @@ test('allow to append multiple `set-cookie` header', async () => {
   const response = new Response(null)
   response.headers.append('set-cookie', 'foo=bar')
   response.headers.append('set-cookie', 'bar=baz')
-  expect(response.headers.getAll?.('set-cookie')).toEqual([
-    'foo=bar',
-    'bar=baz',
-  ])
+
+  expect(response.headers.getSetCookie()).toEqual(['foo=bar', 'bar=baz'])
 })
 
 test('disallow mutate response headers for redirects', async () => {

packages/node-utils/package.json

@@ -23,9 +23,6 @@
     "utils",
     "web"
   ],
-  "dependencies": {
-    "@edge-runtime/cookies": "workspace:*"
-  },
   "devDependencies": {
     "@edge-runtime/primitives": "workspace:*",
     "@types/test-listen": "1.1.0",

packages/node-utils/src/edge-to-node/handler.ts

@@ -12,20 +12,20 @@ import { toToReadable } from './stream'
 
 export function buildToNodeHandler(
   dependencies: BuildDependencies,
-  options: RequestOptions
+  options: RequestOptions,
 ) {
   const toRequest = buildToRequest(dependencies)
   const toFetchEvent = buildToFetchEvent(dependencies)
   return function toNodeHandler(webHandler: WebHandler): NodeHandler {
     return (
       incomingMessage: IncomingMessage,
-      serverResponse: ServerResponse
+      serverResponse: ServerResponse,
     ) => {
       const request = toRequest(incomingMessage, options)
       const maybePromise = webHandler(request, toFetchEvent(request))
       if (maybePromise instanceof Promise) {
         maybePromise.then((response) =>
-          toServerResponse(response, serverResponse)
+          toServerResponse(response, serverResponse),
         )
       } else {
         toServerResponse(maybePromise, serverResponse)
@@ -36,16 +36,15 @@ export function buildToNodeHandler(
 
 function toServerResponse(
   webResponse: Response | null | undefined,
-  serverResponse: ServerResponse
+  serverResponse: ServerResponse,
 ) {
   if (!webResponse) {
     serverResponse.end()
     return
   }
   mergeIntoServerResponse(
-    // @ts-ignore getAll() is not standard https://fetch.spec.whatwg.org/#headers-class
     toOutgoingHeaders(webResponse.headers),
-    serverResponse
+    serverResponse,
   )
 
   serverResponse.statusCode = webResponse.status

packages/node-utils/src/edge-to-node/headers.ts

@@ -1,20 +1,13 @@
-import type { Headers } from '@edge-runtime/primitives'
+import { Headers } from '@edge-runtime/primitives'
 import type { OutgoingHttpHeaders, ServerResponse } from 'node:http'
-import { splitCookiesString } from '@edge-runtime/cookies'
 
-export function toOutgoingHeaders(
-  headers?: Headers & { raw?: () => Record<string, string> },
-): OutgoingHttpHeaders {
+export function toOutgoingHeaders(headers?: Headers): OutgoingHttpHeaders {
   const outputHeaders: OutgoingHttpHeaders = {}
   if (headers) {
-    for (const [name, value] of typeof headers.raw !== 'undefined'
-      ? Object.entries(headers.raw())
-      : headers.entries()) {
-      outputHeaders[name] = value
-      if (name.toLowerCase() === 'set-cookie') {
-        outputHeaders[name] =
-          headers.getAll?.('set-cookie') ?? splitCookiesString(value)
-      }
+    const _headers = new Headers(headers).entries()
+    for (const [name, value] of _headers) {
+      outputHeaders[name] =
+        name === 'set-cookie' ? headers.getSetCookie() : value
     }
   }
   return outputHeaders

packages/node-utils/test/edge-to-node/headers.test.ts

@@ -7,15 +7,15 @@ it('handles simple header values', () => {
       new Headers({
         'Content-Type': 'image/jpeg',
         'X-My-Custom-Header': 'Zeke are cool',
-      })
-    )
+      }),
+    ),
   ).toEqual({
     'content-type': 'image/jpeg',
     'x-my-custom-header': 'Zeke are cool',
   })
 })
 
-it('splits set-cookie with getAll()', () => {
+it('splits set-cookie with getSetCookie()', () => {
   const headers = new Headers({ 'set-cookie': 'value1' })
   headers.append('set-cookie', 'value2')
   headers.append('set-cookie', 'value3')
@@ -24,22 +24,6 @@ it('splits set-cookie with getAll()', () => {
   })
 })
 
-it('slits set-cookie without getAll()', () => {
-  const rawHeaders = {
-    raw: () => ({
-      'set-cookie':
-        'cookie1=value1, cookie2=value2; Max-Age=1000, cookie3=value3; Domain=<domain-value>; Secure',
-    }),
-  }
-  expect(toOutgoingHeaders(rawHeaders as unknown as Headers)).toEqual({
-    'set-cookie': [
-      'cookie1=value1',
-      'cookie2=value2; Max-Age=1000',
-      'cookie3=value3; Domain=<domain-value>; Secure',
-    ],
-  })
-})
-
 it('handles multiple values as single string', () => {
   const headers = new Headers({ 'x-multiple': 'value1' })
   headers.append('x-multiple', 'value2')

packages/primitives/src/primitives/fetch.js

@@ -1,8 +1,6 @@
 import * as FetchSymbols from 'undici/lib/fetch/symbols'
 import * as HeadersModule from 'undici/lib/fetch/headers'
 import * as ResponseModule from 'undici/lib/fetch/response'
-import * as UtilModule from 'undici/lib/fetch/util'
-import * as WebIDLModule from 'undici/lib/fetch/webidl'
 import { Request as BaseRequest } from 'undici/lib/fetch/request'
 
 import { fetch as fetchImpl } from 'undici/lib/fetch'
@@ -45,20 +43,6 @@ HeadersModule.Headers.prototype.values = function* () {
   }
 }
 
-/**
- * Add a new method for retrieving all independent `set-cookie` headers that
- * maybe have been appended. This will only work when getting `set-cookie`
- * headers.
- */
-HeadersModule.Headers.prototype.getAll = function (name) {
-  const _name = normalizeAndValidateHeaderName(name, 'Headers.getAll')
-  if (_name !== 'set-cookie') {
-    throw new Error(`getAll can only be used with 'set-cookie'`)
-  }
-
-  return this.getSetCookie()
-}
-
 /**
  * We also must patch the error static method since it works just like
  * redirect and we need consistency.
@@ -70,55 +54,6 @@ ResponseModule.Response.error = function (...args) {
   return response
 }
 
-/**
- * normalize header name per WHATWG spec, and validate
- *
- * @param {string} potentialName
- * @param {'Header.append' | 'Headers.delete' | 'Headers.get' | 'Headers.has' | 'Header.set'} errorPrefix
- */
-function normalizeAndValidateHeaderName(potentialName, errorPrefix) {
-  const normalizedName = potentialName.toLowerCase()
-
-  if (UtilModule.isValidHeaderName(normalizedName)) {
-    return normalizedName
-  }
-
-  // Generate an WHATWG fetch spec compliant error
-  WebIDLModule.errors.invalidArgument({
-    prefix: errorPrefix,
-    value: normalizedName,
-    type: 'header name',
-  })
-}
-
-/**
- * normalize header value per WHATWG spec, and validate
- *
- * @param {string} potentialValue
- * @param {'Header.append' | 'Header.set'} errorPrefix
- */
-function normalizeAndValidateHeaderValue(potentialValue, errorPrefix) {
-  /**
-   * To normalize a byte sequence potentialValue, remove
-   * any leading and trailing HTTP whitespace bytes from
-   *  potentialValue.
-   *
-   * See https://fetch.spec.whatwg.org/#concept-header-value-normalize
-   */
-  const normalizedValue = potentialValue.replace(/^[\r\n\t ]+|[\r\n\t ]+$/g, '')
-
-  if (UtilModule.isValidHeaderValue(normalizedValue)) {
-    return normalizedValue
-  }
-
-  // Generate an WHATWG fetch spec compliant error
-  WebIDLModule.errors.invalidArgument({
-    prefix: errorPrefix,
-    value: normalizedValue,
-    type: 'header value',
-  })
-}
-
 /**
  * A global agent to be used with every fetch request. We also define a
  * couple of globals that we can hide in the runtime for advanced use.

packages/primitives/type-definitions/fetch.d.ts

@@ -1,6 +1,4 @@
-export class Headers extends globalThis.Headers {
-  getAll?(key: 'set-cookie'): string[]
-}
+export class Headers extends globalThis.Headers {}
 
 export class Request extends globalThis.Request {
   readonly headers: Headers
@@ -16,7 +14,7 @@ export type RequestInfo = string | Request | globalThis.Request
 export type RequestInit = globalThis.RequestInit
 declare const fetchImplementation: (
   info: RequestInfo,
-  init?: RequestInit
+  init?: RequestInit,
 ) => Promise<Response>
 
 declare const FileConstructor: typeof File