Skip to content

Commit

Permalink
feat(edge): allows configuring Dynamic code execution guard (#39539)
Browse files Browse the repository at this point in the history
### 📖  What's in there?

Dynamic code evaluation (`eval()`, `new Function()`, ...) is not
supported on the edge runtime, hence why we fail the build when
detecting such statement in the middleware or `experimental-edge` routes
at build time.

However, there could be false positives, which static analysis and
tree-shaking can not exclude:
- `qs` through these dependencies (get-intrinsic:
[source](https://github.com/ljharb/get-intrinsic/blob/main/index.js#L12))
- `function-bind`
([source](https://github.com/Raynos/function-bind/blob/master/implementation.js#L42))
- `has`
([source](https://github.com/tarruda/has/blob/master/src/index.js#L5))

This PR leverages the existing `config` export to let user allow some of
their files.
it’s meant for allowing users to import 3rd party modules who embed
dynamic code evaluation, but do not use it (because or code paths), and
can't be tree-shaked.

By default, it’s keeping the existing behavior: warn in dev, fails to
build.
If users allow dynamic code, and that code is reached at runtime, their
app stills breaks.

### 🧪 How to test?

- (existing) integration tests for disallowing dynamic code evaluation:
`pnpm testheadless --testPathPattern=runtime-dynamic`
- (new) integration tests for allowing dynamic code evaluation: `pnpm
testheadless --testPathPattern=runtime-configurable`
- (amended) production tests for validating the new configuration keys:
`pnpm testheadless --testPathPattern=config-validations`

To try it live, you could have an application such as:
```js
// lib/index.js
/* eslint-disable no-eval */
export function hasUnusedDynamic() {
  if ((() => false)()) {
    eval('100')
  }
}

export function hasDynamic() {
  eval('100')
}

// pages/index.jsx
export default function Page({ edgeRoute }) {
  return <p>{edgeRoute}</p>
}

export const getServerSideProps = async (req) => {
  const res = await fetch(`http://localhost:3000/api/route`)
  const data = await res.json()
  return { props: { edgeRoute: data.ok ? `Hi from the edge route` : '' } }
}

// pages/api/route.js
import { hasDynamic } from '../../lib'

export default async function handle() {
  hasDynamic()
  return Response.json({ ok: true })
}

export const config = { 
  runtime: 'experimental-edge' ,
  allowDynamic: '/lib/**'
}
```

Playing with `config.allowDynamic`, you should be able to:
- build the app even if it uses `eval()` (it will obviously fail at
runtime)
- build the app that _imports but does not use_ `eval()`
- run the app in dev, even if it uses `eval()` with no warning

### 🆙 Notes to reviewers

Before adding documentation and telemetry, I'd like to collect comments
on a couple of points:
- the overall design for this feature: is a list of globs useful and
easy enough?
- should the globs be relative to the application root (current
implementation) to to the edge route/middleware file?
- (especially to @sokra) is the implementation idiomatic enough? I've
leverage loaders to read the _entry point_ configuration once, then the
ModuleGraph to get it back during the parsing phase. I couldn't re-use
the existing `getExtractMetadata()` facility since it's happening late
after the parsing.
- there's a glitch with `import { ServerRuntime } from '../../types'` in
`get-page-static-info.ts`
([here](https://github.com/vercel/next.js/pull/39539/files#diff-cb7ac6392c3dd707c5edab159c3144ec114eafea92dad5d98f4eedfc612174d2L12)).
I had to use `next/types` because it was failing during lint. Any clue
why?

### ☑️ Checklist

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Documentation added
- [x] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
  • Loading branch information
feugy committed Sep 12, 2022
1 parent d83ceee commit 97ac344
Show file tree
Hide file tree
Showing 20 changed files with 706 additions and 76 deletions.
19 changes: 19 additions & 0 deletions docs/api-reference/edge-runtime.md
Expand Up @@ -136,6 +136,25 @@ The following JavaScript language features are disabled, and **will not work:**

- `eval`: Evaluates JavaScript code represented as a string
- `new Function(evalString)`: Creates a new function with the code provided as an argument
- `WebAssembly.compile`
- `WebAssembly.instantiate` with [a buffer parameter](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/instantiate#primary_overload_%E2%80%94_taking_wasm_binary_code)

In rare cases, your code could contain (or import) some dynamic code evaluation statements which _can not be reached at runtime_ and which can not be removed by treeshaking.
You can relax the check to allow specific files with your Middleware or Edge API Route exported configuration:

```javascript
export const config = {
runtime: 'experimental-edge', // for Edge API Routes only
allowDynamic: [
'/lib/utilities.js', // allows a single file
'/node_modules/function-bind/**', // use a glob to allow anything in the function-bind 3rd party module
],
}
```

`allowDynamic` is a [glob](https://github.com/micromatch/micromatch#matching-features), or an array of globs, ignoring dynamic code evaluation for specific files. The globs are relative to your application root folder.

Be warned that if these statements are executed on the Edge, _they will throw and cause a runtime error_.

## Related

Expand Down
34 changes: 34 additions & 0 deletions errors/edge-dynamic-code-evaluation.md
@@ -0,0 +1,34 @@
# Dynamic code evaluation is not available in Middlewares or Edge API Routes

#### Why This Error Occurred

`eval()`, `new Function()` or compiling WASM binaries dynamically is not allowed in Middlewares or Edge API Routes.
Specifically, the following APIs are not supported:

- `eval()`
- `new Function()`
- `WebAssembly.compile`
- `WebAssembly.instantiate` with [a buffer parameter](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/instantiate#primary_overload_%E2%80%94_taking_wasm_binary_code)

#### Possible Ways to Fix It

You can bundle your WASM binaries using `import`:

```typescript
import { NextResponse } from 'next/server'
import squareWasm from './square.wasm?module'

export default async function middleware() {
const m = await WebAssembly.instantiate(squareWasm)
const answer = m.exports.square(9)

const response = NextResponse.next()
response.headers.set('x-square', answer.toString())
return response
}
```

In rare cases, your code could contain (or import) some dynamic code evaluation statements which _can not be reached at runtime_ and which can not be removed by treeshaking.
You can relax the check to allow specific files with your Middleware or Edge API Route exported [configuration](https://nextjs.org/docs/api-reference/edge-runtime#unsupported-apis).

Be warned that if these statements are executed on the Edge, _they will throw and cause a runtime error_.
4 changes: 4 additions & 0 deletions errors/manifest.json
Expand Up @@ -714,6 +714,10 @@
"title": "middleware-dynamic-wasm-compilation",
"path": "/errors/middleware-dynamic-wasm-compilation.md"
},
{
"title": "edge-dynamic-code-evaluation",
"path": "/errors/edge-dynamic-code-evaluation.md"
},
{
"title": "node-module-in-edge-runtime",
"path": "/errors/node-module-in-edge-runtime.md"
Expand Down
2 changes: 1 addition & 1 deletion errors/middleware-dynamic-wasm-compilation.md
Expand Up @@ -19,8 +19,8 @@ import squareWasm from './square.wasm?module'
export default async function middleware() {
const m = await WebAssembly.instantiate(squareWasm)
const answer = m.exports.square(9)

const response = NextResponse.next()

response.headers.set('x-square', answer.toString())
return response
}
Expand Down
32 changes: 30 additions & 2 deletions packages/next/build/analysis/get-page-static-info.ts
Expand Up @@ -12,9 +12,11 @@ import * as Log from '../output/log'
import { SERVER_RUNTIME } from '../../lib/constants'
import { ServerRuntime } from 'next/types'
import { checkCustomRoutes } from '../../lib/load-custom-routes'
import { matcher } from 'next/dist/compiled/micromatch'

export interface MiddlewareConfig {
matchers: MiddlewareMatcher[]
allowDynamicGlobs: string[]
}

export interface MiddlewareMatcher {
Expand Down Expand Up @@ -162,6 +164,7 @@ function getMiddlewareMatchers(
}

function getMiddlewareConfig(
pageFilePath: string,
config: any,
nextConfig: NextConfig
): Partial<MiddlewareConfig> {
Expand All @@ -171,6 +174,23 @@ function getMiddlewareConfig(
result.matchers = getMiddlewareMatchers(config.matcher, nextConfig)
}

if (config.allowDynamic) {
result.allowDynamicGlobs = Array.isArray(config.allowDynamic)
? config.allowDynamic
: [config.allowDynamic]
for (const glob of result.allowDynamicGlobs ?? []) {
try {
matcher(glob)
} catch (err) {
throw new Error(
`${pageFilePath} exported 'config.allowDynamic' contains invalid pattern '${glob}': ${
(err as Error).message
}`
)
}
}
}

return result
}

Expand Down Expand Up @@ -223,7 +243,11 @@ export async function getPageStaticInfo(params: {
const { isDev, pageFilePath, nextConfig, page } = params

const fileContent = (await tryToReadFile(pageFilePath, !isDev)) || ''
if (/runtime|getStaticProps|getServerSideProps|matcher/.test(fileContent)) {
if (
/runtime|getStaticProps|getServerSideProps|matcher|allowDynamic/.test(
fileContent
)
) {
const swcAST = await parseModule(pageFilePath, fileContent)
const { ssg, ssr } = checkExports(swcAST)

Expand Down Expand Up @@ -268,7 +292,11 @@ export async function getPageStaticInfo(params: {
warnAboutExperimentalEdgeApiFunctions()
}

const middlewareConfig = getMiddlewareConfig(config, nextConfig)
const middlewareConfig = getMiddlewareConfig(
page ?? 'middleware/edge API route',
config,
nextConfig
)

return {
ssr,
Expand Down
7 changes: 6 additions & 1 deletion packages/next/build/entries.ts
Expand Up @@ -48,6 +48,7 @@ import { serverComponentRegex } from './webpack/loaders/utils'
import { ServerRuntime } from '../types'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
import { encodeMatchers } from './webpack/loaders/next-middleware-loader'
import { EdgeFunctionLoaderOptions } from './webpack/loaders/next-edge-function-loader'

type ObjectValue<T> = T extends { [key: string]: infer V } ? V : never

Expand Down Expand Up @@ -163,6 +164,7 @@ interface CreateEntrypointsParams {
}

export function getEdgeServerEntry(opts: {
rootDir: string
absolutePagePath: string
buildId: string
bundlePath: string
Expand All @@ -179,6 +181,7 @@ export function getEdgeServerEntry(opts: {
const loaderParams: MiddlewareLoaderOptions = {
absolutePagePath: opts.absolutePagePath,
page: opts.page,
rootDir: opts.rootDir,
matchers: opts.middleware?.matchers
? encodeMatchers(opts.middleware.matchers)
: '',
Expand All @@ -188,9 +191,10 @@ export function getEdgeServerEntry(opts: {
}

if (opts.page.startsWith('/api/') || opts.page === '/api') {
const loaderParams: MiddlewareLoaderOptions = {
const loaderParams: EdgeFunctionLoaderOptions = {
absolutePagePath: opts.absolutePagePath,
page: opts.page,
rootDir: opts.rootDir,
}

return `next-edge-function-loader?${stringify(loaderParams)}!`
Expand Down Expand Up @@ -487,6 +491,7 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {

edgeServer[serverBundlePath] = getEdgeServerEntry({
...params,
rootDir,
absolutePagePath: mappings[page],
bundlePath: clientBundlePath,
isDev: false,
Expand Down
Expand Up @@ -16,6 +16,7 @@ export function getModuleBuildInfo(webpackModule: webpack.Module) {
usingIndirectEval?: boolean | Set<string>
route?: RouteMeta
importLocByPath?: Map<string, any>
rootDir?: string
}
}

Expand Down
Expand Up @@ -4,16 +4,18 @@ import { stringifyRequest } from '../stringify-request'
export type EdgeFunctionLoaderOptions = {
absolutePagePath: string
page: string
rootDir: string
}

export default function middlewareLoader(this: any) {
const { absolutePagePath, page }: EdgeFunctionLoaderOptions =
const { absolutePagePath, page, rootDir }: EdgeFunctionLoaderOptions =
this.getOptions()
const stringifiedPagePath = stringifyRequest(this, absolutePagePath)
const buildInfo = getModuleBuildInfo(this._module)
buildInfo.nextEdgeApiFunction = {
page: page || '/',
}
buildInfo.rootDir = rootDir

return `
import { adapter, enhanceGlobals } from 'next/dist/server/web/adapter'
Expand Down
3 changes: 3 additions & 0 deletions packages/next/build/webpack/loaders/next-middleware-loader.ts
Expand Up @@ -6,6 +6,7 @@ import { MIDDLEWARE_LOCATION_REGEXP } from '../../../lib/constants'
export type MiddlewareLoaderOptions = {
absolutePagePath: string
page: string
rootDir: string
matchers?: string
}

Expand All @@ -25,6 +26,7 @@ export default function middlewareLoader(this: any) {
const {
absolutePagePath,
page,
rootDir,
matchers: encodedMatchers,
}: MiddlewareLoaderOptions = this.getOptions()
const matchers = encodedMatchers ? decodeMatchers(encodedMatchers) : undefined
Expand All @@ -35,6 +37,7 @@ export default function middlewareLoader(this: any) {
page:
page.replace(new RegExp(`/${MIDDLEWARE_LOCATION_REGEXP}$`), '') || '/',
}
buildInfo.rootDir = rootDir

return `
import { adapter, blockUnallowedResponse, enhanceGlobals } from 'next/dist/server/web/adapter'
Expand Down
16 changes: 16 additions & 0 deletions packages/next/build/webpack/loaders/utils.ts
@@ -1,3 +1,5 @@
import { getPageStaticInfo } from '../../analysis/get-page-static-info'

export const defaultJsFileExtensions = ['js', 'mjs', 'jsx', 'ts', 'tsx']
const imageExtensions = ['jpg', 'jpeg', 'png', 'webp', 'avif']
const nextClientComponents = [
Expand Down Expand Up @@ -47,3 +49,17 @@ export const clientComponentRegex = new RegExp(
export const serverComponentRegex = new RegExp(
`\\.server(\\.(${defaultJsFileExtensions.join('|')}))?$`
)

export async function loadEdgeFunctionConfigFromFile(
absolutePagePath: string,
resolve: (context: string, request: string) => Promise<string>
) {
const pageFilePath = await resolve('/', absolutePagePath)
return (
await getPageStaticInfo({
nextConfig: {},
pageFilePath,
isDev: false,
})
).middleware
}

0 comments on commit 97ac344

Please sign in to comment.