Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set cookies followed by redirect() #49965

Merged
merged 13 commits into from
May 19, 2023
17 changes: 11 additions & 6 deletions packages/next/src/client/components/redirect.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { requestAsyncStorage } from './request-async-storage'
import type { ResponseCookies } from '../../server/web/spec-extension/cookies'

const REDIRECT_ERROR_CODE = 'NEXT_REDIRECT'

export enum RedirectType {
Expand All @@ -7,18 +10,20 @@ export enum RedirectType {

type RedirectError<U extends string> = Error & {
digest: `${typeof REDIRECT_ERROR_CODE};${RedirectType};${U}`
mutableCookies: ResponseCookies
}

export function getRedirectError(
url: string,
type: RedirectType
): RedirectError<typeof url> {
// eslint-disable-next-line no-throw-literal
const error = new Error(REDIRECT_ERROR_CODE)
;(
error as RedirectError<typeof url>
).digest = `${REDIRECT_ERROR_CODE};${type};${url}`
return error as RedirectError<typeof url>
const error = new Error(REDIRECT_ERROR_CODE) as RedirectError<typeof url>
error.digest = `${REDIRECT_ERROR_CODE};${type};${url}`
const requestStore = requestAsyncStorage.getStore()
if (requestStore) {
error.mutableCookies = requestStore.mutableCookies
}
return error
}

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { FlightRenderResult } from './flight-render-result'
import { ActionResult } from './types'
import { ActionAsyncStorage } from '../../client/components/action-async-storage'
import { filterReqHeaders, forbiddenHeaders } from '../lib/server-ipc/utils'
import { getMutableCookieHeaders } from '../future/route-modules/app-route/helpers/get-mutable-cookie-headers'

function nodeToWebReadableStream(nodeReadable: import('stream').Readable) {
if (process.env.NEXT_RUNTIME !== 'edge') {
Expand Down Expand Up @@ -377,6 +378,18 @@ export async function handleAction({
)
}

if (err.mutableCookies) {
const headers = getMutableCookieHeaders(
new Headers(),
err.mutableCookies
)
for (let [key, value] of headers) {
if (!forbiddenHeaders.includes(key)) {
res.setHeader(key, value)
}
}
}

res.setHeader('Location', redirectUrl)
res.statusCode = 303
return new RenderResult('')
Expand Down
13 changes: 13 additions & 0 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ import {
import { handleAction } from './action-handler'
import { NEXT_DYNAMIC_NO_SSR_CODE } from '../../shared/lib/lazy-dynamic/no-ssr-error'
import { warn } from '../../build/output/log'
import { getMutableCookieHeaders } from '../future/route-modules/app-route/helpers/get-mutable-cookie-headers'
import { forbiddenHeaders } from '../lib/server-ipc/utils'

export const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'

Expand Down Expand Up @@ -1505,6 +1507,17 @@ export async function renderToHTMLOrFlight(
}
if (isRedirectError(err)) {
res.statusCode = 307
if (err.mutableCookies) {
const headers = getMutableCookieHeaders(
new Headers(),
Copy link
Member Author

@styfle styfle May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if headers could exist here.

Do we want to do HeadersAdapter.from(req.headers) here just in case?

err.mutableCookies
)
for (let [key, value] of headers) {
if (!forbiddenHeaders.includes(key)) {
styfle marked this conversation as resolved.
Show resolved Hide resolved
res.setHeader(key, value)
}
}
}
res.setHeader('Location', getURLFromRedirectError(err))
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { HeadersAdapter } from '../../../../web/spec-extension/adapters/headers'
import { ResponseCookies } from '../../../../web/spec-extension/cookies'
import { SYMBOL_MODIFY_COOKIE_VALUES } from '../../../../web/spec-extension/adapters/request-cookies'

export function getMutableCookieHeaders(
headers: Headers,
mutableCookies: ResponseCookies
): Headers {
const modifiedCookieValues = (mutableCookies as any)[
SYMBOL_MODIFY_COOKIE_VALUES
] as NonNullable<ReturnType<InstanceType<typeof ResponseCookies>['get']>>[]
if (modifiedCookieValues.length) {
// Return a new response that extends the response with
// the modified cookies as fallbacks. `res`' cookies
// will still take precedence.
const resCookies = new ResponseCookies(HeadersAdapter.from(headers))
const returnedCookies = resCookies.getAll()

// Set the modified cookies as fallbacks.
for (const cookie of modifiedCookieValues) {
resCookies.set(cookie)
}
// Set the original cookies as the final values.
for (const cookie of returnedCookies) {
resCookies.set(cookie)
}

const responseHeaders = new Headers({})
// Set all the headers except for the cookies.
headers.forEach((value, key) => {
if (key.toLowerCase() !== 'set-cookie') {
responseHeaders.append(key, value)
}
})
// Set the final cookies, need to append cookies one
// at a time otherwise it might not work in some browsers.
resCookies.getAll().forEach((cookie) => {
const tempCookies = new ResponseCookies(new Headers())
tempCookies.set(cookie)
responseHeaders.append('Set-Cookie', tempCookies.toString())
})
return responseHeaders
}
return headers
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function resolveHandlerError(err: any): Response | false {
}

// This is a redirect error! Send the redirect response.
return handleTemporaryRedirectResponse(redirect)
return handleTemporaryRedirectResponse(redirect, err.mutableCookies)
}

if (isNotFoundError(err)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ import { RouteKind } from '../../route-kind'
import * as Log from '../../../../build/output/log'
import { autoImplementMethods } from './helpers/auto-implement-methods'
import { getNonStaticMethods } from './helpers/get-non-static-methods'
import { SYMBOL_MODIFY_COOKIE_VALUES } from '../../../web/spec-extension/adapters/request-cookies'
import { ResponseCookies } from '../../../web/spec-extension/cookies'
import { HeadersAdapter } from '../../../web/spec-extension/adapters/headers'
import { PrerenderManifest } from '../../../../build'
import { getMutableCookieHeaders } from './helpers/get-mutable-cookie-headers'

/**
* AppRouteRouteHandlerContext is the context that is passed to the route
Expand Down Expand Up @@ -358,54 +356,17 @@ export class AppRouteRouteModule extends RouteModule<
// It's possible cookies were set in the handler, so we need
// to merge the modified cookies and the returned response
// here.
// TODO: Move this into a helper function.
const requestStore = this.requestAsyncStorage.getStore()
if (requestStore && requestStore.mutableCookies) {
const modifiedCookieValues = (
requestStore.mutableCookies as any
)[SYMBOL_MODIFY_COOKIE_VALUES] as NonNullable<
ReturnType<InstanceType<typeof ResponseCookies>['get']>
>[]
if (modifiedCookieValues.length) {
// Return a new response that extends the response with
// the modified cookies as fallbacks. `res`' cookies
// will still take precedence.
const resCookies = new ResponseCookies(
HeadersAdapter.from(res.headers)
)
const returnedCookies = resCookies.getAll()

// Set the modified cookies as fallbacks.
for (const cookie of modifiedCookieValues) {
resCookies.set(cookie)
}
// Set the original cookies as the final values.
for (const cookie of returnedCookies) {
resCookies.set(cookie)
}

const responseHeaders = new Headers({})
// Set all the headers except for the cookies.
res.headers.forEach((value, key) => {
if (key.toLowerCase() !== 'set-cookie') {
responseHeaders.append(key, value)
}
})
// Set the final cookies, need to append cookies one
// at a time otherwise it might not work in some browsers.
resCookies.getAll().forEach((cookie) => {
const tempCookies = new ResponseCookies(new Headers())
tempCookies.set(cookie)
responseHeaders.append(
'Set-Cookie',
tempCookies.toString()
)
})

const headers = getMutableCookieHeaders(
res.headers,
requestStore.mutableCookies
)
if (headers !== res.headers) {
return new Response(res.body, {
status: res.status,
statusText: res.statusText,
headers: responseHeaders,
headers,
})
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
export function handleTemporaryRedirectResponse(url: string): Response {
import { ResponseCookies } from '../../../web/spec-extension/cookies'
import { getMutableCookieHeaders } from '../app-route/helpers/get-mutable-cookie-headers'

export function handleTemporaryRedirectResponse(
url: string,
mutableCookies: ResponseCookies
): Response {
const headers = getMutableCookieHeaders(new Headers(), mutableCookies)
headers.set('location', url)

return new Response(null, {
status: 302,
statusText: 'Found',
headers: {
location: url,
},
headers,
})
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { draftMode } from 'next/headers'
import { redirect } from 'next/navigation'

export function GET() {
draftMode().enable()
return redirect('/some-other-page')
}
11 changes: 11 additions & 0 deletions test/e2e/app-dir/draft-mode/draft-mode-edge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ createNextDescribe(
expect(Cookie).toBeDefined()
})

it('should have set-cookie header with redirect location', async () => {
const res = await next.fetch('/enable-and-redirect', {
redirect: 'manual',
})
expect(res.status).toBe(302)
expect(res.headers.get('location')).toContain('/some-other-page')
const h = res.headers.get('set-cookie') || ''
const c = h.split(';').find((c) => c.startsWith('__prerender_bypass'))
expect(c).toBeDefined()
})

it('should be enabled from page when draft mode enabled', async () => {
const opts = { headers: { Cookie } }
const $ = await next.render$('/', {}, opts)
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/app-dir/draft-mode/draft-mode-node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ createNextDescribe(
expect(Cookie).toBeDefined()
})

it('should have set-cookie header with redirect location', async () => {
const res = await next.fetch('/enable-and-redirect', {
redirect: 'manual',
})
expect(res.status).toBe(302)
expect(res.headers.get('location')).toContain('/some-other-page')
const h = res.headers.get('set-cookie') || ''
const c = h.split(';').find((c) => c.startsWith('__prerender_bypass'))
expect(c).toBeDefined()
})

it('should genenerate rand when draft mode enabled', async () => {
const opts = { headers: { Cookie } }
const $ = await next.render$('/', {}, opts)
Expand Down