Fix cookie merging in Server Action redirections (#61113)
This PR fixes the cookie merging logic in Server Actions. Specifically, when users do `cookies().set(...)` or `cookies.delete(...)` with a `redirect()` to an internal route followed. Currently, we are just concatenating the original cookies (request cookies) and the mutated cookies. That introduces several issues, like it can't override or delete an existing cookie. Closes NEXT-2221
This commit is contained in:
parent
7eab3cdbe9
commit
b7cc5b9baa
4 changed files with 78 additions and 13 deletions
|
@ -42,6 +42,9 @@ import {
|
|||
} from '../lib/server-action-request-meta'
|
||||
import { isCsrfOriginAllowed } from './csrf-protection'
|
||||
import { warn } from '../../build/output/log'
|
||||
import { RequestCookies, ResponseCookies } from '../web/spec-extension/cookies'
|
||||
import { HeadersAdapter } from '../web/spec-extension/adapters/headers'
|
||||
import { fromNodeOutgoingHttpHeaders } from '../web/utils'
|
||||
|
||||
function formDataFromSearchQueryString(query: string) {
|
||||
const searchParams = new URLSearchParams(query)
|
||||
|
@ -70,18 +73,13 @@ function getForwardedHeaders(
|
|||
): Headers {
|
||||
// Get request headers and cookies
|
||||
const requestHeaders = req.headers
|
||||
const requestCookies = requestHeaders['cookie'] ?? ''
|
||||
const requestCookies = new RequestCookies(HeadersAdapter.from(requestHeaders))
|
||||
|
||||
// Get response headers and Set-Cookie header
|
||||
// Get response headers and cookies
|
||||
const responseHeaders = res.getHeaders()
|
||||
const rawSetCookies = responseHeaders['set-cookie']
|
||||
const setCookies = (
|
||||
Array.isArray(rawSetCookies) ? rawSetCookies : [rawSetCookies]
|
||||
).map((setCookie) => {
|
||||
// remove the suffixes like 'HttpOnly' and 'SameSite'
|
||||
const [cookie] = `${setCookie}`.split(';', 1)
|
||||
return cookie
|
||||
})
|
||||
const responseCookies = new ResponseCookies(
|
||||
fromNodeOutgoingHttpHeaders(responseHeaders)
|
||||
)
|
||||
|
||||
// Merge request and response headers
|
||||
const mergedHeaders = filterReqHeaders(
|
||||
|
@ -92,11 +90,18 @@ function getForwardedHeaders(
|
|||
actionsForbiddenHeaders
|
||||
) as Record<string, string>
|
||||
|
||||
// Merge cookies
|
||||
const mergedCookies = requestCookies.split('; ').concat(setCookies).join('; ')
|
||||
// Merge cookies into requestCookies, so responseCookies always take precedence
|
||||
// and overwrite/delete those from requestCookies.
|
||||
responseCookies.getAll().forEach((cookie) => {
|
||||
if (typeof cookie.value === 'undefined') {
|
||||
requestCookies.delete(cookie.name)
|
||||
} else {
|
||||
requestCookies.set(cookie)
|
||||
}
|
||||
})
|
||||
|
||||
// Update the 'cookie' header with the merged cookies
|
||||
mergedHeaders['cookie'] = mergedCookies
|
||||
mergedHeaders['cookie'] = requestCookies.toString()
|
||||
|
||||
// Remove headers that should not be forwarded
|
||||
delete mergedHeaders['transfer-encoding']
|
||||
|
|
|
@ -1048,6 +1048,27 @@ createNextDescribe(
|
|||
expect(responseCodes).toEqual([303])
|
||||
})
|
||||
|
||||
it('merges cookies correctly when redirecting', async () => {
|
||||
const browser = await next.browser('/redirects/action-redirect')
|
||||
|
||||
// set foo and bar to be both 1, and verify
|
||||
await browser.eval(
|
||||
`document.cookie = 'bar=1; Path=/'; document.cookie = 'foo=1; Path=/';`
|
||||
)
|
||||
await browser.refresh()
|
||||
expect(await browser.elementByCss('h1').text()).toBe('foo=1; bar=1')
|
||||
|
||||
// delete foo and set bar to 2, redirect
|
||||
await browser.elementById('redirect-with-cookie-mutation').click()
|
||||
await check(
|
||||
() => browser.url(),
|
||||
/\/redirects\/action-redirect\/redirect-target/
|
||||
)
|
||||
|
||||
// verify that the cookies were merged correctly
|
||||
expect(await browser.elementByCss('h1').text()).toBe('foo=; bar=2')
|
||||
})
|
||||
|
||||
it.each(['307', '308'])(
|
||||
`redirects properly when server action handler redirects with a %s status code`,
|
||||
async (statusCode) => {
|
||||
|
|
|
@ -0,0 +1,26 @@
|
|||
import { cookies } from 'next/headers'
|
||||
import { redirect } from 'next/navigation'
|
||||
|
||||
export default function Page() {
|
||||
const foo = cookies().get('foo')
|
||||
const bar = cookies().get('bar')
|
||||
return (
|
||||
<div>
|
||||
<h1>
|
||||
foo={foo ? foo.value : ''}; bar={bar ? bar.value : ''}
|
||||
</h1>
|
||||
<form
|
||||
action={async () => {
|
||||
'use server'
|
||||
cookies().delete('foo')
|
||||
cookies().set('bar', '2')
|
||||
redirect('/redirects/action-redirect/redirect-target')
|
||||
}}
|
||||
>
|
||||
<button type="submit" id="redirect-with-cookie-mutation">
|
||||
Set Cookies and Redirect
|
||||
</button>
|
||||
</form>
|
||||
</div>
|
||||
)
|
||||
}
|
|
@ -0,0 +1,13 @@
|
|||
import { cookies } from 'next/headers'
|
||||
|
||||
export default function Page() {
|
||||
const foo = cookies().get('foo')
|
||||
const bar = cookies().get('bar')
|
||||
return (
|
||||
<div>
|
||||
<h1>
|
||||
foo={foo ? foo.value : ''}; bar={bar ? bar.value : ''}
|
||||
</h1>
|
||||
</div>
|
||||
)
|
||||
}
|
Loading…
Reference in a new issue