diff --git a/.gitignore b/.gitignore index a1e1b057fd451..bea76abc513db 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,7 @@ test/tmp/** # Editors **/.idea **/.#* +.nvmrc # examples examples/**/out diff --git a/docs/advanced-features/middleware.md b/docs/advanced-features/middleware.md index 70ac0612259a6..d4ce7a5318ead 100644 --- a/docs/advanced-features/middleware.md +++ b/docs/advanced-features/middleware.md @@ -31,10 +31,14 @@ npm install next@latest ```jsx // middleware.ts -import type { NextFetchEvent, NextRequest } from 'next/server' - -export function middleware(req: NextRequest, ev: NextFetchEvent) { - return new Response('Hello, world!') +import type { NextRequest, NextResponse } from 'next/server' +import { areCredentialsValid } from '../lib' + +export function middleware(req: NextRequest) { + if (areCredentialsValid(req.headers.get('authorization')) { + return NextResponse.next() + } + return NextResponse.redirect(`/login?from=${req.nextUrl.pathname}`) } ``` diff --git a/docs/api-reference/next/server.md b/docs/api-reference/next/server.md index d84c2759a1aec..b239eb461fc9a 100644 --- a/docs/api-reference/next/server.md +++ b/docs/api-reference/next/server.md @@ -105,7 +105,6 @@ The following static methods are available on the `NextResponse` class directly: - `redirect()` - Returns a `NextResponse` with a redirect set - `rewrite()` - Returns a `NextResponse` with a rewrite set - `next()` - Returns a `NextResponse` that will continue the middleware chain -- `json()` - A convenience method to create a response that encodes the provided JSON data ```ts import { NextResponse } from 'next/server' @@ -120,7 +119,7 @@ export function middleware(req: NextRequest) { return NextResponse.rewrite('/not-home') } - return NextResponse.json({ message: 'Hello World!' }) + return NextResponse.next() } ``` @@ -183,6 +182,21 @@ console.log(NODE_ENV) console.log(process.env) ``` +### The body limitation + +When using middlewares, it is not permitted to change the response body: you can only set responses headers. +Returning a body from a middleware function will issue an `500` server error with an explicit response message. + +The `NextResponse` API (which eventually is tweaking response headers) allows you to: + +- redirect the incoming request to a different url +- rewrite the response by displaying a given url +- set response cookies +- set response headers + +These are solid tools to implement cases such as A/B testing, authentication, feature flags, bot protection... +A middleware with the ability to change the response's body would bypass Next.js routing logic. + ## Related
diff --git a/errors/manifest.json b/errors/manifest.json index 82ae1b4f93412..213596aaeac9a 100644 --- a/errors/manifest.json +++ b/errors/manifest.json @@ -657,6 +657,10 @@ { "title": "invalid-script", "path": "/errors/invalid-script.md" + }, + { + "title": "returning-response-body-in-middleware", + "path": "/errors/returning-response-body-in-middleware.md" } ] } diff --git a/errors/returning-response-body-in-middleware.md b/errors/returning-response-body-in-middleware.md new file mode 100644 index 0000000000000..cdd9c6aea9624 --- /dev/null +++ b/errors/returning-response-body-in-middleware.md @@ -0,0 +1,84 @@ +# Returning response body in middleware + +#### Why This Error Occurred + +Your [`middleware`](https://nextjs.org/docs/advanced-features/middleware) function returns a response body, which is not supported. + +Letting middleware respond to incoming requests would bypass Next.js routing mechanism, creating an unecessary escape hatch. + +#### Possible Ways to Fix It + +Next.js middleware gives you a great opportunity to run code and adjust to the requesting user. + +It is intended for use cases like: + +- A/B testing, where you **_rewrite_** to a different page based on external data (User agent, user location, a custom header or cookie...) + + ```js + export function middleware(req: NextRequest) { + let res = NextResponse.next() + // reuses cookie, or builds a new one. + const cookie = req.cookies.get(COOKIE_NAME) ?? buildABTestingCookie() + + // the cookie contains the displayed variant, 0 being default + const [, variantId] = cookie.split('.') + if (variantId !== '0') { + const url = req.nextUrl.clone() + url.pathname = url.pathname.replace('/', `/${variantId}/`) + // rewrites the response to display desired variant + res = NextResponse.rewrite(url) + } + + // don't forget to set cookie if not set yet + if (!req.cookies.has(COOKIE_NAME)) { + res.cookies.set(COOKIE_NAME, cookie) + } + return res + } + ``` + +- authentication, where you **_redirect_** to your log-in/sign-in page any un-authenticated request + + ```js + export function middleware(req: NextRequest) { + const basicAuth = req.headers.get('authorization') + + if (basicAuth) { + const auth = basicAuth.split(' ')[1] + const [user, pwd] = atob(auth).split(':') + if (areCredentialsValid(user, pwd)) { + return NextResponse.next() + } + } + + return NextResponse.redirect(`/login?from=${req.nextUrl.pathname}`) + } + ``` + +- detecting bots and **_rewrite_** response to display to some sink + + ```js + export function middleware(req: NextRequest) { + if (isABotRequest(req)) { + // Bot detected! rewrite to the sink + const url = req.nextUrl.clone() + url.pathname = '/bot-detected' + return NextResponse.rewrite(url) + } + return NextResponse.next() + } + ``` + +- programmatically adding **_headers_** to the response, like cookies. + + ```js + export function middleware(req: NextRequest) { + const res = NextResponse.next(null, { + // sets a custom response header + headers: { 'response-greetings': 'Hej!' }, + }) + // configures cookies + response.cookies.set('hello', 'world') + return res + } + ``` diff --git a/packages/next/build/webpack/loaders/next-middleware-loader.ts b/packages/next/build/webpack/loaders/next-middleware-loader.ts index 668d6106c5fea..4a76f02923010 100644 --- a/packages/next/build/webpack/loaders/next-middleware-loader.ts +++ b/packages/next/build/webpack/loaders/next-middleware-loader.ts @@ -16,7 +16,7 @@ export default function middlewareLoader(this: any) { } return ` - import { adapter } from 'next/dist/server/web/adapter' + import { adapter, blockUnallowedResponse } from 'next/dist/server/web/adapter' // The condition is true when the "process" module is provided if (process !== global.process) { @@ -33,11 +33,11 @@ export default function middlewareLoader(this: any) { } export default function (opts) { - return adapter({ + return blockUnallowedResponse(adapter({ ...opts, page: ${JSON.stringify(page)}, handler, - }) + })) } ` } diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts index dd2d6eda63147..3bbdac4679afa 100644 --- a/packages/next/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/build/webpack/plugins/middleware-plugin.ts @@ -54,11 +54,14 @@ export default class MiddlewarePlugin { apply(compiler: webpack5.Compiler) { compiler.hooks.compilation.tap(NAME, (compilation, params) => { const { hooks } = params.normalModuleFactory - /** * This is the static code analysis phase. */ - const codeAnalyzer = getCodeAnalizer({ dev: this.dev, compiler }) + const codeAnalyzer = getCodeAnalizer({ + dev: this.dev, + compiler, + compilation, + }) hooks.parser.for('javascript/auto').tap(NAME, codeAnalyzer) hooks.parser.for('javascript/dynamic').tap(NAME, codeAnalyzer) hooks.parser.for('javascript/esm').tap(NAME, codeAnalyzer) @@ -94,11 +97,13 @@ export default class MiddlewarePlugin { function getCodeAnalizer(params: { dev: boolean compiler: webpack5.Compiler + compilation: webpack5.Compilation }) { return (parser: webpack5.javascript.JavascriptParser) => { const { dev, compiler: { webpack: wp }, + compilation, } = params const { hooks } = parser @@ -176,6 +181,31 @@ function getCodeAnalizer(params: { } } + /** + * 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 ( + isUserMiddlewareUserFile(parser.state.current) && + 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) + } + } + } + /** * A noop handler to skip analyzing some cases. * Order matters: for it to work, it must be registered first @@ -192,6 +222,8 @@ function getCodeAnalizer(params: { hooks.expression.for(`${prefix}eval`).tap(NAME, handleExpression) hooks.expression.for(`${prefix}Function`).tap(NAME, handleExpression) } + hooks.new.for('Response').tap(NAME, handleNewResponseExpression) + hooks.new.for('NextResponse').tap(NAME, handleNewResponseExpression) hooks.callMemberChain.for('process').tap(NAME, handleCallMemberChain) hooks.expressionMemberChain.for('process').tap(NAME, handleCallMemberChain) } @@ -421,3 +453,17 @@ function getEntryFiles(entryFiles: string[], meta: EntryMetadata) { ) return files } + +function isUserMiddlewareUserFile(module: any) { + return ( + module.layer === 'middleware' && /middleware\.\w+$/.test(module.rawRequest) + ) +} + +function isNullLiteral(expr: any) { + return expr.value === null +} + +function isUndefinedIdentifier(expr: any) { + return expr.name === 'undefined' +} diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 6252ea738f5cc..ba385e3f52b56 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -27,14 +27,36 @@ export async function adapter(params: { }) const event = new NextFetchEvent({ request, page: params.page }) - const original = await params.handler(request, event) + const response = await params.handler(request, event) return { - response: original || NextResponse.next(), + response: response || NextResponse.next(), waitUntil: Promise.all(event[waitUntilSymbol]), } } +export function blockUnallowedResponse( + promise: Promise +): Promise { + return promise.then((result) => { + if (result.response?.body) { + console.error( + new Error( + `A middleware can not alter response's body. Learn more: https://nextjs.org/docs/messages/returning-response-body-in-middleware` + ) + ) + return { + ...result, + response: new Response('Internal Server Error', { + status: 500, + statusText: 'Internal Server Error', + }), + } + } + return result + }) +} + class NextRequestHint extends NextRequest { sourcePage: string diff --git a/packages/next/server/web/error.ts b/packages/next/server/web/error.ts index ebe14d9f27d78..cb55b3e5dd0c7 100644 --- a/packages/next/server/web/error.ts +++ b/packages/next/server/web/error.ts @@ -3,7 +3,7 @@ export class DeprecationError extends Error { super(`The middleware "${page}" accepts an async API directly with the form: export function middleware(request, event) { - return new Response("Hello " + request.url) + return NextResponse.redirect('/new-location') } Read more: https://nextjs.org/docs/messages/middleware-new-signature diff --git a/packages/next/server/web/spec-compliant/response.ts b/packages/next/server/web/spec-compliant/response.ts index 97334d4facd0e..d94b59c52633e 100644 --- a/packages/next/server/web/spec-compliant/response.ts +++ b/packages/next/server/web/spec-compliant/response.ts @@ -46,8 +46,8 @@ class BaseResponse extends Body implements Response { ) } - return new Response(validateURL(url), { - headers: { Location: url }, + return new Response(null, { + headers: { Location: validateURL(url) }, status, }) } diff --git a/packages/next/server/web/spec-extension/response.ts b/packages/next/server/web/spec-extension/response.ts index 8ab8b08773174..1af4e9f8f8139 100644 --- a/packages/next/server/web/spec-extension/response.ts +++ b/packages/next/server/web/spec-extension/response.ts @@ -51,26 +51,21 @@ export class NextResponse extends Response { ) } - const destination = validateURL(url) - return new NextResponse(destination, { - headers: { Location: destination }, + return new NextResponse(null, { + headers: { Location: validateURL(url) }, status, }) } static rewrite(destination: string | NextURL | URL) { return new NextResponse(null, { - headers: { - 'x-middleware-rewrite': validateURL(destination), - }, + headers: { 'x-middleware-rewrite': validateURL(destination) }, }) } static next() { return new NextResponse(null, { - headers: { - 'x-middleware-next': '1', - }, + headers: { 'x-middleware-next': '1' }, }) } } diff --git a/test/development/middleware-warnings/index.test.ts b/test/development/middleware-warnings/index.test.ts new file mode 100644 index 0000000000000..0bae0cdb177d6 --- /dev/null +++ b/test/development/middleware-warnings/index.test.ts @@ -0,0 +1,86 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { sandbox } from '../acceptance/helpers' + +const middlewarePath = 'middleware.js' +const middlewareWarning = `A middleware can not alter response's body` + +describe('middlewares', () => { + let next: NextInstance + let cleanup + + beforeAll(async () => { + next = await createNext({ + files: {}, + skipStart: true, + }) + }) + + afterAll(() => next.destroy()) + + afterEach(() => cleanup?.()) + + it.each([ + { + title: 'returning response with literal string', + code: `export default function middleware() { + return new Response('this is not allowed'); + }`, + }, + { + title: 'returning response with literal number', + code: `export default function middleware() { + return new Response(10); + }`, + }, + { + title: 'returning response with JSON.stringify', + code: `export default function middleware() { + return new Response(JSON.stringify({ foo: 'this is not allowed' })); + }`, + }, + { + title: 'populating response with a value', + code: `export default function middleware(request) { + const body = JSON.stringify({ foo: 'this should not be allowed, but hard to detect with AST' }) + return new Response(body); + }`, + }, + { + title: 'populating response with a function call', + code: `function buildBody() { + return 'this should not be allowed, but hard to detect with AST' + } + export default function middleware(request) { + return new Response(buildBody()); + }`, + }, + { + title: 'populating response with an async function call', + code: `export default async function middleware(request) { + return new Response(await fetch('https://example.com')); + }`, + }, + ])('warns when $title', async ({ code }) => { + ;({ cleanup } = await sandbox(next, new Map([[middlewarePath, code]]))) + expect(next.cliOutput).toMatch(middlewareWarning) + }) + + it.each([ + { + title: 'returning null reponse body', + code: `export default function middleware() { + return new Response(null); + }`, + }, + { + title: 'returning undefined response body', + code: `export default function middleware() { + return new Response(undefined); + }`, + }, + ])('does not warn when $title', async ({ code }) => { + ;({ cleanup } = await sandbox(next, new Map([[middlewarePath, code]]))) + expect(next.cliOutput).not.toMatch(middlewareWarning) + }) +}) diff --git a/test/e2e/middleware-can-use-wasm-files/index.test.ts b/test/e2e/middleware-can-use-wasm-files/index.test.ts index 5441f4d9138e1..945ad80a5c7b9 100644 --- a/test/e2e/middleware-can-use-wasm-files/index.test.ts +++ b/test/e2e/middleware-can-use-wasm-files/index.test.ts @@ -25,7 +25,7 @@ function baseNextConfig(): Parameters[0] { export default async function middleware(request) { const input = Number(request.nextUrl.searchParams.get('input')) || 1; const value = await increment(input); - return new Response(JSON.stringify({ input, value })); + return new Response(null, { headers: { data: JSON.stringify({ input, value }) } }); } `, }, @@ -43,7 +43,7 @@ describe('middleware can use wasm files', () => { it('uses the wasm file', async () => { const response = await fetchViaHTTP(next.url, '/') - expect(await response.json()).toEqual({ + expect(extractJSON(response)).toEqual({ input: 1, value: 2, }) @@ -51,7 +51,7 @@ describe('middleware can use wasm files', () => { it('can be called twice', async () => { const response = await fetchViaHTTP(next.url, '/', { input: 2 }) - expect(await response.json()).toEqual({ + expect(extractJSON(response)).toEqual({ input: 2, value: 3, }) @@ -100,9 +100,13 @@ describe('middleware can use wasm files with the experimental modes on', () => { it('uses the wasm file', async () => { const response = await fetchViaHTTP(next.url, '/') - expect(await response.json()).toEqual({ + expect(extractJSON(response)).toEqual({ input: 1, value: 2, }) }) }) + +function extractJSON(response) { + return JSON.parse(response.headers.get('data') ?? '{}') +} diff --git a/test/integration/middleware-build-errors/middleware.js b/test/integration/middleware-build-errors/middleware.js new file mode 100644 index 0000000000000..3dfc1f78793df --- /dev/null +++ b/test/integration/middleware-build-errors/middleware.js @@ -0,0 +1 @@ +// this will be populated by each test diff --git a/test/integration/middleware-build-errors/pages/index.js b/test/integration/middleware-build-errors/pages/index.js new file mode 100644 index 0000000000000..7786f48037692 --- /dev/null +++ b/test/integration/middleware-build-errors/pages/index.js @@ -0,0 +1,3 @@ +export default function Home() { + return
A page
+} diff --git a/test/integration/middleware-build-errors/test/index.test.js b/test/integration/middleware-build-errors/test/index.test.js new file mode 100644 index 0000000000000..e1d75aa48d4cb --- /dev/null +++ b/test/integration/middleware-build-errors/test/index.test.js @@ -0,0 +1,91 @@ +import { remove, writeFile } from 'fs-extra' +import { nextBuild } from 'next-test-utils' +import { join } from 'path' + +describe('Middleware validation during build', () => { + const appDir = join(__dirname, '..') + const middlewareFile = join(appDir, 'middleware.js') + const middlewareError = 'Your middleware is returning a response body' + + beforeEach(() => remove(join(appDir, '.next'))) + + afterEach(() => + writeFile(middlewareFile, '// this will be populated by each test\n') + ) + + describe.each([ + { + title: 'returning a text body', + code: `export default function () { + return new Response('this is not allowed') + }`, + }, + { + title: 'building body with JSON.stringify', + code: `export default function () { + return new Response(JSON.stringify({ error: 'this is not allowed' })) + }`, + }, + { + title: 'building response body with a variable', + code: `export default function () { + const body = 'this is not allowed, but hard to detect with AST' + return new Response(body) + }`, + }, + { + title: 'building response body with custom code', + code: `function buildResponse() { + return JSON.stringify({ message: 'this is not allowed, but hard to detect with AST' }) + } + + export default function () { + return new Response(buildResponse()) + }`, + }, + { + title: 'returning a text body with NextResponse', + code: `import { NextResponse } from 'next/server' + export default function () { + return new NextResponse('this is not allowed') + }`, + }, + ])('given a middleware $title', ({ code }) => { + beforeAll(() => writeFile(middlewareFile, code)) + + it('throws an error', async () => { + const { stderr, code } = await nextBuild(appDir, [], { + stderr: true, + stdout: true, + }) + expect(stderr).toMatch(middlewareError) + expect(code).toBe(1) + }) + }) + + describe.each([ + { + title: 'returning a null body', + code: `export default function () { + return new Response(null) + }`, + }, + { + title: 'returning an undefined body', + code: `export default function () { + return new Response(undefined) + }`, + }, + ])('given a middleware $title', ({ code }) => { + beforeAll(() => writeFile(middlewareFile, code)) + + it('builds successfully', async () => { + const { stderr, code } = await nextBuild(appDir, [], { + stderr: true, + stdout: true, + }) + expect(stderr).not.toMatch(middlewareError) + expect(code).toBe(0) + }) + }) +}) diff --git a/test/integration/middleware-dynamic-code/middleware.js b/test/integration/middleware-dynamic-code/middleware.js index 60a8860854161..841fd30564617 100644 --- a/test/integration/middleware-dynamic-code/middleware.js +++ b/test/integration/middleware-dynamic-code/middleware.js @@ -2,18 +2,14 @@ import { notUsingEval, usingEval } from './lib/utils' export async function middleware(request) { if (request.nextUrl.pathname === '/using-eval') { - return new Response(JSON.stringify(await usingEval()), { - headers: { - 'Content-Type': 'application/json', - }, + return new Response(null, { + headers: { data: JSON.stringify(await usingEval()) }, }) } if (request.nextUrl.pathname === '/not-using-eval') { - return new Response(JSON.stringify(await notUsingEval()), { - headers: { - 'Content-Type': 'application/json', - }, + return new Response(null, { + headers: { data: JSON.stringify(await notUsingEval()) }, }) } } diff --git a/test/integration/middleware-dynamic-code/test/index.test.js b/test/integration/middleware-dynamic-code/test/index.test.js index a51409d84eb3e..31750978f6252 100644 --- a/test/integration/middleware-dynamic-code/test/index.test.js +++ b/test/integration/middleware-dynamic-code/test/index.test.js @@ -40,7 +40,7 @@ describe('Middleware usage of dynamic code evaluation', () => { it('shows a warning when running code with eval', async () => { const res = await fetchViaHTTP(context.appPort, `/using-eval`) - const json = await res.json() + const json = JSON.parse(res.headers.get('data')) await waitFor(500) expect(json.value).toEqual(100) expect(output).toContain(DYNAMIC_CODE_ERROR) @@ -54,7 +54,7 @@ describe('Middleware usage of dynamic code evaluation', () => { it('does not show warning when no code uses eval', async () => { const res = await fetchViaHTTP(context.appPort, `/not-using-eval`) - const json = await res.json() + const json = JSON.parse(res.headers.get('data')) await waitFor(500) expect(json.value).toEqual(100) expect(output).not.toContain(DYNAMIC_CODE_ERROR) diff --git a/test/integration/middleware-general/middleware.js b/test/integration/middleware-general/middleware.js index 40dd617a6efed..e4f57524d24ff 100644 --- a/test/integration/middleware-general/middleware.js +++ b/test/integration/middleware-general/middleware.js @@ -10,19 +10,9 @@ export async function middleware(request) { const apiRoute = new URL(url) apiRoute.pathname = '/api/headers' const res = await fetch(apiRoute) - return new Response(await res.text(), { - status: 200, - headers: { - 'content-type': 'application/json', - }, - }) + return serializeData(await res.text()) } catch (err) { - return new Response(JSON.stringify({ error: err.message }), { - status: 500, - headers: { - 'content-type': 'application/json', - }, - }) + return serializeError(err) } } @@ -35,19 +25,9 @@ export async function middleware(request) { 'user-agent': 'custom-agent', }, }) - return new Response(await res.text(), { - status: 200, - headers: { - 'content-type': 'application/json', - }, - }) + return serializeData(await res.text()) } catch (err) { - return new Response(JSON.stringify({ error: err.message }), { - status: 500, - headers: { - 'content-type': 'application/json', - }, - }) + return serializeError(err) } } @@ -55,19 +35,11 @@ export async function middleware(request) { // The next line is required to allow to find the env variable // eslint-disable-next-line no-unused-expressions process.env.MIDDLEWARE_TEST - return NextResponse.json({ - process: { - env: process.env, - }, - }) + return serializeData(JSON.stringify({ process: { env: process.env } })) } if (url.pathname.endsWith('/globalthis')) { - return new NextResponse(JSON.stringify(Object.keys(globalThis)), { - headers: { - 'content-type': 'application/json; charset=utf-8', - }, - }) + return serializeData(JSON.stringify(Object.keys(globalThis))) } if (url.pathname.endsWith('/webcrypto')) { @@ -84,11 +56,7 @@ export async function middleware(request) { } catch (err) { response.error = true } finally { - return new NextResponse(JSON.stringify(response), { - headers: { - 'content-type': 'application/json; charset=utf-8', - }, - }) + return serializeData(JSON.stringify(response)) } } @@ -102,11 +70,7 @@ export async function middleware(request) { message: err.message, } } finally { - return new NextResponse(JSON.stringify(response), { - headers: { - 'content-type': 'application/json; charset=utf-8', - }, - }) + return serializeData(JSON.stringify(response)) } } @@ -125,11 +89,7 @@ export async function middleware(request) { message: err.message, } } finally { - return new NextResponse(JSON.stringify(response), { - headers: { - 'content-type': 'application/json; charset=utf-8', - }, - }) + return serializeData(JSON.stringify(response)) } } @@ -147,11 +107,13 @@ export async function middleware(request) { if (url.pathname.startsWith('/url')) { try { if (request.nextUrl.pathname === '/url/relative-url') { - return NextResponse.json({ message: String(new URL('/relative')) }) + new URL('/relative') + return Response.next() } if (request.nextUrl.pathname === '/url/relative-request') { - return fetch(new Request('/urls-b')) + await fetch(new Request('/urls-b')) + return Response.next() } if (request.nextUrl.pathname === '/url/relative-redirect') { @@ -167,14 +129,11 @@ export async function middleware(request) { } if (request.nextUrl.pathname === '/url/relative-next-request') { - return fetch(new NextRequest('/urls-b')) + await fetch(new NextRequest('/urls-b')) + return NextResponse.next() } } catch (error) { - return NextResponse.json({ - error: { - message: error.message, - }, - }) + return new NextResponse(null, { headers: { error: error.message } }) } } @@ -190,3 +149,11 @@ export async function middleware(request) { }, }) } + +function serializeData(data) { + return new NextResponse(null, { headers: { data } }) +} + +function serializeError(error) { + return new NextResponse(null, { headers: { error: error.message } }) +} diff --git a/test/integration/middleware-general/test/index.test.js b/test/integration/middleware-general/test/index.test.js index 73d52d1ae5889..9a10010b50105 100644 --- a/test/integration/middleware-general/test/index.test.js +++ b/test/integration/middleware-general/test/index.test.js @@ -138,19 +138,20 @@ function tests(context, locale = '') { context.appPort, `${locale}/fetch-user-agent-default` ) - expect((await res.json()).headers['user-agent']).toBe('Next.js Middleware') + expect(readMiddlewareJSON(res).headers['user-agent']).toBe( + 'Next.js Middleware' + ) const res2 = await fetchViaHTTP( context.appPort, `${locale}/fetch-user-agent-crypto` ) - expect((await res2.json()).headers['user-agent']).toBe('custom-agent') + expect(readMiddlewareJSON(res2).headers['user-agent']).toBe('custom-agent') }) it('should contain process polyfill', async () => { const res = await fetchViaHTTP(context.appPort, `/global`) - const json = await res.json() - expect(json).toEqual({ + expect(readMiddlewareJSON(res)).toEqual({ process: { env: { MIDDLEWARE_TEST: 'asdf', @@ -162,26 +163,24 @@ function tests(context, locale = '') { it(`should contain \`globalThis\``, async () => { const res = await fetchViaHTTP(context.appPort, '/globalthis') - const globals = await res.json() - expect(globals.length > 0).toBe(true) + expect(readMiddlewareJSON(res).length > 0).toBe(true) }) it(`should contain crypto APIs`, async () => { const res = await fetchViaHTTP(context.appPort, '/webcrypto') - const response = await res.json() - expect('error' in response).toBe(false) + expect('error' in readMiddlewareJSON(res)).toBe(false) }) it(`should accept a URL instance for fetch`, async () => { const res = await fetchViaHTTP(context.appPort, '/fetch-url') - const response = await res.json() - expect('error' in response).toBe(true) + const response = readMiddlewareJSON(res) + expect(response).toHaveProperty('error.name') expect(response.error.name).not.toBe('TypeError') }) it(`should allow to abort a fetch request`, async () => { const res = await fetchViaHTTP(context.appPort, '/abort-controller') - const response = await res.json() + const response = readMiddlewareJSON(res) expect('error' in response).toBe(true) expect(response.error.name).toBe('AbortError') expect(response.error.message).toBe('The user aborted a request.') @@ -228,27 +227,14 @@ function tests(context, locale = '') { expect(res.headers.get('req-url-locale')).toBe('en') }) - it(`should render correctly rewriting with a root subrequest`, async () => { - const browser = await webdriver(context.appPort, '/root-subrequest') - const element = await browser.elementByCss('.title') - expect(await element.text()).toEqual('Dynamic route') - }) - - it(`should allow subrequests without infinite loops`, async () => { - const res = await fetchViaHTTP(context.appPort, `/root-subrequest`) - expect(res.headers.get('x-dynamic-path')).toBe('true') - }) - it('should throw when using URL with a relative URL', async () => { const res = await fetchViaHTTP(context.appPort, `/url/relative-url`) - const json = await res.json() - expect(json.error.message).toContain('Invalid URL') + expect(readMiddlewareError(res)).toContain('Invalid URL') }) it('should throw when using Request with a relative URL', async () => { const res = await fetchViaHTTP(context.appPort, `/url/relative-request`) - const json = await res.json() - expect(json.error.message).toContain('Invalid URL') + expect(readMiddlewareError(res)).toContain('Invalid URL') }) it('should throw when using NextRequest with a relative URL', async () => { @@ -256,8 +242,7 @@ function tests(context, locale = '') { context.appPort, `/url/relative-next-request` ) - const json = await res.json() - expect(json.error.message).toContain('Invalid URL') + expect(readMiddlewareError(res)).toContain('Invalid URL') }) it('should warn when using Response.redirect with a relative URL', async () => { @@ -265,11 +250,7 @@ function tests(context, locale = '') { context.appPort, `/url/relative-redirect` ) - expect(await response.json()).toEqual({ - error: { - message: expect.stringContaining(urlsError), - }, - }) + expect(readMiddlewareError(response)).toContain(urlsError) }) it('should warn when using NextResponse.redirect with a relative URL', async () => { @@ -277,11 +258,7 @@ function tests(context, locale = '') { context.appPort, `/url/relative-next-redirect` ) - expect(await response.json()).toEqual({ - error: { - message: expect.stringContaining(urlsError), - }, - }) + expect(readMiddlewareError(response)).toContain(urlsError) }) it('should throw when using NextResponse.rewrite with a relative URL', async () => { @@ -289,10 +266,14 @@ function tests(context, locale = '') { context.appPort, `/url/relative-next-rewrite` ) - expect(await response.json()).toEqual({ - error: { - message: expect.stringContaining(urlsError), - }, - }) + expect(readMiddlewareError(response)).toContain(urlsError) }) } + +function readMiddlewareJSON(response) { + return JSON.parse(response.headers.get('data')) +} + +function readMiddlewareError(response) { + return response.headers.get('error') +} diff --git a/test/integration/middleware-responses/lib/utils.js b/test/integration/middleware-responses/lib/utils.js deleted file mode 100644 index 1e3f457afc11a..0000000000000 --- a/test/integration/middleware-responses/lib/utils.js +++ /dev/null @@ -1,8 +0,0 @@ -export function getTextWithEval() { - // eslint-disable-next-line no-eval - return eval('with some text') -} - -export function getText() { - return 'with some text' -} diff --git a/test/integration/middleware-responses/middleware.js b/test/integration/middleware-responses/middleware.js index 2e91aec519d86..2116abc64f1c2 100644 --- a/test/integration/middleware-responses/middleware.js +++ b/test/integration/middleware-responses/middleware.js @@ -1,7 +1,8 @@ import { NextResponse } from 'next/server' -import { createElement } from 'react' -import { renderToString } from 'react-dom/server.browser' -import { getText } from './lib/utils' + +// we use this trick to fool static analysis at build time, so we can build a +// middleware that will return a body at run time, and check it is disallowed. +class MyResponse extends Response {} export async function middleware(request, ev) { // eslint-disable-next-line no-undef @@ -36,9 +37,7 @@ export async function middleware(request, ev) { const headers = new Headers() headers.append('set-cookie', 'foo=chocochip') headers.append('set-cookie', 'bar=chocochip') - return new Response('cookies set', { - headers, - }) + return new Response(null, { headers }) } // Streams a basic response @@ -47,84 +46,24 @@ export async function middleware(request, ev) { (async () => { writer.write(encoder.encode('this is a streamed ')) writer.write(encoder.encode('response ')) - writer.write(encoder.encode(getText())) writer.close() })() ) - return new Response(readable) + return new MyResponse(readable) } if (url.pathname === '/bad-status') { - return new Response('Auth required', { + return new Response(null, { headers: { 'WWW-Authenticate': 'Basic realm="Secure Area"' }, status: 401, }) } - if (url.pathname === '/stream-long') { - ev.waitUntil( - (async () => { - writer.write(encoder.encode('this is a streamed '.repeat(10))) - await sleep(200) - writer.write(encoder.encode('after 2 seconds '.repeat(10))) - await sleep(200) - writer.write(encoder.encode('after 4 seconds '.repeat(10))) - await sleep(200) - writer.close() - })() - ) - - return new Response(readable) - } - // Sends response if (url.pathname === '/send-response') { - return new Response(JSON.stringify({ message: 'hi!' })) - } - - // Render React component - if (url.pathname === '/react') { - return new Response( - renderToString( - createElement( - 'h1', - {}, - 'SSR with React! Hello, ' + url.searchParams.get('name') - ) - ) - ) - } - - // Stream React component - if (url.pathname === '/react-stream') { - ev.waitUntil( - (async () => { - writer.write( - encoder.encode( - renderToString(createElement('h1', {}, 'I am a stream')) - ) - ) - await sleep(500) - writer.write( - encoder.encode( - renderToString(createElement('p', {}, 'I am another stream')) - ) - ) - writer.close() - })() - ) - - return new Response(readable) + return new MyResponse(JSON.stringify({ message: 'hi!' })) } return next } - -function sleep(time) { - return new Promise((resolve) => { - setTimeout(() => { - resolve() - }, time) - }) -} diff --git a/test/integration/middleware-responses/test/index.test.js b/test/integration/middleware-responses/test/index.test.js index e5dcaa21faf4b..19509dad2cb74 100644 --- a/test/integration/middleware-responses/test/index.test.js +++ b/test/integration/middleware-responses/test/index.test.js @@ -1,7 +1,6 @@ /* eslint-env jest */ import { join } from 'path' -import cheerio from 'cheerio' import { fetchViaHTTP, findPort, @@ -12,14 +11,22 @@ import { } from 'next-test-utils' jest.setTimeout(1000 * 60 * 2) -const context = { appDir: join(__dirname, '../') } +const context = { appDir: join(__dirname, '../'), output: '' } describe('Middleware Responses', () => { describe('dev mode', () => { afterAll(() => killApp(context.app)) beforeAll(async () => { + context.output = '' context.appPort = await findPort() - context.app = await launchApp(context.appDir, context.appPort) + context.app = await launchApp(context.appDir, context.appPort, { + onStdout(msg) { + context.output += msg + }, + onStderr(msg) { + context.output += msg + }, + }) }) testsWithLocale(context) @@ -29,9 +36,17 @@ describe('Middleware Responses', () => { describe('production mode', () => { afterAll(() => killApp(context.app)) beforeAll(async () => { + context.output = '' await nextBuild(context.appDir) context.appPort = await findPort() - context.app = await nextStart(context.appDir, context.appPort) + context.app = await nextStart(context.appDir, context.appPort, { + onStdout(msg) { + context.output += msg + }, + onStderr(msg) { + context.output += msg + }, + }) }) testsWithLocale(context) @@ -50,53 +65,32 @@ function testsWithLocale(context, locale = '') { ]) }) - it(`${label}should stream a response`, async () => { + it(`${label}should fail when returning a stream`, async () => { const res = await fetchViaHTTP( context.appPort, `${locale}/stream-a-response` ) - const html = await res.text() - expect(html).toBe('this is a streamed response with some text') + expect(res.status).toBe(500) + expect(await res.text()).toEqual('Internal Server Error') + expect(context.output).toContain( + `A middleware can not alter response's body. Learn more: https://nextjs.org/docs/messages/returning-response-body-in-middleware` + ) }) - it(`${label}should respond with a body`, async () => { + it(`${label}should fail when returning a text body`, async () => { const res = await fetchViaHTTP(context.appPort, `${locale}/send-response`) - const html = await res.text() - expect(html).toBe('{"message":"hi!"}') + expect(res.status).toBe(500) + expect(await res.text()).toEqual('Internal Server Error') + expect(context.output).toContain( + `A middleware can not alter response's body. Learn more: https://nextjs.org/docs/messages/returning-response-body-in-middleware` + ) }) it(`${label}should respond with a 401 status code`, async () => { const res = await fetchViaHTTP(context.appPort, `${locale}/bad-status`) const html = await res.text() expect(res.status).toBe(401) - expect(html).toBe('Auth required') - }) - - it(`${label}should render a React component`, async () => { - const res = await fetchViaHTTP(context.appPort, `${locale}/react?name=jack`) - const html = await res.text() - expect(html).toBe('

SSR with React! Hello, jack

') - }) - - it(`${label}should stream a React component`, async () => { - const res = await fetchViaHTTP(context.appPort, `${locale}/react-stream`) - const html = await res.text() - expect(html).toBe('

I am a stream

I am another stream

') - }) - - it(`${label}should stream a long response`, async () => { - const res = await fetchViaHTTP(context.appPort, '/stream-long') - const html = await res.text() - expect(html).toBe( - 'this is a streamed this is a streamed this is a streamed this is a streamed this is a streamed this is a streamed this is a streamed this is a streamed this is a streamed this is a streamed after 2 seconds after 2 seconds after 2 seconds after 2 seconds after 2 seconds after 2 seconds after 2 seconds after 2 seconds after 2 seconds after 2 seconds after 4 seconds after 4 seconds after 4 seconds after 4 seconds after 4 seconds after 4 seconds after 4 seconds after 4 seconds after 4 seconds after 4 seconds ' - ) - }) - - it(`${label}should render the right content via SSR`, async () => { - const res = await fetchViaHTTP(context.appPort, '/') - const html = await res.text() - const $ = cheerio.load(html) - expect($('.title').text()).toBe('Hello World') + expect(html).toBe('') }) it(`${label}should respond with one header`, async () => { diff --git a/test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts b/test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts index b3645de051292..753c4fb63c027 100644 --- a/test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts +++ b/test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts @@ -1,6 +1,6 @@ import { createNext } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' -import { renderViaHTTP } from 'next-test-utils' +import { fetchViaHTTP } from 'next-test-utils' import { readJson } from 'fs-extra' import path from 'path' @@ -32,13 +32,13 @@ describe('dependencies can use env vars in middlewares', () => { 'middleware.js': ` import customPackage from 'my-custom-package'; export default function middleware(_req) { - return new Response(JSON.stringify({ - string: "a constant string", - hello: process.env.ENV_VAR_USED_IN_MIDDLEWARE, - customPackage: customPackage(), - }), { - headers: { - 'Content-Type': 'application/json' + return new Response(null, { + headers: { + data: JSON.stringify({ + string: "a constant string", + hello: process.env.ENV_VAR_USED_IN_MIDDLEWARE, + customPackage: customPackage(), + }) } }) } @@ -70,13 +70,11 @@ describe('dependencies can use env vars in middlewares', () => { }) it('uses the environment variables', async () => { - const html = await renderViaHTTP(next.url, '/api') - expect(html).toContain( - JSON.stringify({ - string: 'a constant string', - hello: 'env-var-used-in-middleware', - customPackage: 'my-custom-package-env-var', - }) - ) + const response = await fetchViaHTTP(next.url, '/api') + expect(JSON.parse(response.headers.get('data'))).toEqual({ + string: 'a constant string', + hello: 'env-var-used-in-middleware', + customPackage: 'my-custom-package-env-var', + }) }) }) diff --git a/test/production/generate-middleware-source-maps/index.test.ts b/test/production/generate-middleware-source-maps/index.test.ts index 6bf2be3f8e994..9428992b0cd98 100644 --- a/test/production/generate-middleware-source-maps/index.test.ts +++ b/test/production/generate-middleware-source-maps/index.test.ts @@ -18,8 +18,9 @@ describe('experimental.middlewareSourceMaps: true', () => { export default function () { return
Hello, world!
} `, 'middleware.js': ` + import { NextResponse } from "next/server"; export default function middleware() { - return new Response("Hello, world!"); + return NextResponse.next(); } `, }, @@ -48,8 +49,9 @@ describe('experimental.middlewareSourceMaps: false', () => { export default function () { return
Hello, world!
} `, 'middleware.js': ` + import { NextResponse } from "next/server"; export default function middleware() { - return new Response("Hello, world!"); + return NextResponse.next(); } `, }, diff --git a/test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts b/test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts index a364d7bca84eb..7a92988615a11 100644 --- a/test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts +++ b/test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts @@ -1,6 +1,6 @@ import { createNext } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' -import { renderViaHTTP } from 'next-test-utils' +import { fetchViaHTTP } from 'next-test-utils' describe('middleware environment variables in node server reflect the usage inference', () => { let next: NextInstance @@ -19,12 +19,12 @@ describe('middleware environment variables in node server reflect the usage infe `, 'middleware.js': ` export default function middleware() { - return new Response(JSON.stringify({ - canBeInferred: process.env.CAN_BE_INFERRED, - rest: process.env - }), { + return new Response(null, { headers: { - 'Content-Type': 'application/json', + data: JSON.stringify({ + canBeInferred: process.env.CAN_BE_INFERRED, + rest: process.env + }), 'X-Custom-Header': process.env.X_CUSTOM_HEADER, } }) @@ -37,12 +37,8 @@ describe('middleware environment variables in node server reflect the usage infe afterAll(() => next.destroy()) it('limits process.env to only contain env vars that are inferred from usage', async () => { - const html = await renderViaHTTP(next.url, '/test') - let parsed: any - expect(() => { - parsed = JSON.parse(html) - }).not.toThrow() - expect(parsed).toEqual({ + const response = await fetchViaHTTP(next.url, '/test') + expect(JSON.parse(response.headers.get('data'))).toEqual({ canBeInferred: 'can-be-inferred', rest: { CAN_BE_INFERRED: 'can-be-inferred', diff --git a/test/production/middleware-typescript/app/middleware.ts b/test/production/middleware-typescript/app/middleware.ts index 4b84726dd5e9d..5fa7ff9b59c1b 100644 --- a/test/production/middleware-typescript/app/middleware.ts +++ b/test/production/middleware-typescript/app/middleware.ts @@ -2,8 +2,9 @@ import { NextMiddleware, NextResponse } from 'next/server' export const middleware: NextMiddleware = function (request) { if (request.nextUrl.pathname === '/static') { - return new NextResponse('hello from middleware', { + return new NextResponse(null, { headers: { + data: 'hello from middleware', 'req-url-basepath': request.nextUrl.basePath, 'req-url-pathname': request.nextUrl.pathname, 'req-url-params': JSON.stringify(request.page.params), diff --git a/test/production/middleware-typescript/test/index.test.ts b/test/production/middleware-typescript/test/index.test.ts index b14ab2a372f66..e43d1f6a4bea8 100644 --- a/test/production/middleware-typescript/test/index.test.ts +++ b/test/production/middleware-typescript/test/index.test.ts @@ -3,7 +3,7 @@ import { join } from 'path' import { createNext, FileRef } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' -import { renderViaHTTP } from 'next-test-utils' +import { fetchViaHTTP } from 'next-test-utils' const appDir = join(__dirname, '../app') @@ -29,7 +29,7 @@ describe('should set-up next', () => { afterAll(() => next.destroy()) it('should have built and started', async () => { - const html = await renderViaHTTP(next.url, '/static') - expect(html).toContain('hello from middleware') + const response = await fetchViaHTTP(next.url, '/static') + expect(response.headers.get('data')).toEqual('hello from middleware') }) }) diff --git a/test/production/reading-request-body-in-middleware/index.test.ts b/test/production/reading-request-body-in-middleware/index.test.ts index 2a3f79d73cc24..88a92a98b05f0 100644 --- a/test/production/reading-request-body-in-middleware/index.test.ts +++ b/test/production/reading-request-body-in-middleware/index.test.ts @@ -13,7 +13,7 @@ describe('reading request body in middleware', () => { export default async function middleware(request) { if (!request.body) { - return new Response('No body', { status: 400 }); + return new Response(null, { status: 400 }); } let json; @@ -28,13 +28,10 @@ describe('reading request body in middleware', () => { return res; } - return new Response(JSON.stringify({ - root: true, - ...json, - }), { + return new Response(null, { status: 200, headers: { - 'content-type': 'application/json', + data: JSON.stringify({ root: true, ...json }), }, }) } @@ -72,7 +69,7 @@ describe('reading request body in middleware', () => { } ) expect(response.status).toEqual(200) - expect(await response.json()).toEqual({ + expect(JSON.parse(response.headers.get('data'))).toEqual({ foo: 'bar', root: true, }) @@ -101,6 +98,7 @@ describe('reading request body in middleware', () => { api: true, }) expect(response.headers.get('x-from-root-middleware')).toEqual('1') + expect(response.headers.has('data')).toBe(false) }) it('passes the body to the api endpoint when no body is consumed on middleware', async () => { @@ -127,5 +125,6 @@ describe('reading request body in middleware', () => { api: true, }) expect(response.headers.get('x-from-root-middleware')).toEqual('1') + expect(response.headers.has('data')).toBe(false) }) }) diff --git a/test/unit/web-runtime/next-cookies.test.ts b/test/unit/web-runtime/next-cookies.test.ts index 4292608293e94..78719719c3c86 100644 --- a/test/unit/web-runtime/next-cookies.test.ts +++ b/test/unit/web-runtime/next-cookies.test.ts @@ -157,7 +157,9 @@ it('response.cookie does not modify options', async () => { ) const options = { maxAge: 10000 } - const response = NextResponse.json(null) + const response = new NextResponse(null, { + headers: { 'content-type': 'application/json' }, + }) response.cookies.set('cookieName', 'cookieValue', options) expect(options).toEqual({ maxAge: 10000 }) }) diff --git a/test/unit/web-runtime/next-response.test.ts b/test/unit/web-runtime/next-response.test.ts deleted file mode 100644 index 85351b6e22122..0000000000000 --- a/test/unit/web-runtime/next-response.test.ts +++ /dev/null @@ -1,73 +0,0 @@ -/* eslint-env jest */ - -import { Blob, File, FormData } from 'next/dist/compiled/formdata-node' -import { Crypto } from 'next/dist/server/web/sandbox/polyfills' -import { Response } from 'next/dist/server/web/spec-compliant/response' -import { Headers } from 'next/dist/server/web/spec-compliant/headers' -import * as streams from 'web-streams-polyfill/ponyfill' - -beforeAll(() => { - global['Blob'] = Blob - global['crypto'] = new Crypto() - global['File'] = File - global['FormData'] = FormData - global['Headers'] = Headers - global['ReadableStream'] = streams.ReadableStream - global['TransformStream'] = streams.TransformStream - global['Response'] = Response -}) - -afterAll(() => { - delete global['Blob'] - delete global['crypto'] - delete global['File'] - delete global['Headers'] - delete global['FormData'] - delete global['ReadableStream'] - delete global['TransformStream'] -}) - -const toJSON = async (response) => ({ - body: await response.json(), - contentType: response.headers.get('content-type'), - status: response.status, -}) - -it('automatically parses and formats JSON', async () => { - const { NextResponse } = await import( - 'next/dist/server/web/spec-extension/response' - ) - - expect(await toJSON(NextResponse.json({ message: 'hello!' }))).toMatchObject({ - contentType: 'application/json', - body: { message: 'hello!' }, - }) - - expect( - await toJSON(NextResponse.json({ status: 'success' }, { status: 201 })) - ).toMatchObject({ - contentType: 'application/json', - body: { status: 'success' }, - status: 201, - }) - - expect( - await toJSON( - NextResponse.json({ error: { code: 'bad_request' } }, { status: 400 }) - ) - ).toMatchObject({ - contentType: 'application/json', - body: { error: { code: 'bad_request' } }, - status: 400, - }) - - expect(await toJSON(NextResponse.json(null))).toMatchObject({ - contentType: 'application/json', - body: null, - }) - - expect(await toJSON(NextResponse.json(''))).toMatchObject({ - contentType: 'application/json', - body: '', - }) -})