2022-04-30 13:19:27 +02:00
import type { EdgeMiddlewareMeta } from '../loaders/get-module-build-info'
import type { EdgeSSRMeta , WasmBinding } from '../loaders/get-module-build-info'
2022-05-19 17:46:21 +02:00
import { getNamedMiddlewareRegex } from '../../../shared/lib/router/utils/route-regex'
2022-04-30 13:19:27 +02:00
import { getModuleBuildInfo } from '../loaders/get-module-build-info'
2021-10-20 19:52:11 +02:00
import { getSortedRoutes } from '../../../shared/lib/router/utils'
2022-04-30 13:19:27 +02:00
import { webpack , sources , webpack5 } from 'next/dist/compiled/webpack/webpack'
2021-10-26 18:50:56 +02:00
import {
2022-04-30 13:19:27 +02:00
EDGE_RUNTIME_WEBPACK ,
2022-05-23 11:07:26 +02:00
EDGE_UNSUPPORTED_NODE_APIS ,
2021-10-26 18:50:56 +02:00
MIDDLEWARE_BUILD_MANIFEST ,
2022-04-30 13:19:27 +02:00
MIDDLEWARE_FLIGHT_MANIFEST ,
MIDDLEWARE_MANIFEST ,
2021-10-26 18:50:56 +02:00
MIDDLEWARE_REACT_LOADABLE_MANIFEST ,
2022-05-18 13:18:28 +02:00
NEXT_CLIENT_SSR_ENTRY_SUFFIX ,
2021-10-26 18:50:56 +02:00
} from '../../../shared/lib/constants'
2021-10-20 19:52:11 +02:00
export interface MiddlewareManifest {
version : 1
2021-10-26 22:18:08 +02:00
sortedMiddleware : string [ ]
2021-11-25 10:46:00 +01:00
clientInfo : [ location : string , isSSR : boolean ] [ ]
2022-05-31 22:11:12 +02:00
middleware : {
[ page : string ] : {
env : string [ ]
files : string [ ]
name : string
page : string
regexp : string
wasm? : WasmBinding [ ]
}
}
2021-10-20 19:52:11 +02:00
}
2022-04-30 13:19:27 +02:00
interface EntryMetadata {
edgeMiddleware? : EdgeMiddlewareMeta
edgeSSR? : EdgeSSRMeta
env : Set < string >
wasmBindings : Set < WasmBinding >
}
const NAME = 'MiddlewarePlugin'
2021-11-08 13:41:49 +01:00
const middlewareManifest : MiddlewareManifest = {
sortedMiddleware : [ ] ,
clientInfo : [ ] ,
middleware : { } ,
version : 1 ,
}
2022-01-31 16:46:04 +01:00
2022-04-30 13:19:27 +02:00
export default class MiddlewarePlugin {
dev : boolean
2022-03-02 16:09:36 +01:00
2022-04-30 13:19:27 +02:00
constructor ( { dev } : { dev : boolean } ) {
this . dev = dev
}
2022-01-31 16:46:04 +01:00
2022-04-30 13:19:27 +02:00
apply ( compiler : webpack5.Compiler ) {
compiler . hooks . compilation . tap ( NAME , ( compilation , params ) = > {
const { hooks } = params . normalModuleFactory
/ * *
* This is the static code analysis phase .
* /
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
const codeAnalyzer = getCodeAnalizer ( {
dev : this.dev ,
compiler ,
compilation ,
} )
2022-04-30 13:19:27 +02:00
hooks . parser . for ( 'javascript/auto' ) . tap ( NAME , codeAnalyzer )
hooks . parser . for ( 'javascript/dynamic' ) . tap ( NAME , codeAnalyzer )
hooks . parser . for ( 'javascript/esm' ) . tap ( NAME , codeAnalyzer )
/ * *
* Extract all metadata for the entry points in a Map object .
* /
const metadataByEntry = new Map < string , EntryMetadata > ( )
compilation . hooks . afterOptimizeModules . tap (
NAME ,
getExtractMetadata ( {
compilation ,
compiler ,
dev : this.dev ,
metadataByEntry ,
2022-04-27 11:50:29 +02:00
} )
2022-04-30 13:19:27 +02:00
)
2022-01-31 16:46:04 +01:00
2022-04-30 13:19:27 +02:00
/ * *
* Emit the middleware manifest .
* /
compilation . hooks . processAssets . tap (
{
name : 'NextJsMiddlewareManifest' ,
stage : ( webpack as any ) . Compilation . PROCESS_ASSETS_STAGE_ADDITIONS ,
} ,
getCreateAssets ( { compilation , metadataByEntry } )
)
2022-01-31 16:46:04 +01:00
} )
}
}
2022-04-30 13:19:27 +02:00
function getCodeAnalizer ( params : {
2021-10-20 19:52:11 +02:00
dev : boolean
2022-04-30 13:19:27 +02:00
compiler : webpack5.Compiler
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
compilation : webpack5.Compilation
2022-04-30 13:19:27 +02:00
} ) {
return ( parser : webpack5.javascript.JavascriptParser ) = > {
const {
dev ,
compiler : { webpack : wp } ,
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
compilation ,
2022-04-30 13:19:27 +02:00
} = params
const { hooks } = parser
/ * *
* This expression handler allows to wrap a dynamic code expression with a
* function call where we can warn about dynamic code not being allowed
* but actually execute the expression .
* /
const handleWrapExpression = ( expr : any ) = > {
2022-05-23 11:07:26 +02:00
if ( ! isInMiddlewareLayer ( parser ) ) {
2022-04-30 13:19:27 +02:00
return
}
2022-04-27 11:50:29 +02:00
2022-04-30 13:19:27 +02:00
if ( dev ) {
const { ConstDependency } = wp . dependencies
const dep1 = new ConstDependency (
'__next_eval__(function() { return ' ,
expr . range [ 0 ]
)
dep1 . loc = expr . loc
parser . state . module . addPresentationalDependency ( dep1 )
const dep2 = new ConstDependency ( '})' , expr . range [ 1 ] )
dep2 . loc = expr . loc
parser . state . module . addPresentationalDependency ( dep2 )
}
2021-10-20 19:52:11 +02:00
2022-04-30 13:19:27 +02:00
handleExpression ( )
return true
}
2021-10-20 19:52:11 +02:00
2022-04-30 13:19:27 +02:00
/ * *
* For an expression this will check the graph to ensure it is being used
* by exports . Then it will store in the module buildInfo a boolean to
* express that it contains dynamic code and , if it is available , the
* module path that is using it .
* /
const handleExpression = ( ) = > {
2022-05-23 11:07:26 +02:00
if ( ! isInMiddlewareLayer ( parser ) ) {
2022-04-30 13:19:27 +02:00
return
2021-10-26 22:18:08 +02:00
}
2021-10-20 19:52:11 +02:00
2022-04-30 13:19:27 +02:00
wp . optimize . InnerGraph . onUsage ( parser . state , ( used = true ) = > {
const buildInfo = getModuleBuildInfo ( parser . state . module )
if ( buildInfo . usingIndirectEval === true || used === false ) {
return
}
2021-11-05 21:48:43 +01:00
2022-04-30 13:19:27 +02:00
if ( ! buildInfo . usingIndirectEval || used === true ) {
buildInfo . usingIndirectEval = used
return
2022-01-31 16:46:04 +01:00
}
2022-04-30 13:19:27 +02:00
buildInfo . usingIndirectEval = new Set ( [
. . . Array . from ( buildInfo . usingIndirectEval ) ,
. . . Array . from ( used ) ,
] )
2022-01-31 16:46:04 +01:00
} )
2022-04-30 13:19:27 +02:00
}
2022-01-31 16:46:04 +01:00
2022-06-09 13:49:58 +02:00
/ * *
* Declares an environment variable that is being used in this module
* through this static analysis .
* /
const addUsedEnvVar = ( envVarName : string ) = > {
const buildInfo = getModuleBuildInfo ( parser . state . module )
if ( buildInfo . nextUsedEnvVars === undefined ) {
buildInfo . nextUsedEnvVars = new Set ( )
}
buildInfo . nextUsedEnvVars . add ( envVarName )
}
2022-04-30 13:19:27 +02:00
/ * *
* A handler for calls to ` process.env ` where we identify the name of the
* ENV variable being assigned and store it in the module info .
* /
const handleCallMemberChain = ( _ : unknown , members : string [ ] ) = > {
if ( members . length >= 2 && members [ 0 ] === 'env' ) {
2022-06-09 13:49:58 +02:00
addUsedEnvVar ( members [ 1 ] )
2022-05-23 11:07:26 +02:00
if ( ! isInMiddlewareLayer ( parser ) ) {
2022-01-31 16:46:04 +01:00
return true
}
2022-04-30 13:19:27 +02:00
}
}
2021-11-05 21:48:43 +01:00
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
/ * *
* A handler for calls to ` new Response() ` so we can fail if user is setting the response ' s body .
* /
const handleNewResponseExpression = ( node : any ) = > {
const firstParameter = node ? . arguments ? . [ 0 ]
if (
2022-05-23 11:07:26 +02:00
isInMiddlewareFile ( parser ) &&
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
firstParameter &&
! isNullLiteral ( firstParameter ) &&
! isUndefinedIdentifier ( firstParameter )
) {
const error = new wp . WebpackError (
` Your middleware is returning a response body (line: ${ node . loc . start . line } ), which is not supported. Learn more: https://nextjs.org/docs/messages/returning-response-body-in-middleware `
)
error . name = NAME
error . module = parser . state . current
error . loc = node . loc
if ( dev ) {
compilation . warnings . push ( error )
} else {
compilation . errors . push ( error )
}
}
}
2022-04-30 13:19:27 +02:00
/ * *
* A noop handler to skip analyzing some cases .
fix(middleware): false positive dynamic code detection at build time (#36955)
## What's in there?
Partially fixes https://github.com/vercel/edge-functions/issues/82
Relates to #36715
Our webpack plugin for middleware leverages static analysis to detect Dyanamic code evaluation in user `_middleware.js` file (and depedencies). Since edge function runtime do not allow them, the build is aborted.
The use of `Function.bind` is considered invalid, while it is legit. A customer using `@aws-sdk/client-s3` reported it.
This PR fixes it.
Please note that this check is too strict: some dynamic code may be in the bundle (despite treeshaking), but may never be used (because of code branches). Since this point is under discussion, this PR adds tests covering some false positives (`@apollo/react-hook`, `qs` and `has`), but does not change the behavior (consider them as errors).
## Notes to reviewer
I looked for test facilities allowing to download the required 3rd party modules. `createNext()` in production context made my day, but showed two issues:
- `cliOutput` is not cleaned in between tests. While clearance during `stop()` would be annoying, I hope that clearance during `start()` is better.
- if `start()` fails while building, the created instance can never be stopped. This is because we don't clear `childProcess` after `build`.
## Bug
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
## Feature
- [ ] 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`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-17 21:35:48 +02:00
* Order matters : for it to work , it must be registered first
2022-04-30 13:19:27 +02:00
* /
2022-05-23 11:07:26 +02:00
const skip = ( ) = > ( isInMiddlewareLayer ( parser ) ? true : undefined )
2022-04-30 13:19:27 +02:00
fix(middleware): false positive dynamic code detection at build time (#36955)
## What's in there?
Partially fixes https://github.com/vercel/edge-functions/issues/82
Relates to #36715
Our webpack plugin for middleware leverages static analysis to detect Dyanamic code evaluation in user `_middleware.js` file (and depedencies). Since edge function runtime do not allow them, the build is aborted.
The use of `Function.bind` is considered invalid, while it is legit. A customer using `@aws-sdk/client-s3` reported it.
This PR fixes it.
Please note that this check is too strict: some dynamic code may be in the bundle (despite treeshaking), but may never be used (because of code branches). Since this point is under discussion, this PR adds tests covering some false positives (`@apollo/react-hook`, `qs` and `has`), but does not change the behavior (consider them as errors).
## Notes to reviewer
I looked for test facilities allowing to download the required 3rd party modules. `createNext()` in production context made my day, but showed two issues:
- `cliOutput` is not cleaned in between tests. While clearance during `stop()` would be annoying, I hope that clearance during `start()` is better.
- if `start()` fails while building, the created instance can never be stopped. This is because we don't clear `childProcess` after `build`.
## Bug
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
## Feature
- [ ] 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`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-17 21:35:48 +02:00
for ( const prefix of [ '' , 'global.' ] ) {
hooks . expression . for ( ` ${ prefix } Function.prototype ` ) . tap ( NAME , skip )
hooks . expression . for ( ` ${ prefix } Function.bind ` ) . tap ( NAME , skip )
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 )
}
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
hooks . new . for ( 'Response' ) . tap ( NAME , handleNewResponseExpression )
hooks . new . for ( 'NextResponse' ) . tap ( NAME , handleNewResponseExpression )
2022-04-30 13:19:27 +02:00
hooks . callMemberChain . for ( 'process' ) . tap ( NAME , handleCallMemberChain )
hooks . expressionMemberChain . for ( 'process' ) . tap ( NAME , handleCallMemberChain )
2022-06-09 13:49:58 +02:00
/ * *
* Support static analyzing environment variables through
* destructuring ` process.env ` or ` process["env"] ` :
*
* const { MY_ENV , "MY-ENV" : myEnv } = process . env
* ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^
* /
hooks . declarator . tap ( NAME , ( declarator ) = > {
if (
declarator . init ? . type === 'MemberExpression' &&
isProcessEnvMemberExpression ( declarator . init ) &&
declarator . id ? . type === 'ObjectPattern'
) {
for ( const property of declarator . id . properties ) {
if ( property . type === 'RestElement' ) continue
if (
property . key . type === 'Literal' &&
typeof property . key . value === 'string'
) {
addUsedEnvVar ( property . key . value )
} else if ( property . key . type === 'Identifier' ) {
addUsedEnvVar ( property . key . name )
}
}
if ( ! isInMiddlewareLayer ( parser ) ) {
return true
}
}
} )
2022-05-23 11:07:26 +02:00
registerUnsupportedApiHooks ( parser , compilation )
2022-04-30 13:19:27 +02:00
}
}
2021-11-05 21:48:43 +01:00
2022-04-30 13:19:27 +02:00
function getExtractMetadata ( params : {
compilation : webpack5.Compilation
compiler : webpack5.Compiler
dev : boolean
metadataByEntry : Map < string , EntryMetadata >
} ) {
const { dev , compilation , metadataByEntry , compiler } = params
const { webpack : wp } = compiler
return ( ) = > {
metadataByEntry . clear ( )
for ( const [ entryName , entryData ] of compilation . entries ) {
if ( entryData . options . runtime !== EDGE_RUNTIME_WEBPACK ) {
// Only process edge runtime entries
continue
}
2021-11-10 16:31:46 +01:00
2022-04-30 13:19:27 +02:00
const { moduleGraph } = compilation
const entryModules = new Set < webpack5.Module > ( )
const addEntriesFromDependency = ( dependency : any ) = > {
const module = moduleGraph . getModule ( dependency )
if ( module ) {
entryModules . add ( module )
2022-01-31 16:46:04 +01:00
}
2022-04-30 13:19:27 +02:00
}
2021-10-20 19:52:11 +02:00
2022-04-30 13:19:27 +02:00
entryData . dependencies . forEach ( addEntriesFromDependency )
entryData . includeDependencies . forEach ( addEntriesFromDependency )
2021-10-20 19:52:11 +02:00
2022-04-30 13:19:27 +02:00
const entryMetadata : EntryMetadata = {
env : new Set < string > ( ) ,
wasmBindings : new Set < WasmBinding > ( ) ,
}
2021-10-20 19:52:11 +02:00
2022-04-30 13:19:27 +02:00
for ( const entryModule of entryModules ) {
const buildInfo = getModuleBuildInfo ( entryModule )
/ * *
* When building for production checks if the module is using ` eval `
* and in such case produces a compilation error . The module has to
* be in use .
* /
if (
! dev &&
buildInfo . usingIndirectEval &&
isUsingIndirectEvalAndUsedByExports ( {
entryModule : entryModule ,
moduleGraph : moduleGraph ,
runtime : wp.util.runtime.getEntryRuntime ( compilation , entryName ) ,
usingIndirectEval : buildInfo.usingIndirectEval ,
wp ,
} )
) {
const id = entryModule . identifier ( )
if ( /node_modules[\\/]regenerator-runtime[\\/]runtime\.js/ . test ( id ) ) {
continue
2022-01-31 16:46:04 +01:00
}
2022-04-30 13:19:27 +02:00
const error = new wp . WebpackError (
` Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Middleware ${ entryName } ${
typeof buildInfo . usingIndirectEval !== 'boolean'
? ` \ nUsed by ${ Array . from ( buildInfo . usingIndirectEval ) . join (
', '
) } `
: ''
} `
)
error . module = entryModule
compilation . errors . push ( error )
}
2022-01-31 16:46:04 +01:00
2022-04-30 13:19:27 +02:00
/ * *
* The entry module has to be either a page or a middleware and hold
* the corresponding metadata .
* /
if ( buildInfo ? . nextEdgeSSR ) {
entryMetadata . edgeSSR = buildInfo . nextEdgeSSR
} else if ( buildInfo ? . nextEdgeMiddleware ) {
entryMetadata . edgeMiddleware = buildInfo . nextEdgeMiddleware
}
2022-01-31 16:46:04 +01:00
2022-04-30 13:19:27 +02:00
/ * *
* If there are env vars found in the module , append them to the set
* of env vars for the entry .
* /
if ( buildInfo ? . nextUsedEnvVars !== undefined ) {
for ( const envName of buildInfo . nextUsedEnvVars ) {
entryMetadata . env . add ( envName )
}
}
2022-01-31 16:46:04 +01:00
2022-04-30 13:19:27 +02:00
/ * *
* If the module is a WASM module we read the binding information and
* append it to the entry wasm bindings .
* /
if ( buildInfo ? . nextWasmMiddlewareBinding ) {
entryMetadata . wasmBindings . add ( buildInfo . nextWasmMiddlewareBinding )
}
2022-01-31 16:46:04 +01:00
2022-04-30 13:19:27 +02:00
/ * *
* Append to the list of modules to process outgoingConnections from
* the module that is being processed .
* /
for ( const conn of moduleGraph . getOutgoingConnections ( entryModule ) ) {
if ( conn . module ) {
entryModules . add ( conn . module )
}
2022-01-31 16:46:04 +01:00
}
2022-04-30 13:19:27 +02:00
}
metadataByEntry . set ( entryName , entryMetadata )
2022-01-31 16:46:04 +01:00
}
2022-04-30 13:19:27 +02:00
}
2021-10-20 19:52:11 +02:00
}
2021-11-05 21:48:43 +01:00
2022-04-30 13:19:27 +02:00
/ * *
* Checks the value of usingIndirectEval and when it is a set of modules it
* check if any of the modules is actually being used . If the value is
* simply truthy it will return true .
* /
function isUsingIndirectEvalAndUsedByExports ( args : {
entryModule : webpack5.Module
2021-11-05 21:48:43 +01:00
moduleGraph : webpack5.ModuleGraph
runtime : any
2022-04-30 13:19:27 +02:00
usingIndirectEval : true | Set < string >
wp : typeof webpack5
2021-11-05 21:48:43 +01:00
} ) : boolean {
2022-04-30 13:19:27 +02:00
const { moduleGraph , runtime , entryModule , usingIndirectEval , wp } = args
if ( typeof usingIndirectEval === 'boolean' ) {
return usingIndirectEval
}
const exportsInfo = moduleGraph . getExportsInfo ( entryModule )
for ( const exportName of usingIndirectEval ) {
if ( exportsInfo . getUsed ( exportName , runtime ) !== wp . UsageState . Unused ) {
2021-11-05 21:48:43 +01:00
return true
2022-04-30 13:19:27 +02:00
}
2021-11-05 21:48:43 +01:00
}
2022-04-30 13:19:27 +02:00
2021-11-05 21:48:43 +01:00
return false
}
2022-04-30 13:19:27 +02:00
function getCreateAssets ( params : {
compilation : webpack5.Compilation
metadataByEntry : Map < string , EntryMetadata >
} ) {
const { compilation , metadataByEntry } = params
return ( assets : any ) = > {
for ( const entrypoint of compilation . entrypoints . values ( ) ) {
if ( ! entrypoint . name ) {
continue
}
// There should always be metadata for the entrypoint.
const metadata = metadataByEntry . get ( entrypoint . name )
2022-05-31 22:11:12 +02:00
const page = metadata ? . edgeMiddleware ? . page || metadata ? . edgeSSR ? . page
2022-04-30 13:19:27 +02:00
if ( ! page ) {
continue
}
2022-05-19 17:46:21 +02:00
const { namedRegex } = getNamedMiddlewareRegex ( page , {
2022-05-31 22:11:12 +02:00
catchAll : ! metadata . edgeSSR ,
2022-05-19 17:46:21 +02:00
} )
2022-06-06 23:37:47 +02:00
const regexp = metadata ? . edgeMiddleware ? . matcherRegexp || namedRegex
2022-05-19 17:46:21 +02:00
2022-05-31 22:11:12 +02:00
middlewareManifest . middleware [ page ] = {
2022-04-30 13:19:27 +02:00
env : Array.from ( metadata . env ) ,
files : getEntryFiles ( entrypoint . getFiles ( ) , metadata ) ,
name : entrypoint.name ,
page : page ,
2022-06-03 18:35:44 +02:00
regexp ,
2022-04-30 13:19:27 +02:00
wasm : Array.from ( metadata . wasmBindings ) ,
}
}
middlewareManifest . sortedMiddleware = getSortedRoutes (
Object . keys ( middlewareManifest . middleware )
)
middlewareManifest . clientInfo = middlewareManifest . sortedMiddleware . map (
( key ) = > [
key ,
! ! metadataByEntry . get ( middlewareManifest . middleware [ key ] . name ) ? . edgeSSR ,
]
)
assets [ MIDDLEWARE_MANIFEST ] = new sources . RawSource (
JSON . stringify ( middlewareManifest , null , 2 )
)
}
}
function getEntryFiles ( entryFiles : string [ ] , meta : EntryMetadata ) {
const files : string [ ] = [ ]
if ( meta . edgeSSR ) {
if ( meta . edgeSSR . isServerComponent ) {
files . push ( ` server/ ${ MIDDLEWARE_FLIGHT_MANIFEST } .js ` )
2022-05-13 19:48:53 +02:00
files . push (
. . . entryFiles
. filter (
( file ) = >
file . startsWith ( 'pages/' ) && ! file . endsWith ( '.hot-update.js' )
)
2022-05-18 13:18:28 +02:00
. map (
( file ) = >
'server/' +
file . replace ( '.js' , NEXT_CLIENT_SSR_ENTRY_SUFFIX + '.js' )
)
2022-05-13 19:48:53 +02:00
)
2022-04-30 13:19:27 +02:00
}
files . push (
` server/ ${ MIDDLEWARE_BUILD_MANIFEST } .js ` ,
` server/ ${ MIDDLEWARE_REACT_LOADABLE_MANIFEST } .js `
)
}
files . push (
. . . entryFiles
. filter ( ( file ) = > ! file . endsWith ( '.hot-update.js' ) )
. map ( ( file ) = > 'server/' + file )
)
return files
}
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
2022-05-23 11:07:26 +02:00
function registerUnsupportedApiHooks (
parser : webpack5.javascript.JavascriptParser ,
compilation : webpack5.Compilation
) {
const { WebpackError } = compilation . compiler . webpack
for ( const expression of EDGE_UNSUPPORTED_NODE_APIS ) {
const warnForUnsupportedApi = ( node : any ) = > {
if ( ! isInMiddlewareLayer ( parser ) ) {
return
}
compilation . warnings . push (
makeUnsupportedApiError ( WebpackError , parser , expression , node . loc )
)
return true
}
parser . hooks . call . for ( expression ) . tap ( NAME , warnForUnsupportedApi )
parser . hooks . expression . for ( expression ) . tap ( NAME , warnForUnsupportedApi )
parser . hooks . callMemberChain
. for ( expression )
. tap ( NAME , warnForUnsupportedApi )
parser . hooks . expressionMemberChain
. for ( expression )
. tap ( NAME , warnForUnsupportedApi )
}
const warnForUnsupportedProcessApi = ( node : any , [ callee ] : string [ ] ) = > {
if ( ! isInMiddlewareLayer ( parser ) || callee === 'env' ) {
return
}
compilation . warnings . push (
makeUnsupportedApiError (
WebpackError ,
parser ,
` process. ${ callee } ` ,
node . loc
)
)
return true
}
parser . hooks . callMemberChain
. for ( 'process' )
. tap ( NAME , warnForUnsupportedProcessApi )
parser . hooks . expressionMemberChain
. for ( 'process' )
. tap ( NAME , warnForUnsupportedProcessApi )
}
function makeUnsupportedApiError (
WebpackError : typeof webpack5 . WebpackError ,
parser : webpack5.javascript.JavascriptParser ,
name : string ,
loc : any
) {
const error = new WebpackError (
` You're using a Node.js API ( ${ name } at line: ${ loc . start . line } ) which is not supported in the Edge Runtime that Middleware uses.
Learn more : https : //nextjs.org/docs/api-reference/edge-runtime`
)
error . name = NAME
error . module = parser . state . current
error . loc = loc
return error
}
function isInMiddlewareLayer ( parser : webpack5.javascript.JavascriptParser ) {
return parser . state . module ? . layer === 'middleware'
}
function isInMiddlewareFile ( parser : webpack5.javascript.JavascriptParser ) {
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
return (
2022-05-23 11:07:26 +02:00
parser . state . current ? . layer === 'middleware' &&
/middleware\.\w+$/ . test ( parser . state . current ? . rawRequest )
feat(middleware)!: forbids middleware response body (#36835)
_Hello Next.js team! First PR here, I hope I've followed the right practices._
### What's in there?
It has been decided to only support the following uses cases in Next.js' middleware:
- rewrite the URL (`x-middleware-rewrite` response header)
- redirect to another URL (`Location` response header)
- pass on to the next piece in the request pipeline (`x-middleware-next` response header)
1. during development, a warning on console tells developers when they are returning a response (either with `Response` or `NextResponse`).
2. at build time, this warning becomes an error.
3. at run time, returning a response body will trigger a 500 HTTP error with a JSON payload containing the detailed error.
All returned/thrown errors contain a link to the documentation.
This is a breaking feature compared to the _beta_ middleware implementation, and also removes `NextResponse.json()` which makes no sense any more.
### How to try it?
- runtime behavior: `HEADLESS=true yarn jest test/integration/middleware/core`
- build behavior : `yarn jest test/integration/middleware/build-errors`
- development behavior: `HEADLESS=true yarn jest test/development/middleware-warnings`
### Notes to reviewers
The limitation happens in next's web adapter. ~The initial implementation was to check `response.body` existence, but it turns out [`Response.redirect()`](https://github.com/vercel/next.js/blob/canary/packages/next/server/web/spec-compliant/response.ts#L42-L53) may set the response body (https://github.com/vercel/next.js/pull/31886). Hence why the proposed implementation specifically looks at response headers.~
`Response.redirect()` and `NextResponse.redirect()` do not need to include the final location in their body: it is handled by next server https://github.com/vercel/next.js/blob/canary/packages/next/server/next-server.ts#L1142
Because this is a breaking change, I had to adjust several tests cases, previously returning JSON/stream/text bodies. When relevant, these middlewares are returning data using response headers.
About DevEx: relying on AST analysis to detect forbidden use cases is not as good as running the code.
Such cases are easy to detect:
```js
new Response('a text value')
new Response(JSON.stringify({ /* whatever */ })
```
But these are false-positive cases:
```js
function returnNull() { return null }
new Response(returnNull())
function doesNothing() {}
new Response(doesNothing())
```
However, I see no good reasons to let users ship middleware such as the one above, hence why the build will fail, even if _technically speaking_, they are not setting the response body.
## Feature
- [x] 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
- [ ] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
2022-05-20 00:02:20 +02:00
)
}
function isNullLiteral ( expr : any ) {
return expr . value === null
}
function isUndefinedIdentifier ( expr : any ) {
return expr . name === 'undefined'
}
2022-06-09 13:49:58 +02:00
function isProcessEnvMemberExpression ( memberExpression : any ) : boolean {
return (
memberExpression . object ? . type === 'Identifier' &&
memberExpression . object . name === 'process' &&
( ( memberExpression . property ? . type === 'Literal' &&
memberExpression . property . value === 'env' ) ||
( memberExpression . property ? . type === 'Identifier' &&
memberExpression . property . name === 'env' ) )
)
}