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

update status codes for redirect and permanentRedirect in action handlers #58885

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ description: API Reference for the permanentRedirect function.

The `permanentRedirect` function allows you to redirect the user to another URL. `permanentRedirect` can be used in Server Components, Client Components, [Route Handlers](/docs/app/building-your-application/routing/route-handlers), and [Server Actions](/docs/app/building-your-application/data-fetching/forms-and-mutations).

When used in a streaming context, this will insert a meta tag to emit the redirect on the client side. Otherwise, it will serve a 308 (Permanent) HTTP redirect response to the caller.
When used in a streaming context, this will insert a meta tag to emit the redirect on the client side. When used in a server action, it will serve a 303 HTTP redirect response to the caller. Otherwise, it will serve a 308 (Permanent) HTTP redirect response to the caller.

If a resource doesn't exist, you can use the [`notFound` function](/docs/app/api-reference/functions/not-found) instead.

Expand Down
2 changes: 1 addition & 1 deletion docs/02-app/02-api-reference/04-functions/redirect.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ description: API Reference for the redirect function.

The `redirect` function allows you to redirect the user to another URL. `redirect` can be used in Server Components, Client Components, [Route Handlers](/docs/app/building-your-application/routing/route-handlers), and [Server Actions](/docs/app/building-your-application/data-fetching/forms-and-mutations).

When used in a [streaming context](/docs/app/building-your-application/routing/loading-ui-and-streaming#what-is-streaming), this will insert a meta tag to emit the redirect on the client side. Otherwise, it will serve a 307 HTTP redirect response to the caller.
When used in a [streaming context](/docs/app/building-your-application/routing/loading-ui-and-streaming#what-is-streaming), this will insert a meta tag to emit the redirect on the client side. When used in a server action, it will serve a 303 HTTP redirect response to the caller. Otherwise, it will serve a 307 HTTP redirect response to the caller.

If a resource doesn't exist, you can use the [`notFound` function](/docs/app/api-reference/functions/not-found) instead.

Expand Down
50 changes: 38 additions & 12 deletions packages/next/src/client/components/redirect.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { requestAsyncStorage } from './request-async-storage.external'
import type { ResponseCookies } from '../../server/web/spec-extension/cookies'
import { actionAsyncStorage } from './action-async-storage.external'
import { RedirectStatusCode } from '../../shared/lib/constants'

const REDIRECT_ERROR_CODE = 'NEXT_REDIRECT'

Expand All @@ -9,17 +11,17 @@ export enum RedirectType {
}

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

export function getRedirectError(
url: string,
type: RedirectType,
permanent: boolean = false
statusCode: RedirectStatusCode = RedirectStatusCode.TemporaryRedirect
): RedirectError<typeof url> {
const error = new Error(REDIRECT_ERROR_CODE) as RedirectError<typeof url>
error.digest = `${REDIRECT_ERROR_CODE};${type};${url};${permanent}`
error.digest = `${REDIRECT_ERROR_CODE};${type};${url};${statusCode};`
const requestStore = requestAsyncStorage.getStore()
if (requestStore) {
error.mutableCookies = requestStore.mutableCookies
Expand All @@ -30,29 +32,49 @@ export function getRedirectError(
/**
* When used in a streaming context, this will insert a meta tag to
* redirect the user to the target page. When used in a custom app route, it
* will serve a 307 to the caller.
* will serve a 307/303 to the caller.
*
* @param url the url to redirect to
*/
export function redirect(
url: string,
type: RedirectType = RedirectType.replace
): never {
throw getRedirectError(url, type, false)
const actionStore = actionAsyncStorage.getStore()
throw getRedirectError(
url,
type,
// If we're in an action, we want to use a 303 redirect
// as we don't want the POST request to follow the redirect,
// as it could result in erroneous re-submissions.
actionStore?.isAction
? RedirectStatusCode.SeeOther
: RedirectStatusCode.TemporaryRedirect
)
}

/**
* When used in a streaming context, this will insert a meta tag to
* redirect the user to the target page. When used in a custom app route, it
* will serve a 308 to the caller.
* will serve a 308/303 to the caller.
*
* @param url the url to redirect to
*/
export function permanentRedirect(
url: string,
type: RedirectType = RedirectType.replace
): never {
throw getRedirectError(url, type, true)
const actionStore = actionAsyncStorage.getStore()
throw getRedirectError(
url,
type,
// If we're in an action, we want to use a 303 redirect
// as we don't want the POST request to follow the redirect,
// as it could result in erroneous re-submissions.
actionStore?.isAction
? RedirectStatusCode.SeeOther
: RedirectStatusCode.PermanentRedirect
)
}

/**
Expand All @@ -67,15 +89,19 @@ export function isRedirectError<U extends string>(
): error is RedirectError<U> {
if (typeof error?.digest !== 'string') return false

const [errorCode, type, destination, permanent] = (
error.digest as string
).split(';', 4)
const [errorCode, type, destination, status] = (error.digest as string).split(
';',
4
)

const statusCode = Number(status)

return (
errorCode === REDIRECT_ERROR_CODE &&
(type === 'replace' || type === 'push') &&
typeof destination === 'string' &&
(permanent === 'true' || permanent === 'false')
!isNaN(statusCode) &&
statusCode in RedirectStatusCode
)
}

Expand Down Expand Up @@ -114,5 +140,5 @@ export function getRedirectStatusCodeFromError<U extends string>(
throw new Error('Not a redirect error')
}

return error.digest.split(';', 4)[3] === 'true' ? 308 : 307
return Number(error.digest.split(';', 4)[3])
}
9 changes: 4 additions & 5 deletions packages/next/src/lib/redirect-status.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import {
PERMANENT_REDIRECT_STATUS,
TEMPORARY_REDIRECT_STATUS,
} from '../shared/lib/constants'
import { RedirectStatusCode } from '../shared/lib/constants'

export const allowedStatusCodes = new Set([301, 302, 303, 307, 308])

Expand All @@ -11,7 +8,9 @@ export function getRedirectStatus(route: {
}): number {
return (
route.statusCode ||
(route.permanent ? PERMANENT_REDIRECT_STATUS : TEMPORARY_REDIRECT_STATUS)
(route.permanent
? RedirectStatusCode.PermanentRedirect
: RedirectStatusCode.TemporaryRedirect)
)
}

Expand Down
4 changes: 3 additions & 1 deletion packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from '../../client/components/app-router-headers'
import { isNotFoundError } from '../../client/components/not-found'
import {
getRedirectStatusCodeFromError,
getURLFromRedirectError,
isRedirectError,
} from '../../client/components/redirect'
Expand Down Expand Up @@ -574,6 +575,7 @@ To configure the body size limit for Server Actions, see: https://nextjs.org/doc
} catch (err) {
if (isRedirectError(err)) {
const redirectUrl = getURLFromRedirectError(err)
const statusCode = getRedirectStatusCodeFromError(err)

// if it's a fetch action, we don't want to mess with the status code
// and we'll handle it on the client router
Expand Down Expand Up @@ -605,7 +607,7 @@ To configure the body size limit for Server Actions, see: https://nextjs.org/doc
}

res.setHeader('Location', redirectUrl)
res.statusCode = 303
res.statusCode = statusCode
return {
type: 'done',
result: RenderResult.fromStatic(''),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from '../../client/components/redirect'
import { renderToReadableStream } from 'react-dom/server.edge'
import { streamToString } from '../stream-utils/node-web-streams-helper'
import { RedirectStatusCode } from '../../shared/lib/constants'

export function makeGetServerInsertedHTML({
polyfills,
Expand Down Expand Up @@ -38,8 +39,9 @@ export function makeGetServerInsertedHTML({
)
} else if (isRedirectError(error)) {
const redirectUrl = getURLFromRedirectError(error)
const statusCode = getRedirectStatusCodeFromError(error)
const isPermanent =
getRedirectStatusCodeFromError(error) === 308 ? true : false
statusCode === RedirectStatusCode.PermanentRedirect ? true : false
if (redirectUrl) {
errorMetaTags.push(
<meta
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/base-http/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { IncomingHttpHeaders, OutgoingHttpHeaders } from 'http'
import type { I18NConfig } from '../config-shared'

import { PERMANENT_REDIRECT_STATUS } from '../../shared/lib/constants'
import { RedirectStatusCode } from '../../shared/lib/constants'
import type { NextApiRequestCookies } from '../api-utils'
import { getCookieParser } from '../api-utils/get-cookie-parser'

Expand Down Expand Up @@ -84,7 +84,7 @@ export abstract class BaseNextResponse<Destination = any> {

// Since IE11 doesn't support the 308 header add backwards
// compatibility using refresh header
if (statusCode === PERMANENT_REDIRECT_STATUS) {
if (statusCode === RedirectStatusCode.PermanentRedirect) {
this.setHeader('Refresh', `0;url=${destination}`)
}
return this
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ import {
APP_PATHS_MANIFEST,
NEXT_BUILTIN_DOCUMENT,
PAGES_MANIFEST,
RedirectStatusCode,
STATIC_STATUS_PAGES,
TEMPORARY_REDIRECT_STATUS,
} from '../shared/lib/constants'
import { isDynamicRoute } from '../shared/lib/router/utils'
import { checkIsOnDemandRevalidate } from './api-utils'
Expand Down Expand Up @@ -1213,7 +1213,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {

if (redirect) {
return res
.redirect(redirect, TEMPORARY_REDIRECT_STATUS)
.redirect(redirect, RedirectStatusCode.TemporaryRedirect)
.body(redirect)
.send()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { requestAsyncStorage } from '../../../../client/components/request-async
import { staticGenerationAsyncStorage } from '../../../../client/components/static-generation-async-storage.external'
import { actionAsyncStorage } from '../../../../client/components/action-async-storage.external'
import * as sharedModules from './shared-modules'
import { getIsServerAction } from '../../../lib/server-action-request-meta'

/**
* The AppRouteModule is the type of the module exported by the bundled App
Expand Down Expand Up @@ -274,6 +275,7 @@ export class AppRouteRouteModule extends RouteModule<
const response: unknown = await this.actionAsyncStorage.run(
{
isAppRoute: true,
isAction: getIsServerAction(request),
},
() =>
RequestAsyncStorageWrapper.wrap(
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/lib/router-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { isPostpone } from './router-utils/is-postpone'
import {
PHASE_PRODUCTION_SERVER,
PHASE_DEVELOPMENT_SERVER,
PERMANENT_REDIRECT_STATUS,
RedirectStatusCode,
} from '../../shared/lib/constants'
import { DevBundlerService } from './dev-bundler-service'
import { type Span, trace } from '../../trace'
Expand Down Expand Up @@ -353,7 +353,7 @@ export async function initialize(opts: {
res.statusCode = statusCode
res.setHeader('location', destination)

if (statusCode === PERMANENT_REDIRECT_STATUS) {
if (statusCode === RedirectStatusCode.PermanentRedirect) {
res.setHeader('Refresh', `0;url=${destination}`)
}
return res.end(destination)
Expand Down
14 changes: 10 additions & 4 deletions packages/next/src/server/lib/server-action-request-meta.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { IncomingMessage } from 'http'
import type { BaseNextRequest } from '../base-http'
import type { NextRequest } from '../web/exports'
import { ACTION } from '../../client/components/app-router-headers'

export function getServerActionRequestMetadata(
req: IncomingMessage | BaseNextRequest
req: IncomingMessage | BaseNextRequest | NextRequest
): {
actionId: string | null
isURLEncodedAction: boolean
Expand All @@ -13,8 +14,13 @@ export function getServerActionRequestMetadata(
let actionId: string | null
let contentType: string | null

actionId = (req.headers[ACTION.toLowerCase()] as string) ?? null
contentType = req.headers['content-type'] ?? null
if (req.headers instanceof Headers) {
actionId = req.headers.get(ACTION.toLowerCase()) ?? null
contentType = req.headers.get('content-type')
} else {
actionId = (req.headers[ACTION.toLowerCase()] as string) ?? null
contentType = req.headers['content-type'] ?? null
}

const isURLEncodedAction = Boolean(
req.method === 'POST' && contentType === 'application/x-www-form-urlencoded'
Expand All @@ -32,7 +38,7 @@ export function getServerActionRequestMetadata(
}

export function getIsServerAction(
req: IncomingMessage | BaseNextRequest
req: IncomingMessage | BaseNextRequest | NextRequest
): boolean {
const { isFetchAction, isURLEncodedAction, isMultipartAction } =
getServerActionRequestMetadata(req)
Expand Down
8 changes: 6 additions & 2 deletions packages/next/src/shared/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ export const CLIENT_STATIC_FILES_RUNTIME_POLYFILLS_SYMBOL = Symbol(
CLIENT_STATIC_FILES_RUNTIME_POLYFILLS
)
export const EDGE_RUNTIME_WEBPACK = 'edge-runtime-webpack'
export const TEMPORARY_REDIRECT_STATUS = 307
export const PERMANENT_REDIRECT_STATUS = 308
export const STATIC_PROPS_ID = '__N_SSG'
export const SERVER_PROPS_ID = '__N_SSP'
export const PAGE_SEGMENT_KEY = '__PAGE__'
Expand Down Expand Up @@ -156,3 +154,9 @@ export const SYSTEM_ENTRYPOINTS = new Set<string>([
CLIENT_STATIC_FILES_RUNTIME_AMP,
CLIENT_STATIC_FILES_RUNTIME_MAIN_APP,
])

export enum RedirectStatusCode {
SeeOther = 303,
TemporaryRedirect = 307,
PermanentRedirect = 308,
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable jest/no-standalone-expect */
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
import type { Response } from 'playwright-chromium'

createNextDescribe(
'app-dir action progressive enhancement',
Expand All @@ -15,8 +16,18 @@ createNextDescribe(
},
({ next, isNextDev, isNextStart, isNextDeploy }) => {
it('should support formData and redirect without JS', async () => {
let responseCode
const browser = await next.browser('/server', {
disableJavaScript: true,
beforePageLoad(page) {
page.on('response', (response: Response) => {
const url = new URL(response.url())
const status = response.status()
if (url.pathname.includes('/server')) {
responseCode = status
}
})
},
})

await browser.eval(`document.getElementById('name').value = 'test'`)
Expand All @@ -25,6 +36,8 @@ createNextDescribe(
await check(() => {
return browser.eval('window.location.pathname + window.location.search')
}, '/header?name=test&constructor=_FormData&hidden-info=hi')

expect(responseCode).toBe(303)
})

it('should support actions from client without JS', async () => {
Expand Down
Loading