fetch implementation injected into middleware misbehaves with set-cookie

abuinitski commented 1 year ago

Link to the code that reproduces this issue or a replay of the bug

https://github.com/abuinitski/next-fetch-bug-setcookie

To Reproduce

start the application with npm run dev
trigger POST http://localhost:3000/api manually. Review response headers. Observe three cookies are set in response via three distinct set-cookie headers (correct behavior)
open GET http://localhost:3000. Review application logs. Observe single set-cookie header is seen by the middleware. Note that three expected header values are instead concatenated.

Current vs. Expected behavior

Set-Cookie is a bit special HTTP header. When multiple cookies needs to be set for a single response, multiple Set-Cookie headers should be sent. While usually HTTP headers are expected to have at most one occurrence in a single HTTP payload.

Reason is, after set-cookie values are concatenated, they are no longer properly parseable, as concatenation joiner is comma, and other commas are also present inside each individual Set-Cookie value.

fetch API has a dedicated method to handle it: fetch.headers.getSetCookie(), but it is not working as expected when used within a middleware.

Expected behavior:

when using fetch within a middleware to trigger an API response that sets multiple cookies, calling fetch.headers.getSetCookie() should return an array of strings with each individual Set-Cookie value. Actual behavior:
when using fetch within a middleware to trigger an API response that sets multiple cookies, calling fetch.headers.getSetCookie() returns an array of single string, where all individual values are concatenated.

Verify canary release

[X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.6.0: Wed Jul  5 22:21:53 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6020
    Binaries:
      Node: 20.5.0
      npm: 9.8.0
      Yarn: N/A
      pnpm: N/A
    Relevant Packages:
      next: 13.4.20-canary.18
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.2.2
    Next.js Config:
      output: N/A

Which area(s) are affected? (Select all that apply)

Data fetching (gS(S)P, getInitialProps), Middleware / Edge (API routes, runtime)

Additional context

No response

davidkhierl commented 1 year ago

is there any movement on this? it's quite a blocker specially for implementing authentication with multiple cookies.

devjiwonchoi commented 6 months ago

Temporary workaround:

/**
 * @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
}

vercel / next.js