Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce absolute URLs in Edge Functions runtime #33410

Merged
merged 9 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions errors/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]
}
Expand Down
38 changes: 38 additions & 0 deletions errors/middleware-relative-urls.md
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 4 additions & 0 deletions packages/next/server/web/next-url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ export class NextURL {
toJSON() {
return this.href
}

clone() {
return new NextURL(String(this), this[Internal].options)
}
}

const REGEX_LOCALHOST_HOSTNAME =
Expand Down
3 changes: 2 additions & 1 deletion packages/next/server/web/spec-compliant/response.ts
Original file line number Diff line number Diff line change
@@ -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])
Expand Down Expand Up @@ -45,7 +46,7 @@ class BaseResponse extends Body implements Response {
)
}

return new Response(url, {
return new Response(validateURL(url), {
headers: { Location: url },
status,
})
Expand Down
10 changes: 4 additions & 6 deletions packages/next/server/web/spec-extension/response.ts
Original file line number Diff line number Diff line change
@@ -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'

Expand Down Expand Up @@ -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,
Expand All @@ -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),
},
})
}
Expand Down
17 changes: 17 additions & 0 deletions packages/next/server/web/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
13 changes: 8 additions & 5 deletions test/integration/middleware/core/pages/rewrites/_middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
35 changes: 35 additions & 0 deletions test/integration/middleware/core/pages/urls/_middleware.js
Original file line number Diff line number Diff line change
@@ -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,
},
})
}
}
3 changes: 3 additions & 0 deletions test/integration/middleware/core/pages/urls/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function URLsA() {
return <p className="title">URLs A</p>
}
3 changes: 3 additions & 0 deletions test/integration/middleware/core/pages/urls/urls-b.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function URLsB() {
return <p className="title">URLs B</p>
}
73 changes: 65 additions & 8 deletions test/integration/middleware/core/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
})
})
Expand All @@ -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, {
Expand All @@ -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
},
})
})
Expand All @@ -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 () => {
Expand All @@ -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()}`
Expand Down
4 changes: 2 additions & 2 deletions test/integration/middleware/hmr/pages/about/_middleware.js
Original file line number Diff line number Diff line change
@@ -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))
}
18 changes: 18 additions & 0 deletions test/unit/web-runtime/next-url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})