diff --git a/errors/manifest.json b/errors/manifest.json index 4aaeead555a5a..69d0763077574 100644 --- a/errors/manifest.json +++ b/errors/manifest.json @@ -527,6 +527,10 @@ { "title": "invalid-styled-jsx-children", "path": "/errors/invalid-styled-jsx-children.md" + }, + { + "title": "middleware-relative-urls", + "path": "/errors/middleware-relative-urls.md" } ] } diff --git a/errors/middleware-relative-urls.md b/errors/middleware-relative-urls.md new file mode 100644 index 0000000000000..dabdf33060855 --- /dev/null +++ b/errors/middleware-relative-urls.md @@ -0,0 +1,38 @@ +# Middleware Relative URLs + +#### Why This Error Occurred + +You are using a Middleware function that uses `Response.redirect(url)`, `NextResponse.redirect(url)` or `NextResponse.rewrite(url)` where `url` is a relative or an invalid URL. Currently this will work, but building a request with `new Request(url)` or running `fetch(url)` when `url` is a relative URL will **not** work. For this reason and to bring consistency to Next.js Middleware, this behavior will be deprecated soon in favor of always using absolute URLs. + +#### Possible Ways to Fix It + +To fix this warning you must always pass absolute URL for redirecting and rewriting. There are several ways to get the absolute URL but the recommended way is to clone `NextURL` and mutate it: + +```typescript +import type { NextRequest } from 'next/server' +import { NextResponse } from 'next/server' + +export function middleware(request: NextRequest) { + const url = request.nextUrl.clone() + url.pathname = '/dest' + return NextResponse.rewrite(url) +} +``` + +Another way to fix this error could be to use the original URL as the base but this will not consider configuration like `basePath` or `locale`: + +```typescript +import type { NextRequest } from 'next/server' +import { NextResponse } from 'next/server' + +export function middleware(request: NextRequest) { + return NextResponse.rewrite(new URL('/dest', request.url)) +} +``` + +You can also pass directly a string containing a valid absolute URL. + +### Useful Links + +- [URL Documentation](https://developer.mozilla.org/en-US/docs/Web/API/URL) +- [Response Documentation](https://developer.mozilla.org/en-US/docs/Web/API/Response) diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index 8ee57ff66a6cb..532b93e64ab38 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -244,6 +244,10 @@ export class NextURL { toJSON() { return this.href } + + clone() { + return new NextURL(String(this), this[Internal].options) + } } const REGEX_LOCALHOST_HOSTNAME = diff --git a/packages/next/server/web/spec-compliant/response.ts b/packages/next/server/web/spec-compliant/response.ts index c7d5c746e9316..3ea1a55650f6b 100644 --- a/packages/next/server/web/spec-compliant/response.ts +++ b/packages/next/server/web/spec-compliant/response.ts @@ -1,5 +1,6 @@ import { Body, BodyInit, cloneBody, extractContentType } from './body' import { NextURL } from '../next-url' +import { validateURL } from '../utils' const INTERNALS = Symbol('internal response') const REDIRECTS = new Set([301, 302, 303, 307, 308]) @@ -45,7 +46,7 @@ class BaseResponse extends Body implements Response { ) } - return new Response(url, { + return new Response(validateURL(url), { headers: { Location: url }, status, }) diff --git a/packages/next/server/web/spec-extension/response.ts b/packages/next/server/web/spec-extension/response.ts index 76b40a8490407..240096c3bd30d 100644 --- a/packages/next/server/web/spec-extension/response.ts +++ b/packages/next/server/web/spec-extension/response.ts @@ -1,6 +1,6 @@ import type { I18NConfig } from '../../config-shared' import { NextURL } from '../next-url' -import { toNodeHeaders } from '../utils' +import { toNodeHeaders, validateURL } from '../utils' import cookie from 'next/dist/compiled/cookie' import { CookieSerializeOptions } from '../types' @@ -79,7 +79,8 @@ export class NextResponse extends Response { 'Failed to execute "redirect" on "response": Invalid status code' ) } - const destination = typeof url === 'string' ? url : url.toString() + + const destination = validateURL(url) return new NextResponse(destination, { headers: { Location: destination }, status, @@ -89,10 +90,7 @@ export class NextResponse extends Response { static rewrite(destination: string | NextURL) { return new NextResponse(null, { headers: { - 'x-middleware-rewrite': - typeof destination === 'string' - ? destination - : destination.toString(), + 'x-middleware-rewrite': validateURL(destination), }, }) } diff --git a/packages/next/server/web/utils.ts b/packages/next/server/web/utils.ts index 2c5dae0e83dcd..06e257457f09d 100644 --- a/packages/next/server/web/utils.ts +++ b/packages/next/server/web/utils.ts @@ -147,3 +147,20 @@ export function splitCookiesString(cookiesString: string) { return cookiesStrings } + +/** + * We will be soon deprecating the usage of relative URLs in Middleware introducing + * URL validation. This helper puts the future code in place and prints a warning + * for cases where it will break. Meanwhile we preserve the previous behavior. + */ +export function validateURL(url: string | URL): string { + try { + return String(new URL(String(url))) + } catch (error: any) { + console.log( + `warn -`, + 'using relative URLs for Middleware will be deprecated soon - https://nextjs.org/docs/messages/middleware-relative-urls' + ) + return String(url) + } +} diff --git a/test/integration/middleware/core/pages/interface/_middleware.js b/test/integration/middleware/core/pages/interface/_middleware.js index 81b57cba1fb4d..1aee8917ed5c9 100644 --- a/test/integration/middleware/core/pages/interface/_middleware.js +++ b/test/integration/middleware/core/pages/interface/_middleware.js @@ -81,7 +81,8 @@ export async function middleware(request) { } if (url.pathname.endsWith('/dynamic-replace')) { - return NextResponse.rewrite('/_interface/dynamic-path') + url.pathname = '/_interface/dynamic-path' + return NextResponse.rewrite(url) } return new Response(null, { diff --git a/test/integration/middleware/core/pages/redirects/_middleware.js b/test/integration/middleware/core/pages/redirects/_middleware.js index e97cb99011b04..0a8dafc5c8b26 100644 --- a/test/integration/middleware/core/pages/redirects/_middleware.js +++ b/test/integration/middleware/core/pages/redirects/_middleware.js @@ -56,6 +56,6 @@ export async function middleware(request) { if (url.pathname === '/redirects/infinite-loop-1') { url.pathname = '/redirects/infinite-loop' - return Response.redirect(url.pathname) + return Response.redirect(url) } } diff --git a/test/integration/middleware/core/pages/rewrites/_middleware.js b/test/integration/middleware/core/pages/rewrites/_middleware.js index 13d4453938251..20113e2978d16 100644 --- a/test/integration/middleware/core/pages/rewrites/_middleware.js +++ b/test/integration/middleware/core/pages/rewrites/_middleware.js @@ -5,24 +5,27 @@ export async function middleware(request) { if (url.pathname.startsWith('/rewrites/to-blog')) { const slug = url.pathname.split('/').pop() - console.log('rewriting to slug', slug) - return NextResponse.rewrite(`/rewrites/fallback-true-blog/${slug}`) + url.pathname = `/rewrites/fallback-true-blog/${slug}` + return NextResponse.rewrite(url) } if (url.pathname === '/rewrites/rewrite-to-ab-test') { let bucket = request.cookies.bucket if (!bucket) { bucket = Math.random() >= 0.5 ? 'a' : 'b' - const response = NextResponse.rewrite(`/rewrites/${bucket}`) + url.pathname = `/rewrites/${bucket}` + const response = NextResponse.rewrite(url) response.cookie('bucket', bucket, { maxAge: 10000 }) return response } - return NextResponse.rewrite(`/rewrites/${bucket}`) + url.pathname = `/rewrites/${bucket}` + return NextResponse.rewrite(url) } if (url.pathname === '/rewrites/rewrite-me-to-about') { - return NextResponse.rewrite('/rewrites/about') + url.pathname = '/rewrites/about' + return NextResponse.rewrite(url) } if (url.pathname === '/rewrites/rewrite-me-to-vercel') { diff --git a/test/integration/middleware/core/pages/urls/_middleware.js b/test/integration/middleware/core/pages/urls/_middleware.js new file mode 100644 index 0000000000000..43efc371d2e6d --- /dev/null +++ b/test/integration/middleware/core/pages/urls/_middleware.js @@ -0,0 +1,35 @@ +import { NextResponse, NextRequest } from 'next/server' + +export function middleware(request) { + try { + if (request.nextUrl.pathname === '/urls/relative-url') { + return NextResponse.json({ message: String(new URL('/relative')) }) + } + + if (request.nextUrl.pathname === '/urls/relative-request') { + return fetch(new Request('/urls/urls-b')) + } + + if (request.nextUrl.pathname === '/urls/relative-redirect') { + return Response.redirect('/urls/urls-b') + } + + if (request.nextUrl.pathname === '/urls/relative-next-redirect') { + return NextResponse.redirect('/urls/urls-b') + } + + if (request.nextUrl.pathname === '/urls/relative-next-rewrite') { + return NextResponse.rewrite('/urls/urls-b') + } + + if (request.nextUrl.pathname === '/urls/relative-next-request') { + return fetch(new NextRequest('/urls/urls-b')) + } + } catch (error) { + return NextResponse.json({ + error: { + message: error.message, + }, + }) + } +} diff --git a/test/integration/middleware/core/pages/urls/index.js b/test/integration/middleware/core/pages/urls/index.js new file mode 100644 index 0000000000000..a1a42f2f39375 --- /dev/null +++ b/test/integration/middleware/core/pages/urls/index.js @@ -0,0 +1,3 @@ +export default function URLsA() { + return

URLs A

+} diff --git a/test/integration/middleware/core/pages/urls/urls-b.js b/test/integration/middleware/core/pages/urls/urls-b.js new file mode 100644 index 0000000000000..19326a7b8d8ac --- /dev/null +++ b/test/integration/middleware/core/pages/urls/urls-b.js @@ -0,0 +1,3 @@ +export default function URLsB() { + return

URLs B

+} diff --git a/test/integration/middleware/core/test/index.test.js b/test/integration/middleware/core/test/index.test.js index 25f4e01634ab3..32e2b99c2f4a2 100644 --- a/test/integration/middleware/core/test/index.test.js +++ b/test/integration/middleware/core/test/index.test.js @@ -19,18 +19,20 @@ const context = {} context.appDir = join(__dirname, '../') const middlewareWarning = 'using beta Middleware (not covered by semver)' +const urlsWarning = 'using relative URLs for Middleware will be deprecated soon' describe('Middleware base tests', () => { describe('dev mode', () => { - let output = '' + const log = { output: '' } + beforeAll(async () => { context.appPort = await findPort() context.app = await launchApp(context.appDir, context.appPort, { onStdout(msg) { - output += msg + log.output += msg }, onStderr(msg) { - output += msg + log.output += msg }, }) }) @@ -43,14 +45,16 @@ describe('Middleware base tests', () => { responseTests('/fr') interfaceTests() interfaceTests('/fr') + urlTests(log) + urlTests(log, '/fr') it('should have showed warning for middleware usage', () => { - expect(output).toContain(middlewareWarning) + expect(log.output).toContain(middlewareWarning) }) }) describe('production mode', () => { + let serverOutput = { output: '' } let buildOutput - let serverOutput beforeAll(async () => { const res = await nextBuild(context.appDir, undefined, { @@ -62,10 +66,10 @@ describe('Middleware base tests', () => { context.appPort = await findPort() context.app = await nextStart(context.appDir, context.appPort, { onStdout(msg) { - serverOutput += msg + serverOutput.output += msg }, onStderr(msg) { - serverOutput += msg + serverOutput.output += msg }, }) }) @@ -78,13 +82,15 @@ describe('Middleware base tests', () => { responseTests('/fr') interfaceTests() interfaceTests('/fr') + urlTests(serverOutput) + urlTests(serverOutput, '/fr') it('should have middleware warning during build', () => { expect(buildOutput).toContain(middlewareWarning) }) it('should have middleware warning during start', () => { - expect(serverOutput).toContain(middlewareWarning) + expect(serverOutput.output).toContain(middlewareWarning) }) it('should have correct files in manifest', async () => { @@ -104,6 +110,57 @@ describe('Middleware base tests', () => { }) }) +function urlTests(log, locale = '') { + it('rewrites by default to a target location', async () => { + const res = await fetchViaHTTP(context.appPort, `${locale}/urls`) + const html = await res.text() + const $ = cheerio.load(html) + expect($('.title').text()).toBe('URLs A') + }) + + it('throws when using URL with a relative URL', async () => { + const res = await fetchViaHTTP( + context.appPort, + `${locale}/urls/relative-url` + ) + const json = await res.json() + expect(json.error.message).toContain('Invalid URL') + }) + + it('throws when using Request with a relative URL', async () => { + const res = await fetchViaHTTP( + context.appPort, + `${locale}/urls/relative-request` + ) + const json = await res.json() + expect(json.error.message).toContain('Invalid URL') + }) + + it('throws when using NextRequest with a relative URL', async () => { + const res = await fetchViaHTTP( + context.appPort, + `${locale}/urls/relative-next-request` + ) + const json = await res.json() + expect(json.error.message).toContain('Invalid URL') + }) + + it('warns when using Response.redirect with a relative URL', async () => { + await fetchViaHTTP(context.appPort, `${locale}/urls/relative-redirect`) + expect(log.output).toContain(urlsWarning) + }) + + it('warns when using NextResponse.redirect with a relative URL', async () => { + await fetchViaHTTP(context.appPort, `${locale}/urls/relative-next-redirect`) + expect(log.output).toContain(urlsWarning) + }) + + it('warns when using NextResponse.rewrite with a relative URL', async () => { + await fetchViaHTTP(context.appPort, `${locale}/urls/relative-next-rewrite`) + expect(log.output).toContain(urlsWarning) + }) +} + function rewriteTests(locale = '') { it('should rewrite to fallback: true page successfully', async () => { const randomSlug = `another-${Date.now()}` diff --git a/test/integration/middleware/hmr/pages/about/_middleware.js b/test/integration/middleware/hmr/pages/about/_middleware.js index 64dba899b58b9..66902bd191d7f 100644 --- a/test/integration/middleware/hmr/pages/about/_middleware.js +++ b/test/integration/middleware/hmr/pages/about/_middleware.js @@ -1,5 +1,5 @@ import { NextResponse } from 'next/server' -export function middleware() { - return NextResponse.rewrite('/about/a') +export function middleware(request) { + return NextResponse.rewrite(new URL('/about/a', request.url)) } diff --git a/test/unit/web-runtime/next-url.test.ts b/test/unit/web-runtime/next-url.test.ts index e3914a938e920..ed13e63eb217f 100644 --- a/test/unit/web-runtime/next-url.test.ts +++ b/test/unit/web-runtime/next-url.test.ts @@ -170,3 +170,21 @@ it('allows to change the port', () => { url.port = '' expect(url.href).toEqual('https://localhost/foo') }) + +it('allows to clone a new copy', () => { + const url = new NextURL('/root/es/bar', { + base: 'http://127.0.0.1', + basePath: '/root', + i18n: { + defaultLocale: 'en', + locales: ['en', 'es', 'fr'], + }, + }) + + const clone = url.clone() + clone.pathname = '/test' + clone.basePath = '/root-test' + + expect(url.toString()).toEqual('http://localhost/root/es/bar') + expect(clone.toString()).toEqual('http://localhost/root-test/es/test') +})