Skip to content

Commit fb67605

Browse files
authored
fix: remove useless set-cookie in action-handler (#76839)
`handleAction` had a weird spot where it'd add a `set-cookie` header if a redirect was thrown during handling a no-js action. AFAICT, this is unnecessary -- we set the `set-cookie` header on the response as soon as `cookies().set()` is called, so there's no need to set the cookies again in that branch of the handler. this is done through a combination of the patched setter from `RequestCookies`: https://github.com/vercel/next.js/blob/0abd87717f19bced6c35af1255837628513cf772/packages/next/src/server/web/spec-extension/adapters/request-cookies.ts#L160-L171 and this cookie updating function from the request store (which `updateResponseCookies` above would invoke with the new cookies): https://github.com/vercel/next.js/blob/0abd87717f19bced6c35af1255837628513cf772/packages/next/src/server/async-storage/request-store.ts#L170-L174 i've also added a test for this specific scenario to ensure that this wasn't actually doing anything useful.
1 parent 2d7d3c9 commit fb67605

File tree

4 files changed

+52
-11
lines changed

4 files changed

+52
-11
lines changed

packages/next/src/server/app-render/action-handler.ts

+1-11
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ import {
3030
filterReqHeaders,
3131
actionsForbiddenHeaders,
3232
} from '../lib/server-ipc/utils'
33-
import {
34-
appendMutableCookies,
35-
getModifiedCookieValues,
36-
} from '../web/spec-extension/adapters/request-cookies'
33+
import { getModifiedCookieValues } from '../web/spec-extension/adapters/request-cookies'
3734

3835
import {
3936
NEXT_CACHE_REVALIDATED_TAGS_HEADER,
@@ -976,13 +973,6 @@ export async function handleAction({
976973
}
977974
}
978975

979-
// If there were mutable cookies set, we need to set them on the
980-
// response.
981-
const headers = new Headers()
982-
if (appendMutableCookies(headers, requestStore.mutableCookies)) {
983-
res.setHeader('set-cookie', Array.from(headers.values()))
984-
}
985-
986976
res.setHeader('Location', redirectUrl)
987977
return {
988978
type: 'done',

test/e2e/app-dir/actions/app-action.test.ts

+21
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,27 @@ describe('app-dir action handling', () => {
175175
})
176176
})
177177

178+
it.each([
179+
{ description: 'with javascript', disableJavaScript: false },
180+
{ description: 'no javascript', disableJavaScript: true },
181+
])(
182+
'should support setting cookies when redirecting ($description)',
183+
async ({ disableJavaScript }) => {
184+
const browser = await next.browser('/mutate-cookie-with-redirect', {
185+
disableJavaScript,
186+
})
187+
expect(await browser.elementByCss('#value').text()).toBe('')
188+
189+
await browser.elementByCss('#update-cookie').click()
190+
await browser.elementByCss('#redirect-target')
191+
192+
expect(await browser.elementByCss('#value').text()).toMatch(/\d+/)
193+
expect(await browser.eval('document.cookie')).toMatch(
194+
/(?:^|(?:; ))testCookie=\d+/
195+
)
196+
}
197+
)
198+
178199
it('should push new route when redirecting', async () => {
179200
const browser = await next.browser('/header')
180201

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { cookies } from 'next/headers'
2+
import { redirect } from 'next/navigation'
3+
4+
async function updateCookieAndRedirect() {
5+
'use server'
6+
;(await cookies()).set('testCookie', Date.now())
7+
redirect('/mutate-cookie-with-redirect/redirect-target')
8+
}
9+
10+
export default async function Page() {
11+
return (
12+
<>
13+
<p id="value">{(await cookies()).get('testCookie')?.value ?? null}</p>
14+
<form action={updateCookieAndRedirect}>
15+
<button id="update-cookie" type="submit">
16+
Update Cookie
17+
</button>
18+
</form>
19+
</>
20+
)
21+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { cookies } from 'next/headers'
2+
3+
export default async function Page() {
4+
return (
5+
<div id="redirect-target">
6+
<p id="value">{(await cookies()).get('testCookie')?.value ?? null}</p>
7+
</div>
8+
)
9+
}

0 commit comments

Comments
 (0)