Allow setting cookies with special characters

[abhi12299]

This is a fix for vercel/next.js#70523

What?

Access the ResponseCookie object using the NextResponse class in a nextjs middleware like so:

const response = NextResponse.next()
const data = {
  value: "bar 50%"
}

response.cookies.set({
  name: 'foo',
  value: JSON.stringify(data)
})

You'll notice that the cookie being set crashes the application. The reason for this is explained below:

• ResponseCookie's constructor calls the parseSetCookie method:

  edge-runtime/packages/cookies/src/response-cookies.ts

  Lines 31 to 33 in 8312ccd

  	for (const cookieString of cookieStrings) {
  	const parsed = parseSetCookie(cookieString)
  	if (parsed) this._parsed.set(parsed.name, parsed)

• parseSetCookie function calls the parseCookie function:

  edge-runtime/packages/cookies/src/serialize.ts

  Lines 54 to 59 in 8312ccd

  	export function parseSetCookie(setCookie: string): undefined | ResponseCookie {
  	if (!setCookie) {
  	return undefined
  	}
  	const [[name, value], ...attributes] = parseCookie(setCookie)

• parseCookie calls the decodeURIComponent function on the cookie value:

  edge-runtime/packages/cookies/src/serialize.ts

  Lines 42 to 44 in 8312ccd

  	const [key, value] = [pair.slice(0, splitAt), pair.slice(splitAt + 1)]
  	try {
  	map.set(key, decodeURIComponent(value ?? 'true'))

• parseSetCookie function again calls the decodeURIComponent function on line 75:

  edge-runtime/packages/cookies/src/serialize.ts

  Lines 73 to 76 in 8312ccd

  	const cookie: ResponseCookie = {
  	name,
  	value: decodeURIComponent(value),
  	domain,

This double invocation of decodeURIComponent throws an error and crashes the application if the cookie contains special characters.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 6dcc952

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

@abhi12299 is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
edge-runtime	⬜️ Skipped (Inspect)			Oct 7, 2024 9:33am

[wyattjoh]

It's important for this particular framework that it adheres to the published RFC 6265 which requires that cookie values are URL encoded. We'd prefer in these cases to have the application throw an error during parsing if the cookie values can't be parsed rather than allowing un-encoded values from the cookie parser itself.

If the issue is stemming from double-decoding I'd rather solve that specifically rather than have it silently fail decoding.

[abhi12299]

as per my limited understanding of this codebase, i would assume that the headers being set for set-cookie would still encode the cookie value. see here:

edge-runtime/packages/cookies/src/serialize.ts

Line 21 in 37333f5

const stringified = `${c.name}=${encodeURIComponent(c.value ?? '')}`

i may be wrong, but the outbound headers still encode the cookie value - this issue stems from double decoding. Am i missing something obvious here?

[Sathosk]

What is the purpose of calling decodeURIComponent when defining the new cookie object? The function parseCookie is already returning a new map with all values decoded. It seems redundant to me, or I'm missing something?