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
11 changes: 11 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 { appendMutableCookies } from '../web/spec-extension/adapters/request-cookies'

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

if (err.mutableCookies) {
const headers = new Headers()

// If there were mutable cookies set, we need to set them on the
// response.
if (appendMutableCookies(headers, err.mutableCookies)) {
res.setHeader('set-cookie', Array.from(headers.values()))
}
}

res.setHeader('Location', redirectUrl)
res.statusCode = 303
return new RenderResult('')
Expand Down
10 changes: 10 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,7 @@ 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 { appendMutableCookies } from '../web/spec-extension/adapters/request-cookies'

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

Expand Down Expand Up @@ -1505,6 +1506,15 @@ export async function renderToHTMLOrFlight(
}
if (isRedirectError(err)) {
res.statusCode = 307
if (err.mutableCookies) {
const headers = new Headers()

// If there were mutable cookies set, we need to set them on the
// response.
if (appendMutableCookies(headers, err.mutableCookies)) {
res.setHeader('set-cookie', Array.from(headers.values()))
}
}
res.setHeader('Location', getURLFromRedirectError(err))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function getMutableCookies(
res: ServerResponse | BaseNextResponse | undefined
): ResponseCookies {
const cookies = new RequestCookies(HeadersAdapter.from(headers))
return MutableRequestCookiesAdapter.seal(cookies, res)
return MutableRequestCookiesAdapter.wrap(cookies, res)
}

export type RequestContext = {
Expand Down
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 { appendMutableCookies } from '../../../web/spec-extension/adapters/request-cookies'

/**
* AppRouteRouteHandlerContext is the context that is passed to the route
Expand Down Expand Up @@ -358,54 +356,19 @@ 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 headers = new Headers(res.headers)
if (
appendMutableCookies(
headers,
requestStore.mutableCookies
)
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()
)
})

) {
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,37 +1,29 @@
export function handleTemporaryRedirectResponse(url: string): Response {
return new Response(null, {
status: 302,
statusText: 'Found',
headers: {
location: url,
},
})
import { appendMutableCookies } from '../../../web/spec-extension/adapters/request-cookies'
import { ResponseCookies } from '../../../web/spec-extension/cookies'

export function handleTemporaryRedirectResponse(
url: string,
mutableCookies: ResponseCookies
): Response {
const headers = new Headers({ location: url })

appendMutableCookies(headers, mutableCookies)

return new Response(null, { status: 307, headers })
}

export function handleBadRequestResponse(): Response {
return new Response(null, {
status: 400,
statusText: 'Bad Request',
})
return new Response(null, { status: 400 })
}

export function handleNotFoundResponse(): Response {
return new Response(null, {
status: 404,
statusText: 'Not Found',
})
return new Response(null, { status: 404 })
}

export function handleMethodNotAllowedResponse(): Response {
return new Response(null, {
status: 405,
statusText: 'Method Not Allowed',
})
return new Response(null, { status: 405 })
}

export function handleInternalServerErrorResponse(): Response {
return new Response(null, {
status: 500,
statusText: 'Internal Server Error',
})
return new Response(null, { status: 500 })
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,53 @@ export class RequestCookiesAdapter {
}
}

export const SYMBOL_MODIFY_COOKIE_VALUES = Symbol.for('next.mutated.cookies')
const SYMBOL_MODIFY_COOKIE_VALUES = Symbol.for('next.mutated.cookies')

function getModifiedCookieValues(cookies: ResponseCookies): ResponseCookie[] {
const modified: ResponseCookie[] | undefined = (cookies as unknown as any)[
SYMBOL_MODIFY_COOKIE_VALUES
]
if (!modified || !Array.isArray(modified) || modified.length === 0) {
return []
}

return modified
}

export function appendMutableCookies(
headers: Headers,
mutableCookies: ResponseCookies
): boolean {
const modifiedCookieValues = getModifiedCookieValues(mutableCookies)
if (modifiedCookieValues.length === 0) {
return false
}

// Return a new response that extends the response with
// the modified cookies as fallbacks. `res`' cookies
// will still take precedence.
const resCookies = new ResponseCookies(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)
}

return true
}

type ResponseCookie = NonNullable<
ReturnType<InstanceType<typeof ResponseCookies>['get']>
>

export class MutableRequestCookiesAdapter {
public static seal(
public static wrap(
cookies: RequestCookies,
res: ServerResponse | BaseNextResponse | undefined
): ResponseCookies {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/app-dir/app-routes/app-custom-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ createNextDescribe(
redirect: 'manual',
})

expect(res.status).toEqual(302)
expect(res.status).toEqual(307)
expect(res.headers.get('location')).toEqual('https://nextjs.org/')
expect(await res.text()).toBeEmpty()
})
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(307)
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(307)
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