Update redirect regexes to not match _next (#27143)

This updates redirects' regexes to not match `/_next` paths since this is currently unexpected and can easily cause a multi-match redirect to break loading client-side assets. This also fixes custom-routes not matching correctly when `trailingSlash: true/false` is used ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [x] Errors have helpful link attached, see `contributing.md` x-ref: #24683 x-ref: [slack thread](https://vercel.slack.com/archives/CGU8HUTUH/p1626159845474000)
vercel · Jul 13, 2021 · 8c3c2b7 · 8c3c2b7
1 parent e0ff050
commit 8c3c2b7
Show file tree

Hide file tree

Showing 6 changed files with 205 additions and 81 deletions.
diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts
@@ -19,6 +19,7 @@ import { findPagesDir } from '../lib/find-pages-dir'
 import loadCustomRoutes, {
   CustomRoutes,
   getRedirectStatus,
+  modifyRouteRegex,
   normalizeRouteRegex,
   Redirect,
   Rewrite,
@@ -330,6 +331,10 @@ export default async function build(
       )
     }
 
+    const restrictedRedirectPaths = ['/_next'].map((p) =>
+      config.basePath ? `${config.basePath}${p}` : p
+    )
+
     const buildCustomRoute = (
       r: {
         source: string
@@ -347,6 +352,14 @@ export default async function build(
         sensitive: false,
         delimiter: '/', // default is `/#?`, but Next does not pass query info
       })
+      let regexSource = routeRegex.source
+
+      if (!(r as any).internal) {
+        regexSource = modifyRouteRegex(
+          routeRegex.source,
+          type === 'redirect' ? restrictedRedirectPaths : undefined
+        )
+      }
 
       return {
         ...r,
@@ -356,7 +369,7 @@ export default async function build(
               permanent: undefined,
             }
           : {}),
-        regex: normalizeRouteRegex(routeRegex.source),
+        regex: normalizeRouteRegex(regexSource),
       }
     }
 

diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts
@@ -65,6 +65,22 @@ export function normalizeRouteRegex(regex: string) {
   return regex.replace(/\\\//g, '/')
 }
 
+// for redirects we restrict matching /_next and for all routes
+// we add an optional trailing slash at the end for easier
+// configuring between trailingSlash: true/false
+export function modifyRouteRegex(regex: string, restrictedPaths?: string[]) {
+  if (restrictedPaths) {
+    regex = regex.replace(
+      /\^/,
+      `^(?!${restrictedPaths
+        .map((path) => path.replace(/\//g, '\\/'))
+        .join('|')})`
+    )
+  }
+  regex = regex.replace(/\$$/, '(?:\\/)?$')
+  return regex
+}
+
 function checkRedirect(
   route: Redirect
 ): { invalidParts: string[]; hadInvalidStatus: boolean } {
@@ -525,10 +541,14 @@ function processRoutes<T>(
         }`
       }
     }
-    r.source = `${srcBasePath}${r.source}`
+    r.source = `${srcBasePath}${
+      r.source === '/' && srcBasePath ? '' : r.source
+    }`
 
     if (r.destination) {
-      r.destination = `${destBasePath}${r.destination}`
+      r.destination = `${destBasePath}${
+        r.destination === '/' && destBasePath ? '' : r.destination
+      }`
     }
     newRoutes.push(r)
   }

diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts
@@ -18,6 +18,7 @@ import {
   Rewrite,
   RouteType,
   CustomRoutes,
+  modifyRouteRegex,
 } from '../lib/load-custom-routes'
 import {
   BUILD_ID_FILE,
@@ -822,11 +823,24 @@ export default class Server {
       ...staticFilesRoute,
     ]
 
+    const restrictedRedirectPaths = ['/_next'].map((p) =>
+      this.nextConfig.basePath ? `${this.nextConfig.basePath}${p}` : p
+    )
+
     const getCustomRoute = (
       r: Rewrite | Redirect | Header,
       type: RouteType
     ) => {
-      const match = getCustomRouteMatcher(r.source)
+      const match = getCustomRouteMatcher(
+        r.source,
+        !(r as any).internal
+          ? (regex: string) =>
+              modifyRouteRegex(
+                regex,
+                type === 'redirect' ? restrictedRedirectPaths : undefined
+              )
+          : undefined
+      )
 
       return {
         ...r,

diff --git a/packages/next/shared/lib/router/utils/path-match.ts b/packages/next/shared/lib/router/utils/path-match.ts
@@ -15,13 +15,19 @@ export const customRouteMatcherOptions: pathToRegexp.TokensToRegexpOptions &
 }
 
 export default (customRoute = false) => {
-  return (path: string) => {
+  return (path: string, regexModifier?: (regex: string) => string) => {
     const keys: pathToRegexp.Key[] = []
-    const matcherRegex = pathToRegexp.pathToRegexp(
+    let matcherRegex = pathToRegexp.pathToRegexp(
       path,
       keys,
       customRoute ? customRouteMatcherOptions : matcherOptions
     )
+
+    if (regexModifier) {
+      const regexSource = regexModifier(matcherRegex.source)
+      matcherRegex = new RegExp(regexSource, matcherRegex.flags)
+    }
+
     const matcher = pathToRegexp.regexpToFunction(matcherRegex, keys)
 
     return (pathname: string | null | undefined, params?: any) => {

diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js
@@ -378,6 +378,17 @@ module.exports = {
         destination: '/another?host=1',
         permanent: false,
       },
+      {
+        source: '/:path/has-redirect-5',
+        has: [
+          {
+            type: 'header',
+            key: 'x-test-next',
+          },
+        ],
+        destination: '/somewhere',
+        permanent: false,
+      },
     ]
   },