From b7fa48f644ec6cdee7dfa6d0ffe1549a2ea7cb49 Mon Sep 17 00:00:00 2001 From: Damien Simonin Feugas Date: Fri, 7 Oct 2022 16:14:11 +0200 Subject: [PATCH] fix(middleware): 'instanceof Function' is dynamic code false-positive (#41249) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## ๐Ÿ› What's in there? `foo instanceof Function` is wrongly considered as Dynamic code evaluation by our static analyzer. This PR fixes it. - [ ] Related issues linked using `fixes #number` - [x] Integration tests added - [x] Errors have a helpful link attached, see `contributing.md` ## ๐Ÿงช How to reproduce? 1. Create a simple repro in `examples` folder: ```js // examples/instance-of-function/pages/index.js export default function Home() { return

home

} // examples/instance-of-function/middleware.js import { NextResponse } from 'next/server' export default async function handler() { console.log('is arrow a function?', (() => {}) instanceof Function) return NextResponse.next() } ``` 1. build with next `pnpm next build examples/instance-of-function` > the build fails 1. rebuild next to include PR's fix `pnpm build` 1. build with new next `pnpm next build examples/instance-of-function` > the build works ## ๐Ÿ“” Notes to reviewers `hooks.expression.for(`${prefix}Function`).tap(NAME, handleExpression)` is actually legacy code from the original implementation. It's used when finding `Function` regardless of how it is used. We only want to find `new Function()` or `Function()`, which `hooks.calls` and `hooks.new` are covering. `eval instanceof Function` is perfectly legit code on the edge, despite its uselessness :lol-think: Because we got multiple people asking "how do I relax this error when my code contains unreachable dynamic code evaluation", I've copy-pasted details about `config.unstable_allowDynamic` into the error page. Because users do not always click links :blob_shrug: --- errors/edge-dynamic-code-evaluation.md | 14 ++++- .../webpack/plugins/middleware-plugin.ts | 2 - .../test/index.test.js | 57 +++++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/errors/edge-dynamic-code-evaluation.md b/errors/edge-dynamic-code-evaluation.md index 29a5e74b6329f..26a2de05ef172 100644 --- a/errors/edge-dynamic-code-evaluation.md +++ b/errors/edge-dynamic-code-evaluation.md @@ -29,6 +29,18 @@ export default async function middleware() { ``` 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). +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): + +```typescript +export const config = { + runtime: 'experimental-edge', // for Edge API Routes only + unstable_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 + ], +} +``` + +`unstable_allowDynamic` is a glob, 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_. diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts index e0e206bb11541..5c100ec15b196 100644 --- a/packages/next/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/build/webpack/plugins/middleware-plugin.ts @@ -553,8 +553,6 @@ Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime`, hooks.call.for(`${prefix}eval`).tap(NAME, handleWrapExpression) hooks.call.for(`${prefix}Function`).tap(NAME, handleWrapExpression) hooks.new.for(`${prefix}Function`).tap(NAME, handleWrapExpression) - hooks.expression.for(`${prefix}eval`).tap(NAME, handleExpression) - hooks.expression.for(`${prefix}Function`).tap(NAME, handleExpression) hooks.call .for(`${prefix}WebAssembly.compile`) .tap(NAME, handleWrapWasmCompileExpression) diff --git a/test/integration/edge-runtime-configurable-guards/test/index.test.js b/test/integration/edge-runtime-configurable-guards/test/index.test.js index bc218a0f03265..c55c813b4a8f0 100644 --- a/test/integration/edge-runtime-configurable-guards/test/index.test.js +++ b/test/integration/edge-runtime-configurable-guards/test/index.test.js @@ -387,4 +387,61 @@ describe('Edge runtime configurable guards', () => { expect(output.stderr).toContain(TELEMETRY_EVENT_NAME) }) }) + + describe.each([ + { + title: 'Edge API', + url: routeUrl, + init() { + context.api.write(` + export default async function handler(request) { + return Response.json({ result: (() => {}) instanceof Function }) + } + export const config = { runtime: 'experimental-edge' } + `) + }, + }, + { + title: 'Middleware', + url: middlewareUrl, + init() { + context.middleware.write(` + import { NextResponse } from 'next/server' + import { returnTrue } from './lib' + export default async function () { + (() => {}) instanceof Function + return NextResponse.next() + } + `) + }, + }, + ])('$title with use of Function as a type', ({ init, url }) => { + beforeEach(() => init()) + + it('does not warn in dev at runtime', async () => { + context.app = await launchApp(context.appDir, context.appPort, appOption) + const res = await fetchViaHTTP(context.appPort, url) + await waitFor(500) + expect(res.status).toBe(200) + expect(context.logs.output).not.toContain( + `Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime` + ) + }) + + // eslint-disable-next-line jest/no-identical-title + it('build and does not warn at runtime', async () => { + const output = await nextBuild(context.appDir, undefined, { + stdout: true, + stderr: true, + }) + expect(output.stderr).not.toContain(`Build failed`) + context.app = await nextStart(context.appDir, context.appPort, appOption) + const res = await fetchViaHTTP(context.appPort, url) + expect(res.status).toBe(200) + expect(context.logs.output).not.toContain(`warn`) + expect(context.logs.output).not.toContain( + `Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime` + ) + }) + }) })