Allow setting cookies with special characters
This is a fix for https://github.com/vercel/next.js/issues/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 theparseSetCookiemethod: https://github.com/vercel/edge-runtime/blob/8312ccd3e507aa8683b3705f2ca2d9862183982b/packages/cookies/src/response-cookies.ts#L31-L33parseSetCookiefunction calls theparseCookiefunction: https://github.com/vercel/edge-runtime/blob/8312ccd3e507aa8683b3705f2ca2d9862183982b/packages/cookies/src/serialize.ts#L54-L59parseCookiecalls thedecodeURIComponentfunction on the cookie value: https://github.com/vercel/edge-runtime/blob/8312ccd3e507aa8683b3705f2ca2d9862183982b/packages/cookies/src/serialize.ts#L42-L44parseSetCookiefunction again calls thedecodeURIComponentfunction on line 75: https://github.com/vercel/edge-runtime/blob/8312ccd3e507aa8683b3705f2ca2d9862183982b/packages/cookies/src/serialize.ts#L73-L76
This double invocation of decodeURIComponent throws an error and crashes the application if the cookie contains special characters.
⚠️ No Changeset found
Latest commit: 6dcc952c1f9fed22d8b61c5cd29eac91e5a81c39
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
@abhi12299 is attempting to deploy a commit to the Vercel Team on Vercel.
A member of the Team first needs to authorize it.
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 |
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.
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: https://github.com/vercel/edge-runtime/blob/37333f530635771e42c5cc81ca64534c432f2c29/packages/cookies/src/serialize.ts#L21
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?
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?