diff --git a/packages/next/src/build/webpack/loaders/next-flight-loader/action-client-wrapper.ts b/packages/next/src/build/webpack/loaders/next-flight-loader/action-client-wrapper.ts index c447392c8462a..c8f9a92ce0ef5 100644 --- a/packages/next/src/build/webpack/loaders/next-flight-loader/action-client-wrapper.ts +++ b/packages/next/src/build/webpack/loaders/next-flight-loader/action-client-wrapper.ts @@ -1,7 +1,10 @@ // This file must be bundled in the app's client layer, it shouldn't be directly // imported by the server. -export { callServer } from 'next/dist/client/app-call-server' +export { + callServer, + createBoundActionCallServer, +} from 'next/dist/client/app-call-server' export { findSourceMapURL } from 'next/dist/client/app-find-source-map-url' // A noop wrapper to let the Flight client create the server reference. diff --git a/packages/next/src/build/webpack/loaders/next-flight-server-reference-proxy-loader.ts b/packages/next/src/build/webpack/loaders/next-flight-server-reference-proxy-loader.ts index 07ade3302b83f..7d175a4154327 100644 --- a/packages/next/src/build/webpack/loaders/next-flight-server-reference-proxy-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-flight-server-reference-proxy-loader.ts @@ -18,10 +18,10 @@ const flightServerReferenceProxyLoader: webpack.LoaderDefinitionFunction<{ // Because of that, Webpack is able to concatenate the modules and inline the // reference IDs recursively directly into the module that uses them. return `\ -import { createServerReference, callServer, findSourceMapURL } from 'private-next-rsc-action-client-wrapper' +import { createServerReference, createBoundActionCallServer, findSourceMapURL } from 'private-next-rsc-action-client-wrapper' export ${ name === 'default' ? 'default' : `const ${name} =` - } /*#__PURE__*/createServerReference(${JSON.stringify(id)}, callServer, undefined, findSourceMapURL, ${JSON.stringify(name)})` + } /*#__PURE__*/createServerReference(${JSON.stringify(id)}, /*#__PURE__*/createBoundActionCallServer(), undefined, findSourceMapURL, ${JSON.stringify(name)})` } export default flightServerReferenceProxyLoader diff --git a/packages/next/src/client/app-call-server.ts b/packages/next/src/client/app-call-server.ts index c447e163828af..4c2188cd65b20 100644 --- a/packages/next/src/client/app-call-server.ts +++ b/packages/next/src/client/app-call-server.ts @@ -2,13 +2,32 @@ import { startTransition } from 'react' import { ACTION_SERVER_ACTION } from './components/router-reducer/router-reducer-types' import { dispatchAppRouterAction } from './components/use-action-queue' -export async function callServer(actionId: string, actionArgs: any[]) { +function getCurrentActionDispatchPath() { + if (typeof window === 'undefined') { + return undefined + } + return window.location.pathname + window.location.search +} + +export function createBoundActionCallServer( + actionDispatchPath = getCurrentActionDispatchPath() +) { + return (actionId: string, actionArgs: any[]) => + callServer(actionId, actionArgs, actionDispatchPath) +} + +export async function callServer( + actionId: string, + actionArgs: any[], + actionDispatchPath = getCurrentActionDispatchPath() +) { return new Promise((resolve, reject) => { startTransition(() => { dispatchAppRouterAction({ type: ACTION_SERVER_ACTION, actionId, actionArgs, + actionDispatchPath, resolve, reject, }) diff --git a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts index 1a24d4612ed98..973a18c7b269f 100644 --- a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts @@ -91,13 +91,22 @@ type FetchServerActionResult = { actionFlightDataRenderedSearch: NormalizedSearch | undefined isPrerender: boolean couldBeIntercepted: boolean + isCrossRouteDispatch: boolean } async function fetchServerAction( state: ReadonlyReducerState, nextUrl: ReadonlyReducerState['nextUrl'], - { actionId, actionArgs }: ServerActionAction + { actionId, actionArgs, actionDispatchPath }: ServerActionAction ): Promise { + const currentRequestUrl = new URL(state.canonicalUrl, window.location.href) + const actionRequestUrl = actionDispatchPath + ? new URL(actionDispatchPath, window.location.href) + : currentRequestUrl + const isCrossRouteDispatch = + actionRequestUrl.pathname !== currentRequestUrl.pathname || + actionRequestUrl.search !== currentRequestUrl.search + const temporaryReferences = createTemporaryReferenceSet() const info = extractInfoFromServerReferenceId(actionId) const usedArgs = omitUnusedArgs(actionArgs, info) @@ -133,7 +142,11 @@ async function fetchServerAction( .toString(16) } - const res = await fetch(state.canonicalUrl, { method: 'POST', headers, body }) + const res = await fetch(createHrefFromUrl(actionRequestUrl), { + method: 'POST', + headers, + body, + }) // Handle server actions that the server didn't recognize. const unrecognizedActionHeader = res.headers.get(NEXT_ACTION_NOT_FOUND_HEADER) @@ -239,6 +252,7 @@ async function fetchServerAction( revalidationKind, isPrerender, couldBeIntercepted, + isCrossRouteDispatch, } } @@ -277,7 +291,16 @@ export function serverActionReducer( redirectType, isPrerender, couldBeIntercepted, + isCrossRouteDispatch, }) => { + // A delayed action might be dispatched to a different route than the one + // currently rendered. In that case, don't apply the seeded response tree + // to the current route. + if (isCrossRouteDispatch && redirectLocation === undefined) { + flightData = undefined + flightDataRenderedSearch = undefined + } + if (revalidationKind !== ActionDidNotRevalidate) { // There was either a revalidation or a refresh, or maybe both. diff --git a/packages/next/src/client/components/router-reducer/router-reducer-types.ts b/packages/next/src/client/components/router-reducer/router-reducer-types.ts index 4d79397a25982..1ce88150a5319 100644 --- a/packages/next/src/client/components/router-reducer/router-reducer-types.ts +++ b/packages/next/src/client/components/router-reducer/router-reducer-types.ts @@ -53,6 +53,7 @@ export interface ServerActionAction { type: typeof ACTION_SERVER_ACTION actionId: string actionArgs: any[] + actionDispatchPath?: string resolve: (value: any) => void reject: (reason?: any) => void didRevalidate?: boolean diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts index b183998acd190..af30db3e059ee 100644 --- a/packages/next/src/server/app-render/action-handler.ts +++ b/packages/next/src/server/app-render/action-handler.ts @@ -40,7 +40,6 @@ import { import { getModifiedCookieValues } from '../web/spec-extension/adapters/request-cookies' import { - JSON_CONTENT_TYPE_HEADER, NEXT_CACHE_REVALIDATED_TAGS_HEADER, NEXT_CACHE_REVALIDATE_TAG_TOKEN_HEADER, } from '../../lib/constants' @@ -51,7 +50,7 @@ import { RequestCookies, ResponseCookies } from '../web/spec-extension/cookies' import { HeadersAdapter } from '../web/spec-extension/adapters/headers' import { fromNodeOutgoingHttpHeaders } from '../web/utils' import { - selectWorkerForForwarding, + hasActionWorkerForPage, type ServerModuleMap, getServerActionsManifest, getServerModuleMap, @@ -197,98 +196,6 @@ function addRevalidationHeader( } } -/** - * Forwards a server action request to a separate worker. Used when the requested action is not available in the current worker. - */ -async function createForwardedActionResponse( - req: BaseNextRequest, - res: BaseNextResponse, - host: Host, - workerPathname: string, - basePath: string -) { - if (!host) { - throw new Error( - 'Invariant: Missing `host` header from a forwarded Server Actions request.' - ) - } - - const forwardedHeaders = getForwardedHeaders(req, res) - - // indicate that this action request was forwarded from another worker - // we use this to skip rendering the flight tree so that we don't update the UI - // with the response from the forwarded worker - forwardedHeaders.set('x-action-forwarded', '1') - - const proto = - getRequestMeta(req, 'initProtocol')?.replace(/:+$/, '') || 'https' - - // For standalone or the serverful mode, use the internal origin directly - // other than the host headers from the request. - const origin = process.env.__NEXT_PRIVATE_ORIGIN || `${proto}://${host.value}` - - const fetchUrl = new URL(`${origin}${basePath}${workerPathname}`) - - try { - let body: BodyInit | ReadableStream | undefined - if ( - // The type check here ensures that `req` is correctly typed, and the - // environment variable check provides dead code elimination. - process.env.NEXT_RUNTIME === 'edge' && - isWebNextRequest(req) - ) { - if (!req.body) { - throw new Error('Invariant: missing request body.') - } - - body = req.body - } else if ( - // The type check here ensures that `req` is correctly typed, and the - // environment variable check provides dead code elimination. - process.env.NEXT_RUNTIME !== 'edge' && - isNodeNextRequest(req) - ) { - body = req.stream() - } else { - throw new Error('Invariant: Unknown request type.') - } - - // Forward the request to the new worker - const response = await fetch(fetchUrl, { - method: 'POST', - body, - duplex: 'half', - headers: forwardedHeaders, - redirect: 'manual', - next: { - // @ts-ignore - internal: 1, - }, - }) - - if ( - response.headers.get('content-type')?.startsWith(RSC_CONTENT_TYPE_HEADER) - ) { - // copy the headers from the redirect response to the response we're sending - for (const [key, value] of response.headers) { - if (!actionsForbiddenHeaders.includes(key)) { - res.setHeader(key, value) - } - } - - return new FlightRenderResult(response.body!) - } else { - // Since we aren't consuming the response body, we cancel it to avoid memory leaks - response.body?.cancel() - } - } catch (err) { - // we couldn't stream the forwarded response, so we'll just return an empty response - console.error(`failed to forward action response`, err) - } - - return RenderResult.fromStatic('{}', JSON_CONTENT_TYPE_HEADER) -} - /** * Returns the parsed redirect URL if we deem that it is hosted by us. * @@ -701,25 +608,10 @@ export async function handleAction({ const { actionAsyncStorage } = ComponentMod - const actionWasForwarded = Boolean(req.headers['x-action-forwarded']) - - if (actionId) { - const forwardedWorker = selectWorkerForForwarding(actionId, page) - - // If forwardedWorker is truthy, it means there isn't a worker for the action - // in the current handler, so we forward the request to a worker that has the action. - if (forwardedWorker) { - return { - type: 'done', - result: await createForwardedActionResponse( - req, - res, - host, - forwardedWorker, - ctx.renderOpts.basePath - ), - } - } + // Server Actions fetch requests are only valid on routes that bundle the + // action. Reject invalid route/action combinations before reading the body. + if (actionId && isFetchAction && !hasActionWorkerForPage(actionId, page)) { + return handleUnrecognizedFetchAction(getActionNotFoundError(actionId)) } try { @@ -791,8 +683,7 @@ export async function handleAction({ action as () => Promise, [], workStore, - requestStore, - actionWasForwarded + requestStore ) const formState = await decodeFormState( @@ -999,8 +890,7 @@ export async function handleAction({ action as () => Promise, [], workStore, - requestStore, - actionWasForwarded + requestStore ) const formState = await decodeFormState( @@ -1128,8 +1018,7 @@ export async function handleAction({ actionHandler, boundActionArguments, workStore, - requestStore, - actionWasForwarded + requestStore ).finally(() => { addRevalidationHeader(res, { workStore, requestStore }) if (logInfo) { @@ -1259,12 +1148,10 @@ export async function handleAction({ type: 'done', result: await generateFlight(req, ctx, requestStore, { actionResult: promise, - // If the page was not revalidated, or if the action was forwarded - // from another worker, we can skip rendering the page. + // If the page was not revalidated, we can skip rendering the page. skipPageRendering: workStore.pathWasRevalidated === undefined || - workStore.pathWasRevalidated === ActionDidNotRevalidate || - actionWasForwarded, + workStore.pathWasRevalidated === ActionDidNotRevalidate, temporaryReferences, }), } @@ -1287,14 +1174,13 @@ async function executeActionAndPrepareForRender< action: TFn, args: Parameters, workStore: WorkStore, - requestStore: RequestStore, - actionWasForwarded: boolean + requestStore: RequestStore ): Promise<{ actionResult: Awaited> skipPageRendering: boolean }> { requestStore.phase = 'action' - let skipPageRendering = actionWasForwarded + let skipPageRendering = false if (args.length > SERVER_ACTION_ARGS_LIMIT) { throw new Error( @@ -1307,8 +1193,7 @@ async function executeActionAndPrepareForRender< action.apply(null, args) ) - // If the page was not revalidated, or if the action was forwarded from - // another worker, we can skip rendering the page. + // If the page was not revalidated, we can skip rendering the page. skipPageRendering ||= workStore.pathWasRevalidated === undefined || workStore.pathWasRevalidated === ActionDidNotRevalidate diff --git a/packages/next/src/server/app-render/manifests-singleton.ts b/packages/next/src/server/app-render/manifests-singleton.ts index 5df89c9881e27..938547b9343c3 100644 --- a/packages/next/src/server/app-render/manifests-singleton.ts +++ b/packages/next/src/server/app-render/manifests-singleton.ts @@ -4,7 +4,6 @@ import type { DeepReadonly } from '../../shared/lib/deep-readonly' import { InvariantError } from '../../shared/lib/invariant-error' import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths' import { pathHasPrefix } from '../../shared/lib/router/utils/path-has-prefix' -import { removePathPrefix } from '../../shared/lib/router/utils/remove-path-prefix' import { workAsyncStorage } from './work-async-storage.external' export interface ServerModuleMap { @@ -235,40 +234,21 @@ function normalizeWorkerPageName(pageName: string) { return 'app' + pageName } -/** - * Converts a bundlePath (relative path to the entrypoint) to a routable page - * name. - */ -function denormalizeWorkerPageName(bundlePath: string) { - return normalizeAppPath(removePathPrefix(bundlePath, 'app')) -} - -/** - * Checks if the requested action has a worker for the current page. - * If not, it returns the first worker that has a handler for the action. - */ -export function selectWorkerForForwarding( +export function hasActionWorkerForPage( actionId: string, pageName: string -): string | undefined { +): boolean { const serverActionsManifest = getServerActionsManifest() const workers = serverActionsManifest[ process.env.NEXT_RUNTIME === 'edge' ? 'edge' : 'node' ][actionId]?.workers - // There are no workers to handle this action, nothing to forward to. if (!workers) { - return - } - - // If there is an entry for the current page, we don't need to forward. - if (workers[normalizeWorkerPageName(pageName)]) { - return + return false } - // Otherwise, grab the first worker that has a handler for this action id. - return denormalizeWorkerPageName(Object.keys(workers)[0]) + return Boolean(workers[normalizeWorkerPageName(pageName)]) } export function setManifestsSingleton({ diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts index 84d145cbf6874..8310b6766b4dd 100644 --- a/test/e2e/app-dir/actions/app-action.test.ts +++ b/test/e2e/app-dir/actions/app-action.test.ts @@ -897,10 +897,23 @@ describe('app-dir action handling', () => { } it.each(['node', 'edge'])( - 'should forward action request to a worker that contains the action handler (%s)', + 'should dispatch delayed action request to a route that contains the action handler (%s)', async (runtime) => { const cliOutputIndex = next.cliOutput.length - const browser = await next.browser(`/delayed-action/${runtime}`) + let actionRequestPathname: string | undefined + + const browser = await next.browser(`/delayed-action/${runtime}`, { + beforePageLoad(page) { + page.on('request', async (request) => { + if (request.method() !== 'POST') return + + const headers = await request.allHeaders().catch(() => ({})) + if (!headers['next-action']) return + + actionRequestPathname = new URL(request.url()).pathname + }) + }, + }) // confirm there's no data yet expect(await browser.elementById('delayed-action-result').text()).toBe( @@ -929,6 +942,9 @@ describe('app-dir action handling', () => { // make sure that we still are rendering other-page content expect(await browser.hasElementByCssSelector('#other-page')).toBe(true) + // delayed actions should dispatch against the original route that has the action + expect(actionRequestPathname).toBe(`/delayed-action/${runtime}`) + // make sure we didn't get any errors in the console expect(next.cliOutput.slice(cliOutputIndex)).not.toContain( 'Failed to find Server Action' @@ -937,11 +953,21 @@ describe('app-dir action handling', () => { ) it.each(['node', 'edge'])( - 'should not error when a forwarded action triggers a redirect (%s)', + 'should not error when a delayed action triggers a redirect (%s)', async (runtime) => { let redirectResponseCode + let actionRequestPathname: string | undefined const browser = await next.browser(`/delayed-action/${runtime}`, { beforePageLoad(page) { + page.on('request', async (request) => { + if (request.method() !== 'POST') return + + const headers = await request.allHeaders().catch(() => ({})) + if (!headers['next-action']) return + + actionRequestPathname = new URL(request.url()).pathname + }) + page.on('response', async (res) => { const headers = await res.allHeaders().catch(() => ({})) if (headers['x-action-redirect']) { @@ -962,14 +988,59 @@ describe('app-dir action handling', () => { // confirm a successful response code on the redirected action await retry(async () => { - expect(redirectResponseCode).toBe(200) + expect([200, 303]).toContain(redirectResponseCode) }) + // delayed redirects should dispatch against the original route that has the action + expect(actionRequestPathname).toBe(`/delayed-action/${runtime}`) + // confirm that the redirect was handled await browser.waitForElementByCss('#run-action-redirect') } ) + it.each(['node', 'edge'])( + 'should reject action requests posted to a route that does not bundle the action (%s)', + async (runtime) => { + let actionId: string | undefined + const browser = await next.browser(`/delayed-action/${runtime}`, { + beforePageLoad(page) { + page.on('request', async (request) => { + if (request.method() !== 'POST') return + + const headers = await request.allHeaders().catch(() => ({})) + if (!headers['next-action']) return + + actionId = headers['next-action'] + }) + }, + }) + + await browser.elementById('run-action').click() + await retry(async () => { + expect(actionId).toBeDefined() + }) + + if (!actionId) { + throw new Error('Failed to capture action ID') + } + + const res = await next.fetch(`/delayed-action/${runtime}/other`, { + method: 'POST', + headers: { + 'next-action': actionId, + 'next-resume': '1', + 'content-type': 'text/plain', + }, + body: 'test', + signal: AbortSignal.timeout(3_000), + }) + + expect(res.status).toBe(404) + expect(res.headers.get('x-nextjs-action-not-found')).toBe('1') + } + ) + if (isNextStart) { it('should not expose action content in sourcemaps', async () => { // We check all sourcemaps in the `static` folder for sensitive information given that chunking