Skip to content

Commit

Permalink
Fix module-level Server Action creation with closure-closed values (#…
Browse files Browse the repository at this point in the history
…62437)

With Server Actions, a module-level encryption can happen when you do:

```js
function wrapAction(value) {
  return async function () {
    'use server'
    console.log(value)
  }
}

const action = wrapAction('some-module-level-encryption-value')
```

...as that action will be created when requiring this module, and it
contains an encrypted argument from its closure (`value`). This
currently throws an error during build:

```
Error: Missing manifest for Server Actions. This is a bug in Next.js
    at d (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/chunks/1772.js:1:15202)
    at f (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/chunks/1772.js:1:16917)
    at 714 (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/app/encryption/page.js:1:2806)
    at t (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/webpack-runtime.js:1:127)
    at 7940 (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/app/encryption/page.js:1:941)
    at t (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/webpack-runtime.js:1:127)
    at r (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/app/encryption/page.js:1:4529)
    at /Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/app/encryption/page.js:1:4572
    at t.X (/Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/webpack-runtime.js:1:1181)
    at /Users/shu/Documents/git/next.js/test/e2e/app-dir/actions/.next/server/app/encryption/page.js:1:4542
```

Because during module require phase, the encryption logic can't run as
it doesn't have Server/Client references available yet (which are set
during the rendering phase).

Since both references are global singletons to the server and are
already loaded early, this fix makes sure that they're registered via
`setReferenceManifestsSingleton` before requiring the module.

Closes NEXT-2579
  • Loading branch information
shuding authored and MaxLeiter committed Feb 28, 2024
1 parent 6e59c22 commit 51c6a07
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 26 deletions.
13 changes: 13 additions & 0 deletions packages/next/src/build/templates/edge-ssr-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import type { BuildManifest } from '../../server/get-page-files'
import type { RequestData } from '../../server/web/types'
import type { NextConfigComplete } from '../../server/config-shared'
import { PAGE_TYPES } from '../../lib/page-types'
import { setReferenceManifestsSingleton } from '../../server/app-render/action-encryption-utils'
import { createServerModuleMap } from '../../server/app-render/action-utils'

declare const incrementalCacheHandler: any
// OPTIONAL_IMPORT:incrementalCacheHandler
Expand Down Expand Up @@ -44,6 +46,17 @@ const subresourceIntegrityManifest = sriEnabled
: undefined
const nextFontManifest = maybeJSONParse(self.__NEXT_FONT_MANIFEST)

if (rscManifest && rscServerManifest) {
setReferenceManifestsSingleton({
clientReferenceManifest: rscManifest,
serverActionsManifest: rscServerManifest,
serverModuleMap: createServerModuleMap({
serverActionsManifest: rscServerManifest,
pageName: 'VAR_PAGE',
}),
})
}

const render = getRender({
pagesType: PAGE_TYPES.APP,
dev,
Expand Down
28 changes: 28 additions & 0 deletions packages/next/src/server/app-render/action-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import type { ActionManifest } from '../../build/webpack/plugins/flight-client-entry-plugin'

// This function creates a Flight-acceptable server module map proxy from our
// Server Reference Manifest similar to our client module map.
// This is because our manifest contains a lot of internal Next.js data that
// are relevant to the runtime, workers, etc. that React doesn't need to know.
export function createServerModuleMap({
serverActionsManifest,
pageName,
}: {
serverActionsManifest: ActionManifest
pageName: string
}) {
return new Proxy(
{},
{
get: (_, id: string) => {
return {
id: serverActionsManifest[
process.env.NEXT_RUNTIME === 'edge' ? 'edge' : 'node'
][id].workers['app' + pageName],
name: id,
chunks: [],
}
},
}
)
}
26 changes: 5 additions & 21 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import { DetachedPromise } from '../../lib/detached-promise'
import { isDynamicServerError } from '../../client/components/hooks-server-context'
import { useFlightResponse } from './use-flight-response'
import { isStaticGenBailoutError } from '../../client/components/static-generation-bailout'
import { createServerModuleMap } from './action-utils'

export type GetDynamicParamFromSegment = (
// [slug] / [[slug]] / [...slug]
Expand Down Expand Up @@ -586,27 +587,10 @@ async function renderToHTMLOrFlightImpl(
// TODO: fix this typescript
const clientReferenceManifest = renderOpts.clientReferenceManifest!

const workerName = 'app' + renderOpts.page
const serverModuleMap: {
[id: string]: {
id: string
chunks: string[]
name: string
}
} = new Proxy(
{},
{
get: (_, id: string) => {
return {
id: serverActionsManifest[
process.env.NEXT_RUNTIME === 'edge' ? 'edge' : 'node'
][id].workers[workerName],
name: id,
chunks: [],
}
},
}
)
const serverModuleMap = createServerModuleMap({
serverActionsManifest,
pageName: renderOpts.page,
})

setReferenceManifestsSingleton({
clientReferenceManifest,
Expand Down
30 changes: 25 additions & 5 deletions packages/next/src/server/load-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
} from 'next/types'
import type { RouteModule } from './future/route-modules/route-module'
import type { BuildManifest } from './get-page-files'
import type { ActionManifest } from '../build/webpack/plugins/flight-client-entry-plugin'

import {
BUILD_MANIFEST,
Expand All @@ -26,6 +27,9 @@ import { getTracer } from './lib/trace/tracer'
import { LoadComponentsSpan } from './lib/trace/constants'
import { evalManifest, loadManifest } from './load-manifest'
import { wait } from '../lib/wait'
import { setReferenceManifestsSingleton } from './app-render/action-encryption-utils'
import { createServerModuleMap } from './app-render/action-utils'

export type ManifestItem = {
id: number | string
files: string[]
Expand Down Expand Up @@ -132,15 +136,13 @@ async function loadComponentsImpl<N = any>({
Promise.resolve().then(() => requirePage('/_app', distDir, false)),
])
}
const ComponentMod = await Promise.resolve().then(() =>
requirePage(page, distDir, isAppPath)
)

// Make sure to avoid loading the manifest for Route Handlers
const hasClientManifest =
isAppPath &&
(page.endsWith('/page') || page === '/not-found' || page === '/_not-found')

// Load the manifest files first
const [
buildManifest,
reactLoadableManifest,
Expand All @@ -165,12 +167,30 @@ async function loadComponentsImpl<N = any>({
)
: undefined,
isAppPath
? loadManifestWithRetries(
? (loadManifestWithRetries(
join(distDir, 'server', SERVER_REFERENCE_MANIFEST + '.json')
).catch(() => null)
).catch(() => null) as Promise<ActionManifest | null>)
: null,
])

// Before requring the actual page module, we have to set the reference manifests
// to our global store so Server Action's encryption util can access to them
// at the top level of the page module.
if (serverActionsManifest && clientReferenceManifest) {
setReferenceManifestsSingleton({
clientReferenceManifest,
serverActionsManifest,
serverModuleMap: createServerModuleMap({
serverActionsManifest,
pageName: page,
}),
})
}

const ComponentMod = await Promise.resolve().then(() =>
requirePage(page, distDir, isAppPath)
)

const Component = interopDefault(ComponentMod)
const Document = interopDefault(DocumentMod)
const App = interopDefault(AppMod)
Expand Down
1 change: 1 addition & 0 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,7 @@ createNextDescribe(
const res = await next.fetch('/encryption')
const html = await res.text()
expect(html).not.toContain('qwerty123')
expect(html).not.toContain('some-module-level-encryption-value')
})
})

Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/actions/app/encryption/page.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
// Test top-level encryption (happens during the module load phase)
function wrapAction(value) {
return async function () {
'use server'
console.log(value)
}
}

const action = wrapAction('some-module-level-encryption-value')

// Test runtime encryption (happens during the rendering phase)
export default function Page() {
const secret = 'my password is qwerty123'

Expand All @@ -6,6 +17,7 @@ export default function Page() {
action={async () => {
'use server'
console.log(secret)
await action()
return 'success'
}}
>
Expand Down

0 comments on commit 51c6a07

Please sign in to comment.