Update routeKeys to handle non-word characters (#12801)

This updates the named regexes output in the `routes-manifest` and the associated `routeKeys` to not use any non-word characters as this breaks the named regexes e.g. `"Invalid regular expression: "^/(?<data\-provider\-id>[^/]+?)(?:/)?$"`

x-ref: https://github.com/zeit/now/pull/4355 

packages/next/build/index.ts

@@ -289,7 +289,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
 
   const routesManifestPath = path.join(distDir, ROUTES_MANIFEST)
   const routesManifest: any = {
-    version: 1,
+    version: 3,
     pages404: true,
     basePath: config.experimental.basePath,
     redirects: redirects.map((r) => buildCustomRoute(r, 'redirect')),
@@ -302,8 +302,8 @@ export default async function build(dir: string, conf = null): Promise<void> {
         return {
           page,
           regex: routeRegex.re.source,
+          routeKeys: routeRegex.routeKeys,
           namedRegex: routeRegex.namedRegex,
-          routeKeys: Object.keys(routeRegex.groups),
         }
       }),
   }
@@ -615,8 +615,8 @@ export default async function build(dir: string, conf = null): Promise<void> {
       )
 
       let dataRouteRegex: string
-      let routeKeys: string[] | undefined
       let namedDataRouteRegex: string | undefined
+      let routeKeys: { [named: string]: string } | undefined
 
       if (isDynamicRoute(page)) {
         const routeRegex = getRouteRegex(dataRoute.replace(/\.json$/, ''))
@@ -629,7 +629,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
           /\(\?:\/\)\?\$$/,
           '\\.json$'
         )
-        routeKeys = Object.keys(routeRegex.groups)
+        routeKeys = routeRegex.routeKeys
       } else {
         dataRouteRegex = new RegExp(
           `^${path.posix.join(

packages/next/next-server/lib/router/utils/route-regex.ts

@@ -9,6 +9,7 @@ export function getRouteRegex(
 ): {
   re: RegExp
   namedRegex?: string
+  routeKeys?: { [named: string]: string }
   groups: {
     [groupName: string]: { pos: number; repeat: boolean; optional: boolean }
   }
@@ -47,6 +48,8 @@ export function getRouteRegex(
   // dead code eliminate for browser since it's only needed
   // while generating routes-manifest
   if (typeof window === 'undefined') {
+    const routeKeys: { [named: string]: string } = {}
+
     namedParameterizedRoute = escapedRoute.replace(
       /\/\\\[([^/]+?)\\\](?=\/|$)/g,
       (_, $1) => {
@@ -56,18 +59,28 @@ export function getRouteRegex(
           .replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1')
           .replace(/^\.{3}/, '')
 
+        // replace any non-word characters since they can break
+        // the named regex
+        const cleanedKey = key.replace(/\W/g, '')
+
+        routeKeys[cleanedKey] = key
+
         return isCatchAll
-          ? `/(?<${escapeRegex(key)}>.+?)`
-          : `/(?<${escapeRegex(key)}>[^/]+?)`
+          ? `/(?<${cleanedKey}>.+?)`
+          : `/(?<${cleanedKey}>[^/]+?)`
       }
     )
+
+    return {
+      re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'),
+      groups,
+      routeKeys,
+      namedRegex: `^${namedParameterizedRoute}(?:/)?$`,
+    }
   }
 
   return {
     re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'),
     groups,
-    namedRegex: namedParameterizedRoute
-      ? `^${namedParameterizedRoute}(?:/)?$`
-      : undefined,
   }
 }

packages/next/next-server/lib/router/utils/sorted-routes.ts

@@ -127,11 +127,19 @@ class UrlNode {
           }
         }
 
-        if (slugNames.indexOf(nextSlug) !== -1) {
-          throw new Error(
-            `You cannot have the same slug name "${nextSlug}" repeat within a single dynamic path`
-          )
-        }
+        slugNames.forEach((slug) => {
+          if (slug === nextSlug) {
+            throw new Error(
+              `You cannot have the same slug name "${nextSlug}" repeat within a single dynamic path`
+            )
+          }
+
+          if (slug.replace(/\W/g, '') === nextSegment.replace(/\W/g, '')) {
+            throw new Error(
+              `You cannot have the slug names "${slug}" and "${nextSlug}" differ only by non-word symbols within a single dynamic path`
+            )
+          }
+        })
 
         slugNames.push(nextSlug)
       }

test/integration/custom-routes/test/index.test.js

@@ -491,7 +491,7 @@ const runTests = (isDev = false) => {
       }
 
       expect(manifest).toEqual({
-        version: 1,
+        version: 3,
         pages404: true,
         basePath: '',
         redirects: [
@@ -872,19 +872,25 @@ const runTests = (isDev = false) => {
             namedRegex: '^/another/(?<id>[^/]+?)(?:/)?$',
             page: '/another/[id]',
             regex: normalizeRegEx('^\\/another\\/([^\\/]+?)(?:\\/)?$'),
-            routeKeys: ['id'],
+            routeKeys: {
+              id: 'id',
+            },
           },
           {
             namedRegex: '^/api/dynamic/(?<slug>[^/]+?)(?:/)?$',
             page: '/api/dynamic/[slug]',
             regex: normalizeRegEx('^\\/api\\/dynamic\\/([^\\/]+?)(?:\\/)?$'),
-            routeKeys: ['slug'],
+            routeKeys: {
+              slug: 'slug',
+            },
           },
           {
             namedRegex: '^/blog/(?<post>[^/]+?)(?:/)?$',
             page: '/blog/[post]',
             regex: normalizeRegEx('^\\/blog\\/([^\\/]+?)(?:\\/)?$'),
-            routeKeys: ['post'],
+            routeKeys: {
+              post: 'post',
+            },
           },
         ],
       })

test/integration/dynamic-routing/pages/catchall-dash/[...hello-world].js

+export default () => 'hi'

test/integration/dynamic-routing/pages/dash/[hello-world].js

+export default () => 'hi'

test/integration/dynamic-routing/test/index.test.js

@@ -515,14 +515,22 @@ function runTests(dev) {
 
       for (const route of manifest.dynamicRoutes) {
         route.regex = normalizeRegEx(route.regex)
+
+        // ensure regexes are valid
+        new RegExp(route.regex)
+        new RegExp(route.namedRegex)
       }
 
       for (const route of manifest.dataRoutes) {
         route.dataRouteRegex = normalizeRegEx(route.dataRouteRegex)
+
+        // ensure regexes are valid
+        new RegExp(route.dataRouteRegex)
+        new RegExp(route.namedDataRouteRegex)
       }
 
       expect(manifest).toEqual({
-        version: 1,
+        version: 3,
         pages404: true,
         basePath: '',
         headers: [],
@@ -539,7 +547,9 @@ function runTests(dev) {
               )}\\/p1\\/p2\\/all\\-ssg\\/(.+?)\\.json$`
             ),
             page: '/p1/p2/all-ssg/[...rest]',
-            routeKeys: ['rest'],
+            routeKeys: {
+              rest: 'rest',
+            },
           },
           {
             namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -551,7 +561,9 @@ function runTests(dev) {
               )}\\/p1\\/p2\\/nested\\-all\\-ssg\\/(.+?)\\.json$`
             ),
             page: '/p1/p2/nested-all-ssg/[...rest]',
-            routeKeys: ['rest'],
+            routeKeys: {
+              rest: 'rest',
+            },
           },
           {
             namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -563,7 +575,9 @@ function runTests(dev) {
               )}\\/p1\\/p2\\/predefined\\-ssg\\/(.+?)\\.json$`
             ),
             page: '/p1/p2/predefined-ssg/[...rest]',
-            routeKeys: ['rest'],
+            routeKeys: {
+              rest: 'rest',
+            },
           },
         ],
         dynamicRoutes: [
@@ -573,25 +587,50 @@ function runTests(dev) {
             regex: normalizeRegEx(
               '^\\/blog\\/([^\\/]+?)\\/comment\\/([^\\/]+?)(?:\\/)?$'
             ),
-            routeKeys: ['name', 'id'],
+            routeKeys: {
+              name: 'name',
+              id: 'id',
+            },
+          },
+          {
+            namedRegex: '^/catchall\\-dash/(?<helloworld>.+?)(?:/)?$',
+            page: '/catchall-dash/[...hello-world]',
+            regex: normalizeRegEx('^\\/catchall\\-dash\\/(.+?)(?:\\/)?$'),
+            routeKeys: {
+              helloworld: 'hello-world',
+            },
+          },
+          {
+            namedRegex: '^/dash/(?<helloworld>[^/]+?)(?:/)?$',
+            page: '/dash/[hello-world]',
+            regex: normalizeRegEx('^\\/dash\\/([^\\/]+?)(?:\\/)?$'),
+            routeKeys: {
+              helloworld: 'hello-world',
+            },
           },
           {
             namedRegex: `^/on\\-mount/(?<post>[^/]+?)(?:/)?$`,
             page: '/on-mount/[post]',
             regex: normalizeRegEx('^\\/on\\-mount\\/([^\\/]+?)(?:\\/)?$'),
-            routeKeys: ['post'],
+            routeKeys: {
+              post: 'post',
+            },
           },
           {
             namedRegex: `^/p1/p2/all\\-ssg/(?<rest>.+?)(?:/)?$`,
             page: '/p1/p2/all-ssg/[...rest]',
             regex: normalizeRegEx('^\\/p1\\/p2\\/all\\-ssg\\/(.+?)(?:\\/)?$'),
-            routeKeys: ['rest'],
+            routeKeys: {
+              rest: 'rest',
+            },
           },
           {
             namedRegex: `^/p1/p2/all\\-ssr/(?<rest>.+?)(?:/)?$`,
             page: '/p1/p2/all-ssr/[...rest]',
             regex: normalizeRegEx('^\\/p1\\/p2\\/all\\-ssr\\/(.+?)(?:\\/)?$'),
-            routeKeys: ['rest'],
+            routeKeys: {
+              rest: 'rest',
+            },
           },
           {
             namedRegex: `^/p1/p2/nested\\-all\\-ssg/(?<rest>.+?)(?:/)?$`,
@@ -599,7 +638,9 @@ function runTests(dev) {
             regex: normalizeRegEx(
               '^\\/p1\\/p2\\/nested\\-all\\-ssg\\/(.+?)(?:\\/)?$'
             ),
-            routeKeys: ['rest'],
+            routeKeys: {
+              rest: 'rest',
+            },
           },
           {
             namedRegex: `^/p1/p2/predefined\\-ssg/(?<rest>.+?)(?:/)?$`,
@@ -607,19 +648,25 @@ function runTests(dev) {
             regex: normalizeRegEx(
               '^\\/p1\\/p2\\/predefined\\-ssg\\/(.+?)(?:\\/)?$'
             ),
-            routeKeys: ['rest'],
+            routeKeys: {
+              rest: 'rest',
+            },
           },
           {
             namedRegex: `^/(?<name>[^/]+?)(?:/)?$`,
             page: '/[name]',
             regex: normalizeRegEx('^\\/([^\\/]+?)(?:\\/)?$'),
-            routeKeys: ['name'],
+            routeKeys: {
+              name: 'name',
+            },
           },
           {
             namedRegex: `^/(?<name>[^/]+?)/comments(?:/)?$`,
             page: '/[name]/comments',
             regex: normalizeRegEx('^\\/([^\\/]+?)\\/comments(?:\\/)?$'),
-            routeKeys: ['name'],
+            routeKeys: {
+              name: 'name',
+            },
           },
           {
             namedRegex: `^/(?<name>[^/]+?)/on\\-mount\\-redir(?:/)?$`,
@@ -627,13 +674,18 @@ function runTests(dev) {
             regex: normalizeRegEx(
               '^\\/([^\\/]+?)\\/on\\-mount\\-redir(?:\\/)?$'
             ),
-            routeKeys: ['name'],
+            routeKeys: {
+              name: 'name',
+            },
           },
           {
             namedRegex: `^/(?<name>[^/]+?)/(?<comment>[^/]+?)(?:/)?$`,
             page: '/[name]/[comment]',
             regex: normalizeRegEx('^\\/([^\\/]+?)\\/([^\\/]+?)(?:\\/)?$'),
-            routeKeys: ['name', 'comment'],
+            routeKeys: {
+              name: 'name',
+              comment: 'comment',
+            },
           },
         ],
       })

test/integration/getserversideprops/test/index.test.js

@@ -55,7 +55,9 @@ const expectedManifestRoutes = () => [
       `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/blog\\/([^\\/]+?)\\.json$`
     ),
     page: '/blog/[post]',
-    routeKeys: ['post'],
+    routeKeys: {
+      post: 'post',
+    },
   },
   {
     namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -67,7 +69,10 @@ const expectedManifestRoutes = () => [
       )}\\/blog\\/([^\\/]+?)\\/([^\\/]+?)\\.json$`
     ),
     page: '/blog/[post]/[comment]',
-    routeKeys: ['post', 'comment'],
+    routeKeys: {
+      post: 'post',
+      comment: 'comment',
+    },
   },
   {
     namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -77,7 +82,9 @@ const expectedManifestRoutes = () => [
       `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/catchall\\/(.+?)\\.json$`
     ),
     page: '/catchall/[...path]',
-    routeKeys: ['path'],
+    routeKeys: {
+      path: 'path',
+    },
   },
   {
     dataRouteRegex: normalizeRegEx(
@@ -131,7 +138,9 @@ const expectedManifestRoutes = () => [
       )}\\/user\\/([^\\/]+?)\\/profile\\.json$`
     ),
     page: '/user/[user]/profile',
-    routeKeys: ['user'],
+    routeKeys: {
+      user: 'user',
+    },
   },
 ]
 

test/integration/prerender/test/index.test.js

@@ -860,7 +860,9 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             )}\\/blog\\/([^\\/]+?)\\.json$`
           ),
           page: '/blog/[post]',
-          routeKeys: ['post'],
+          routeKeys: {
+            post: 'post',
+          },
         },
         {
           namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -872,7 +874,10 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             )}\\/blog\\/([^\\/]+?)\\/([^\\/]+?)\\.json$`
           ),
           page: '/blog/[post]/[comment]',
-          routeKeys: ['post', 'comment'],
+          routeKeys: {
+            post: 'post',
+            comment: 'comment',
+          },
         },
         {
           namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -884,7 +889,9 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             )}\\/catchall\\/(.+?)\\.json$`
           ),
           page: '/catchall/[...slug]',
-          routeKeys: ['slug'],
+          routeKeys: {
+            slug: 'slug',
+          },
         },
         {
           namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -896,7 +903,9 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             )}\\/catchall\\-explicit\\/(.+?)\\.json$`
           ),
           page: '/catchall-explicit/[...slug]',
-          routeKeys: ['slug'],
+          routeKeys: {
+            slug: 'slug',
+          },
         },
         {
           dataRouteRegex: normalizeRegEx(
@@ -916,7 +925,9 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             buildId
           )}/fallback\\-only/(?<slug>[^/]+?)\\.json$`,
           page: '/fallback-only/[slug]',
-          routeKeys: ['slug'],
+          routeKeys: {
+            slug: 'slug',
+          },
         },
         {
           namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -928,7 +939,9 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             )}\\/lang\\/([^\\/]+?)\\/about\\.json$`
           ),
           page: '/lang/[lang]/about',
-          routeKeys: ['lang'],
+          routeKeys: {
+            lang: 'lang',
+          },
         },
         {
           namedDataRouteRegex: `^/_next/data/${escapeRegex(
@@ -940,7 +953,9 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             )}\\/non\\-json\\/([^\\/]+?)\\.json$`
           ),
           page: '/non-json/[p]',
-          routeKeys: ['p'],
+          routeKeys: {
+            p: 'p',
+          },
         },
         {
           dataRouteRegex: normalizeRegEx(
@@ -958,7 +973,9 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             )}\\/user\\/([^\\/]+?)\\/profile\\.json$`
           ),
           page: '/user/[user]/profile',
-          routeKeys: ['user'],
+          routeKeys: {
+            user: 'user',
+          },
         },
       ])
     })

test/unit/page-route-sorter.test.js

@@ -204,4 +204,13 @@ describe('getSortedRoutes', () => {
       `"You cannot define a route with the same specificity as a optional catch-all route (\\"/sub\\" and \\"/sub[[...all]]\\")."`
     )
   })
+
+  it('catches param names differing only by non-word characters', () => {
+    expect(() =>
+      getSortedRoutes([
+        '/blog/[helloworld]',
+        '/blog/[helloworld]/[hello-world]',
+      ])
+    ).toThrowError(/differ only by non-word/)
+  })
 })