Apply experimental configs for middleware (#41142)

This applies the experimental configs for testing and also fixes
`set-cookie` headers from middleware/edge functions being merged
unexpectedly.

x-ref: [slack
thread](https://vercel.slack.com/archives/CGU8HUTUH/p1664313529422279)
Fixes: https://github.com/vercel/next.js/issues/40820

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`
This commit is contained in:
JJ Kasper 2022-10-04 10:08:17 -07:00 committed by GitHub
parent d192047a34
commit 8d4840b15a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 386 additions and 49 deletions

View file

@ -253,6 +253,9 @@ export function getDefineEnv({
'process.env.__NEXT_I18N_SUPPORT': JSON.stringify(!!config.i18n),
'process.env.__NEXT_I18N_DOMAINS': JSON.stringify(config.i18n?.domains),
'process.env.__NEXT_ANALYTICS_ID': JSON.stringify(config.analyticsId),
'process.env.__NEXT_NO_MIDDLEWARE_URL_NORMALIZE': JSON.stringify(
config.experimental.skipMiddlewareUrlNormalize
),
'process.env.__NEXT_HAS_WEB_VITALS_ATTRIBUTION': JSON.stringify(
config.experimental.webVitalsAttribution &&
config.experimental.webVitalsAttribution.length > 0

View file

@ -624,50 +624,52 @@ export default async function loadCustomRoutes(
)
}
if (config.trailingSlash) {
redirects.unshift(
{
source: '/:file((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/]+\\.\\w+)/',
destination: '/:file',
permanent: true,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect,
{
source: '/:notfile((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/\\.]+)',
destination: '/:notfile/',
permanent: true,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect
)
if (config.basePath) {
if (!config.experimental?.skipTrailingSlashRedirect) {
if (config.trailingSlash) {
redirects.unshift(
{
source: '/:file((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/]+\\.\\w+)/',
destination: '/:file',
permanent: true,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect,
{
source: '/:notfile((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/\\.]+)',
destination: '/:notfile/',
permanent: true,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect
)
if (config.basePath) {
redirects.unshift({
source: config.basePath,
destination: config.basePath + '/',
permanent: true,
basePath: false,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect)
}
} else {
redirects.unshift({
source: config.basePath,
destination: config.basePath + '/',
source: '/:path+/',
destination: '/:path+',
permanent: true,
basePath: false,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect)
}
} else {
redirects.unshift({
source: '/:path+/',
destination: '/:path+',
permanent: true,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect)
if (config.basePath) {
redirects.unshift({
source: config.basePath + '/',
destination: config.basePath,
permanent: true,
basePath: false,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect)
if (config.basePath) {
redirects.unshift({
source: config.basePath + '/',
destination: config.basePath,
permanent: true,
basePath: false,
locale: config.i18n ? false : undefined,
internal: true,
} as Redirect)
}
}
}

View file

@ -440,6 +440,33 @@ export default abstract class Server<ServerOptions extends Options = Options> {
parsedUrl?: NextUrlWithParsedQuery
): Promise<void> {
try {
// ensure cookies set in middleware are merged and
// not overridden by API routes/getServerSideProps
const _res = (res as any).originalResponse || res
const origSetHeader = _res.setHeader.bind(_res)
_res.setHeader = (name: string, val: string | string[]) => {
if (name.toLowerCase() === 'set-cookie') {
const middlewareValue = getRequestMeta(req, '_nextMiddlewareCookie')
if (
!middlewareValue ||
!Array.isArray(val) ||
!val.every((item, idx) => item === middlewareValue[idx])
) {
val = [
...(middlewareValue || []),
...(typeof val === 'string'
? [val]
: Array.isArray(val)
? val
: []),
]
}
}
return origSetHeader(name, val)
}
const urlParts = (req.url || '').split('?')
const urlNoQuery = urlParts[0]

View file

@ -354,6 +354,12 @@ const configSchema = {
sharedPool: {
type: 'boolean',
},
skipMiddlewareUrlNormalize: {
type: 'boolean',
},
skipTrailingSlashRedirect: {
type: 'boolean',
},
sri: {
properties: {
algorithm: {

View file

@ -79,6 +79,8 @@ export interface NextJsWebpackConfig {
}
export interface ExperimentalConfig {
skipMiddlewareUrlNormalize?: boolean
skipTrailingSlashRedirect?: boolean
optimisticClientCache?: boolean
legacyBrowsers?: boolean
browsersListForSwc?: boolean

View file

@ -84,7 +84,7 @@ import { normalizePagePath } from '../shared/lib/page-path/normalize-page-path'
import { loadComponents } from './load-components'
import isError, { getProperError } from '../lib/is-error'
import { FontManifest } from './font-utils'
import { toNodeHeaders } from './web/utils'
import { splitCookiesString, toNodeHeaders } from './web/utils'
import { relativizeURL } from '../shared/lib/router/utils/relativize-url'
import { prepareDestination } from '../shared/lib/router/utils/prepare-destination'
import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path'
@ -1796,9 +1796,16 @@ export default class NextNodeServer extends BaseServer {
} else {
for (let [key, value] of allHeaders) {
result.response.headers.set(key, value)
if (key.toLowerCase() === 'set-cookie') {
addRequestMeta(
params.request,
'_nextMiddlewareCookie',
splitCookiesString(value)
)
}
}
}
return result
}
@ -2097,8 +2104,13 @@ export default class NextNodeServer extends BaseServer {
params.res.statusCode = result.response.status
params.res.statusMessage = result.response.statusText
result.response.headers.forEach((value, key) => {
params.res.appendHeader(key, value)
result.response.headers.forEach((value: string, key) => {
// the append handling is special cased for `set-cookie`
if (key.toLowerCase() === 'set-cookie') {
params.res.setHeader(key, value)
} else {
params.res.appendHeader(key, value)
}
})
if (result.response.body) {

View file

@ -21,6 +21,7 @@ export interface RequestMeta {
_nextDidRewrite?: boolean
_nextHadBasePath?: boolean
_nextRewroteUrl?: string
_nextMiddlewareCookie?: string[]
_protocol?: string
}

View file

@ -117,9 +117,11 @@ export async function adapter(params: {
nextConfig: params.request.nextConfig,
})
if (rewriteUrl.host === request.nextUrl.host) {
rewriteUrl.buildId = buildId || rewriteUrl.buildId
response.headers.set('x-middleware-rewrite', String(rewriteUrl))
if (!process.env.__NEXT_NO_MIDDLEWARE_URL_NORMALIZE) {
if (rewriteUrl.host === request.nextUrl.host) {
rewriteUrl.buildId = buildId || rewriteUrl.buildId
response.headers.set('x-middleware-rewrite', String(rewriteUrl))
}
}
/**
@ -154,9 +156,11 @@ export async function adapter(params: {
*/
response = new Response(response.body, response)
if (redirectURL.host === request.nextUrl.host) {
redirectURL.buildId = buildId || redirectURL.buildId
response.headers.set('Location', String(redirectURL))
if (!process.env.__NEXT_NO_MIDDLEWARE_URL_NORMALIZE) {
if (redirectURL.host === request.nextUrl.host) {
redirectURL.buildId = buildId || redirectURL.buildId
response.headers.set('Location', String(redirectURL))
}
}
/**

View file

@ -0,0 +1,27 @@
import { NextResponse } from 'next/server'
export default function handler(req) {
if (req.nextUrl.pathname === '/middleware-rewrite-with-slash') {
return NextResponse.rewrite(new URL('/another/', req.nextUrl))
}
if (req.nextUrl.pathname === '/middleware-rewrite-without-slash') {
return NextResponse.rewrite(new URL('/another', req.nextUrl))
}
if (req.nextUrl.pathname === '/middleware-redirect-external-with') {
return NextResponse.redirect('https://example.vercel.sh/somewhere/', 307)
}
if (req.nextUrl.pathname === '/middleware-redirect-external-without') {
return NextResponse.redirect('https://example.vercel.sh/somewhere', 307)
}
if (req.nextUrl.pathname.startsWith('/api/test-cookie')) {
const res = NextResponse.next()
res.cookies.set('from-middleware', 1)
return res
}
return NextResponse.next()
}

View file

@ -0,0 +1,13 @@
import Link from 'next/link'
export default function Page(props) {
return (
<>
<p id="another">another page</p>
<Link href="/">
<a id="to-index">to index</a>
</Link>
<br />
</>
)
}

View file

@ -0,0 +1,12 @@
import { NextResponse } from 'next/server'
export const config = {
runtime: 'experimental-edge',
}
export default function handler(req) {
console.log('setting cookie in api route')
const res = NextResponse.json({ name: 'API' })
res.cookies.set('hello', 'From API')
return res
}

View file

@ -0,0 +1,5 @@
export default function handler(req, res) {
console.log('setting cookie in api route')
res.setHeader('Set-Cookie', 'hello=From API')
res.status(200).json({ name: 'API' })
}

View file

@ -0,0 +1,13 @@
import Link from 'next/link'
export default function Page(props) {
return (
<>
<p id="blog">blog page</p>
<Link href="/">
<a id="to-index">to index</a>
</Link>
<br />
</>
)
}

View file

@ -0,0 +1,17 @@
import Link from 'next/link'
export default function Page(props) {
return (
<>
<p id="index">index page</p>
<Link href="/another">
<a id="to-another">to another</a>
</Link>
<br />
<Link href="/blog/first">
<a id="to-blog-first">to /blog/first</a>
</Link>
<br />
</>
)
}

View file

@ -0,0 +1,193 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { fetchViaHTTP } from 'next-test-utils'
import { join } from 'path'
import webdriver from 'next-webdriver'
describe('skip-trailing-slash-redirect', () => {
let next: NextInstance
beforeAll(async () => {
next = await createNext({
files: new FileRef(join(__dirname, 'app')),
dependencies: {},
nextConfig: {
experimental: {
skipTrailingSlashRedirect: true,
skipMiddlewareUrlNormalize: true,
},
async redirects() {
return [
{
source: '/redirect-me',
destination: '/another',
permanent: false,
},
]
},
async rewrites() {
return [
{
source: '/rewrite-me',
destination: '/another',
},
]
},
},
})
})
afterAll(() => next.destroy())
it('should merge cookies from middleware and API routes correctly', async () => {
const res = await fetchViaHTTP(next.url, '/api/test-cookie', undefined, {
redirect: 'manual',
})
expect(res.status).toBe(200)
expect(res.headers.get('set-cookie')).toEqual(
'from-middleware=1; Path=/, hello=From API'
)
})
it('should merge cookies from middleware and edge API routes correctly', async () => {
const res = await fetchViaHTTP(
next.url,
'/api/test-cookie-edge',
undefined,
{
redirect: 'manual',
}
)
expect(res.status).toBe(200)
expect(res.headers.get('set-cookie')).toEqual(
'from-middleware=1; Path=/, hello=From%20API; Path=/'
)
})
if ((global as any).isNextStart) {
it('should not have trailing slash redirects in manifest', async () => {
const routesManifest = JSON.parse(
await next.readFile('.next/routes-manifest.json')
)
expect(
routesManifest.redirects.some((redirect) => {
return (
redirect.statusCode === 308 &&
(redirect.destination === '/:path+' ||
redirect.destination === '/:path+/')
)
})
).toBe(false)
})
}
it('should correct skip URL normalizing in middleware', async () => {
let res = await fetchViaHTTP(
next.url,
'/middleware-rewrite-with-slash',
undefined,
{ redirect: 'manual', headers: { 'x-nextjs-data': '1' } }
)
expect(res.headers.get('x-nextjs-rewrite').endsWith('/another/')).toBe(true)
res = await fetchViaHTTP(
next.url,
'/middleware-rewrite-without-slash',
undefined,
{ redirect: 'manual', headers: { 'x-nextjs-data': '1' } }
)
expect(res.headers.get('x-nextjs-rewrite').endsWith('/another')).toBe(true)
res = await fetchViaHTTP(
next.url,
'/middleware-redirect-external-with',
undefined,
{ redirect: 'manual' }
)
expect(res.status).toBe(307)
expect(res.headers.get('Location')).toBe(
'https://example.vercel.sh/somewhere/'
)
res = await fetchViaHTTP(
next.url,
'/middleware-redirect-external-without',
undefined,
{ redirect: 'manual' }
)
expect(res.status).toBe(307)
expect(res.headers.get('Location')).toBe(
'https://example.vercel.sh/somewhere'
)
})
it('should apply config redirect correctly', async () => {
const res = await fetchViaHTTP(next.url, '/redirect-me', undefined, {
redirect: 'manual',
})
expect(res.status).toBe(307)
expect(new URL(res.headers.get('location'), 'http://n').pathname).toBe(
'/another'
)
})
it('should apply config rewrites correctly', async () => {
const res = await fetchViaHTTP(next.url, '/rewrite-me', undefined, {
redirect: 'manual',
})
expect(res.status).toBe(200)
expect(await res.text()).toContain('another page')
})
it('should not apply trailing slash redirect (with slash)', async () => {
const res = await fetchViaHTTP(next.url, '/another/', undefined, {
redirect: 'manual',
})
expect(res.status).toBe(200)
expect(await res.text()).toContain('another page')
})
it('should not apply trailing slash redirect (without slash)', async () => {
const res = await fetchViaHTTP(next.url, '/another', undefined, {
redirect: 'manual',
})
expect(res.status).toBe(200)
expect(await res.text()).toContain('another page')
})
it('should respond to index correctly', async () => {
const res = await fetchViaHTTP(next.url, '/', undefined, {
redirect: 'manual',
})
expect(res.status).toBe(200)
expect(await res.text()).toContain('index page')
})
it('should respond to dynamic route correctly', async () => {
const res = await fetchViaHTTP(next.url, '/blog/first', undefined, {
redirect: 'manual',
})
expect(res.status).toBe(200)
expect(await res.text()).toContain('blog page')
})
it('should navigate client side correctly', async () => {
const browser = await webdriver(next.url, '/')
expect(await browser.eval('location.pathname')).toBe('/')
await browser.elementByCss('#to-another').click()
await browser.waitForElementByCss('#another')
expect(await browser.eval('location.pathname')).toBe('/another')
await browser.back()
await browser.waitForElementByCss('#index')
expect(await browser.eval('location.pathname')).toBe('/')
await browser.elementByCss('#to-blog-first').click()
await browser.waitForElementByCss('#blog')
expect(await browser.eval('location.pathname')).toBe('/blog/first')
})
})