Apply experimental configs for middleware (vercel#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: vercel#40820 ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md`
Kikobeats · Oct 24, 2022 · bff3d8e · bff3d8e
1 parent 193e07c
commit bff3d8e
Show file tree

Hide file tree

Showing 15 changed files with 386 additions and 49 deletions.
diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts
@@ -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

diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts
@@ -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) {
-      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: '/:path+/',
-      destination: '/:path+',
-      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)
+      if (config.basePath) {
+        redirects.unshift({
+          source: config.basePath + '/',
+          destination: config.basePath,
+          permanent: true,
+          basePath: false,
+          locale: config.i18n ? false : undefined,
+          internal: true,
+        } as Redirect)
+      }
     }
   }
 

diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
@@ -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]
 

diff --git a/packages/next/server/config-schema.ts b/packages/next/server/config-schema.ts
@@ -354,6 +354,12 @@ const configSchema = {
         sharedPool: {
           type: 'boolean',
         },
+        skipMiddlewareUrlNormalize: {
+          type: 'boolean',
+        },
+        skipTrailingSlashRedirect: {
+          type: 'boolean',
+        },
         sri: {
           properties: {
             algorithm: {

diff --git a/packages/next/server/config-shared.ts b/packages/next/server/config-shared.ts
@@ -79,6 +79,8 @@ export interface NextJsWebpackConfig {
 }
 
 export interface ExperimentalConfig {
+  skipMiddlewareUrlNormalize?: boolean
+  skipTrailingSlashRedirect?: boolean
   optimisticClientCache?: boolean
   legacyBrowsers?: boolean
   browsersListForSwc?: boolean

diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts
@@ -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) {

diff --git a/packages/next/server/request-meta.ts b/packages/next/server/request-meta.ts
@@ -21,6 +21,7 @@ export interface RequestMeta {
   _nextDidRewrite?: boolean
   _nextHadBasePath?: boolean
   _nextRewroteUrl?: string
+  _nextMiddlewareCookie?: string[]
   _protocol?: string
 }
 

diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts
@@ -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))
+      }
     }
 
     /**

diff --git a/test/e2e/skip-trailing-slash-redirect/app/middleware.js b/test/e2e/skip-trailing-slash-redirect/app/middleware.js
@@ -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()
+}
diff --git a/test/e2e/skip-trailing-slash-redirect/app/pages/another.js b/test/e2e/skip-trailing-slash-redirect/app/pages/another.js
@@ -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 />
+    </>
+  )
+}
diff --git a/test/e2e/skip-trailing-slash-redirect/app/pages/api/test-cookie-edge.js b/test/e2e/skip-trailing-slash-redirect/app/pages/api/test-cookie-edge.js
@@ -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
+}
diff --git a/test/e2e/skip-trailing-slash-redirect/app/pages/api/test-cookie.js b/test/e2e/skip-trailing-slash-redirect/app/pages/api/test-cookie.js
@@ -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' })
+}
diff --git a/test/e2e/skip-trailing-slash-redirect/app/pages/blog/[slug].js b/test/e2e/skip-trailing-slash-redirect/app/pages/blog/[slug].js
@@ -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 />
+    </>
+  )
+}
diff --git a/test/e2e/skip-trailing-slash-redirect/app/pages/index.js b/test/e2e/skip-trailing-slash-redirect/app/pages/index.js
@@ -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 />
+    </>
+  )
+}