Skip to content

Commit

Permalink
Disallow rootParams() in cache scope (vercel#75801)
Browse files Browse the repository at this point in the history
root params exposes request data and thus must be disallowed from caches
unless the cache also implicitly keys the entries on the root params
values. For now we will simply disallow this pattern. we will implement
a strategy in the future which will allow root params to be called from
inside a cache scope once we can sufficiently key the cache entries by
the root params values read. The same problem will arise with variants
in the future so we will eventually need to implement support

The implementation removes rootParams from the workStore. This is an
invitation to accidentally leak request data into cache scopes. Now
rootParams are conveyed on the workUnitStore and the cache store types
don't define it.

closes NAR-89
  • Loading branch information
gnoff authored Feb 14, 2025
1 parent eb66a4d commit a8d36dd
Show file tree
Hide file tree
Showing 19 changed files with 325 additions and 71 deletions.
6 changes: 5 additions & 1 deletion packages/next/errors.json
Original file line number Diff line number Diff line change
Expand Up @@ -636,5 +636,9 @@
"635": "%s: Invalid source map. Only conformant source maps can be used to find the original code.",
"636": "Invariant: frames must be a function when the React version has React.use. This is a bug in Next.js.",
"637": "Invariant: frames must be an array when the React version does not have React.use. This is a bug in Next.js.",
"638": "The following tags are missing in the Root Layout: %s.\\nRead more at https://nextjs.org/docs/messages/missing-root-layout-tags"
"638": "The following tags are missing in the Root Layout: %s.\\nRead more at https://nextjs.org/docs/messages/missing-root-layout-tags",
"639": "unstable_rootParams() can only be used within App Router.",
"640": "Route %s used \"unstable_rootParams\" inside \\`\"use cache\"\\` or \\`unstable_cache\\`. Support for this API inside cache scopes is planned for a future version of Next.js.",
"641": "Route %s used \\`unstable_rootParams()\\` in Pages Router. This API is only available within App Router.",
"642": "Route %s used \\`unstable_rootParams()\\` inside \\`\"use cache\"\\` or \\`unstable_cache\\`. Support for this API inside cache scopes is planned for a future version of Next.js."
}
31 changes: 30 additions & 1 deletion packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ import { getRequiredScripts } from './required-scripts'
import { addPathPrefix } from '../../shared/lib/router/utils/add-path-prefix'
import { makeGetServerInsertedHTML } from './make-get-server-inserted-html'
import { walkTreeWithFlightRouterState } from './walk-tree-with-flight-router-state'
import { createComponentTree } from './create-component-tree'
import { createComponentTree, getRootParams } from './create-component-tree'
import { getAssetQueryString } from './get-asset-query-string'
import { setReferenceManifestsSingleton } from './encryption-utils'
import {
Expand Down Expand Up @@ -673,6 +673,11 @@ async function warmupDevRender(
)
}

const rootParams = getRootParams(
ctx.componentMod.tree,
ctx.getDynamicParamFromSegment
)

function onFlightDataRenderError(err: DigestedError) {
return renderOpts.onInstrumentationRequestError?.(
err,
Expand All @@ -696,6 +701,7 @@ async function warmupDevRender(
const prerenderStore: PrerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: [],
renderSignal: renderController.signal,
controller: prerenderController,
Expand Down Expand Up @@ -1441,10 +1447,12 @@ async function renderToHTMLOrFlightImpl(
renderOpts.devRenderResumeDataCache ??
postponedState?.renderResumeDataCache

const rootParams = getRootParams(loaderTree, ctx.getDynamicParamFromSegment)
const requestStore = createRequestStoreForRender(
req,
res,
url,
rootParams,
implicitTags,
renderOpts.onUpdateCookies,
renderOpts.previewProps,
Expand Down Expand Up @@ -2175,6 +2183,10 @@ async function spawnDynamicValidationInDev(
requestStore: RequestStore
): Promise<void> {
const { componentMod: ComponentMod } = ctx
const rootParams = getRootParams(
ComponentMod.tree,
ctx.getDynamicParamFromSegment
)

// Prerender controller represents the lifetime of the prerender.
// It will be aborted when a Task is complete or a synchronously aborting
Expand All @@ -2192,6 +2204,7 @@ async function spawnDynamicValidationInDev(
const initialServerPrerenderStore: PrerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: [],
renderSignal: initialServerRenderController.signal,
controller: initialServerPrerenderController,
Expand All @@ -2208,6 +2221,7 @@ async function spawnDynamicValidationInDev(
const initialClientPrerenderStore: PrerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: [],
renderSignal: initialClientController.signal,
controller: initialClientController,
Expand Down Expand Up @@ -2353,6 +2367,7 @@ async function spawnDynamicValidationInDev(
const finalServerPrerenderStore: PrerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: [],
renderSignal: finalServerController.signal,
controller: finalServerController,
Expand All @@ -2373,6 +2388,7 @@ async function spawnDynamicValidationInDev(
const finalClientPrerenderStore: PrerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: [],
renderSignal: finalClientController.signal,
controller: finalClientController,
Expand Down Expand Up @@ -2543,6 +2559,7 @@ async function prerenderToStream(
// because some shared APIs expect a formState value and this is slightly
// more explicit than making it an optional function argument
const formState = null
const rootParams = getRootParams(tree, ctx.getDynamicParamFromSegment)

const renderOpts = ctx.renderOpts
const ComponentMod = renderOpts.ComponentMod
Expand Down Expand Up @@ -2691,6 +2708,7 @@ async function prerenderToStream(
const initialServerPrerenderStore: PrerenderStore = (prerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: initialServerRenderController.signal,
controller: initialServerPrerenderController,
Expand Down Expand Up @@ -2784,6 +2802,7 @@ async function prerenderToStream(
const initialClientPrerenderStore: PrerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: initialClientController.signal,
controller: initialClientController,
Expand Down Expand Up @@ -2868,6 +2887,7 @@ async function prerenderToStream(
const finalRenderPrerenderStore: PrerenderStore = (prerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: finalServerController.signal,
controller: finalServerController,
Expand Down Expand Up @@ -2936,6 +2956,7 @@ async function prerenderToStream(
const finalClientPrerenderStore: PrerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: finalClientController.signal,
controller: finalClientController,
Expand Down Expand Up @@ -3170,6 +3191,7 @@ async function prerenderToStream(
const initialServerPrerenderStore: PrerenderStore = (prerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: initialServerRenderController.signal,
controller: initialServerPrerenderController,
Expand All @@ -3186,6 +3208,7 @@ async function prerenderToStream(
const initialClientPrerenderStore: PrerenderStore = (prerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: initialClientController.signal,
controller: initialClientController,
Expand Down Expand Up @@ -3337,6 +3360,7 @@ async function prerenderToStream(
const finalServerPrerenderStore: PrerenderStore = (prerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: finalServerController.signal,
controller: finalServerController,
Expand All @@ -3360,6 +3384,7 @@ async function prerenderToStream(
const finalClientPrerenderStore: PrerenderStore = (prerenderStore = {
type: 'prerender',
phase: 'render',
rootParams,
implicitTags: implicitTags,
renderSignal: finalClientController.signal,
controller: finalClientController,
Expand Down Expand Up @@ -3554,6 +3579,7 @@ async function prerenderToStream(
const reactServerPrerenderStore: PrerenderStore = (prerenderStore = {
type: 'prerender-ppr',
phase: 'render',
rootParams,
implicitTags: implicitTags,
dynamicTracking,
revalidate: INFINITE_CACHE,
Expand Down Expand Up @@ -3586,6 +3612,7 @@ async function prerenderToStream(
const ssrPrerenderStore: PrerenderStore = {
type: 'prerender-ppr',
phase: 'render',
rootParams,
implicitTags: implicitTags,
dynamicTracking,
revalidate: INFINITE_CACHE,
Expand Down Expand Up @@ -3775,6 +3802,7 @@ async function prerenderToStream(
const prerenderLegacyStore: PrerenderStore = (prerenderStore = {
type: 'prerender-legacy',
phase: 'render',
rootParams,
implicitTags: implicitTags,
revalidate: INFINITE_CACHE,
expire: INFINITE_CACHE,
Expand Down Expand Up @@ -3934,6 +3962,7 @@ async function prerenderToStream(
const prerenderLegacyStore: PrerenderStore = (prerenderStore = {
type: 'prerender-legacy',
phase: 'render',
rootParams,
implicitTags: implicitTags,
revalidate:
typeof prerenderStore?.revalidate !== 'undefined'
Expand Down
58 changes: 53 additions & 5 deletions packages/next/src/server/app-render/create-component-tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { getLayoutOrPageModule } from '../lib/app-dir-module'
import type { LoaderTree } from '../lib/app-dir-module'
import { interopDefault } from './interop-default'
import { parseLoaderTree } from './parse-loader-tree'
import type { AppRenderContext } from './app-render'
import type { AppRenderContext, GetDynamicParamFromSegment } from './app-render'
import { createComponentStylesAndScripts } from './create-component-styles-and-scripts'
import { getLayerAssets } from './get-layer-assets'
import { hasLoadingComponentInTree } from './has-loading-component-in-tree'
Expand Down Expand Up @@ -392,10 +392,6 @@ async function createComponentTreeInternal({
// Resolve the segment param
const actualSegment = segmentParam ? segmentParam.treeSegment : segment

if (rootLayoutAtThisLevel) {
workStore.rootParams = currentParams
}

// Only render metadata on the actual SSR'd segment not the `default` segment,
// as it's used as a placeholder for navigation.
const metadata =
Expand Down Expand Up @@ -921,3 +917,55 @@ function createErrorBoundaryClientSegmentRoot({
}
return null
}

export function getRootParams(
loaderTree: LoaderTree,
getDynamicParamFromSegment: GetDynamicParamFromSegment
): Params {
return getRootParamsImpl({}, loaderTree, getDynamicParamFromSegment)
}

function getRootParamsImpl(
parentParams: Params,
loaderTree: LoaderTree,
getDynamicParamFromSegment: GetDynamicParamFromSegment
): Params {
const {
segment,
modules: { layout },
parallelRoutes,
} = parseLoaderTree(loaderTree)

const segmentParam = getDynamicParamFromSegment(segment)

let currentParams: Params = parentParams
if (segmentParam && segmentParam.value !== null) {
currentParams = {
...parentParams,
[segmentParam.param]: segmentParam.value,
}
}

const isRootLayout = typeof layout !== 'undefined'

if (isRootLayout) {
return currentParams
} else if (!parallelRoutes.children) {
// This should really be an error but there are bugs in Turbopack that cause
// the _not-found LoaderTree to not have any layouts. For rootParams sake
// this is somewhat irrelevant when you are not customizing the 404 page.
// If you are customizing 404
// TODO update rootParams to make all params optional if `/app/not-found.tsx` is defined
return currentParams
} else {
return getRootParamsImpl(
currentParams,
// We stop looking for root params as soon as we hit the first layout
// and it is not possible to use parallel route children above the root layout
// so every parallelRoutes object that this function can visit will necessarily
// have a single `children` prop and no others.
parallelRoutes.children,
getDynamicParamFromSegment
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type { DeepReadonly } from '../../shared/lib/deep-readonly'
import type { AppSegmentConfig } from '../../build/segment-config/app/app-segment-config'
import type { AfterContext } from '../after/after-context'
import type { CacheLife } from '../use-cache/cache-life'
import type { Params } from '../request/params'

// Share the instance module in the next-shared layer
import { workAsyncStorageInstance } from './work-async-storage-instance' with { 'turbopack-transition': 'next-shared' }
Expand Down Expand Up @@ -71,7 +70,6 @@ export interface WorkStore {
>
readonly assetPrefix?: string

rootParams: Params
dynamicIOEnabled: boolean
dev: boolean
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
RenderResumeDataCache,
PrerenderResumeDataCache,
} from '../resume-data-cache/resume-data-cache'
import type { Params } from '../request/params'

type WorkUnitPhase = 'action' | 'render' | 'after'

Expand Down Expand Up @@ -52,6 +53,7 @@ export type RequestStore = {
readonly serverComponentsHmrCache?: ServerComponentsHmrCache

readonly implicitTags: string[]
readonly rootParams: Params

/**
* The resume data cache for this request. This will be a immutable cache.
Expand Down Expand Up @@ -100,6 +102,8 @@ export type PrerenderStoreModern = {
*/
readonly dynamicTracking: null | DynamicTrackingState

readonly rootParams: Params

// Collected revalidate times and tags for this document during the prerender.
revalidate: number // in seconds. 0 means dynamic. INFINITE_CACHE and higher means never revalidate.
expire: number // server expiration time
Expand All @@ -120,6 +124,7 @@ export type PrerenderStoreModern = {

export type PrerenderStorePPR = {
type: 'prerender-ppr'
readonly rootParams: Params
readonly implicitTags: string[]
readonly dynamicTracking: null | DynamicTrackingState
// Collected revalidate times and tags for this document during the prerender.
Expand All @@ -136,6 +141,7 @@ export type PrerenderStorePPR = {

export type PrerenderStoreLegacy = {
type: 'prerender-legacy'
readonly rootParams: Params
readonly implicitTags: string[]
// Collected revalidate times and tags for this document during the prerender.
revalidate: number // in seconds. 0 means dynamic. INFINITE_CACHE and higher means never revalidate.
Expand Down
6 changes: 6 additions & 0 deletions packages/next/src/server/async-storage/request-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { DraftModeProvider } from './draft-mode-provider'
import { splitCookiesString } from '../web/utils'
import type { ServerComponentsHmrCache } from '../response-cache'
import type { RenderResumeDataCache } from '../resume-data-cache/resume-data-cache'
import type { Params } from '../request/params'

function getHeaders(headers: Headers | IncomingHttpHeaders): ReadonlyHeaders {
const cleaned = HeadersAdapter.from(headers)
Expand Down Expand Up @@ -106,6 +107,7 @@ export function createRequestStoreForRender(
req: RequestContext['req'],
res: RequestContext['res'],
url: RequestContext['url'],
rootParams: Params,
implicitTags: RequestContext['implicitTags'],
onUpdateCookies: RenderOpts['onUpdateCookies'],
previewProps: WrapperRenderOpts['previewProps'],
Expand All @@ -119,6 +121,7 @@ export function createRequestStoreForRender(
req,
res,
url,
rootParams,
implicitTags,
onUpdateCookies,
renderResumeDataCache,
Expand All @@ -141,6 +144,7 @@ export function createRequestStoreForAPI(
req,
undefined,
url,
{},
implicitTags,
onUpdateCookies,
undefined,
Expand All @@ -155,6 +159,7 @@ function createRequestStoreImpl(
req: RequestContext['req'],
res: RequestContext['res'],
url: RequestContext['url'],
rootParams: Params,
implicitTags: RequestContext['implicitTags'],
onUpdateCookies: RenderOpts['onUpdateCookies'],
renderResumeDataCache: RenderResumeDataCache | undefined,
Expand Down Expand Up @@ -184,6 +189,7 @@ function createRequestStoreImpl(
// to ensure we don't use parts of the URL that we shouldn't. This also
// lets us avoid requiring an empty string for `search` in the type.
url: { pathname: url.pathname, search: url.search ?? '' },
rootParams,
get headers() {
if (!cache.headers) {
// Seal the headers object that'll freeze out any methods that could
Expand Down
2 changes: 0 additions & 2 deletions packages/next/src/server/async-storage/work-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ export function createWorkStore({

isDraftMode: renderOpts.isDraftMode,

rootParams: {},

requestEndedState,
isPrefetchRequest,
buildId,
Expand Down
Loading

0 comments on commit a8d36dd

Please sign in to comment.