Make sure routeKeys are PCRE compliant

vercel · Jul 8, 2020 · b02b1fe · b02b1fe
1 parent 731cfa4
commit b02b1fe
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 1 deletion.
diff --git a/packages/next/next-server/lib/router/utils/route-regex.ts b/packages/next/next-server/lib/router/utils/route-regex.ts
@@ -51,6 +51,25 @@ export function getRouteRegex(
   // dead code eliminate for browser since it's only needed
   // while generating routes-manifest
   if (typeof window === 'undefined') {
+    let routeKeyCharCode = 97
+    let routeKeyCharLength = 1
+
+    // builds a minimal routeKey using only a-z and minimal number of characters
+    const getSafeRouteKey = () => {
+      let routeKey = ''
+
+      for (let i = 0; i < routeKeyCharLength; i++) {
+        routeKey += String.fromCharCode(routeKeyCharCode)
+        routeKeyCharCode++
+
+        if (routeKeyCharCode > 122) {
+          routeKeyCharLength++
+          routeKeyCharCode = 97
+        }
+      }
+      return routeKey
+    }
+
     const routeKeys: { [named: string]: string } = {}
 
     let namedParameterizedRoute = segments
@@ -59,7 +78,22 @@ export function getRouteRegex(
           const { key, optional, repeat } = parseParameter(segment.slice(1, -1))
           // replace any non-word characters since they can break
           // the named regex
-          const cleanedKey = key.replace(/\W/g, '')
+          let cleanedKey = key.replace(/\W/g, '')
+          let invalidKey = false
+
+          // check if the key is still invalid and fallback to using a known
+          // safe key
+          if (cleanedKey.length === 0 || cleanedKey.length > 30) {
+            invalidKey = true
+          }
+          if (!isNaN(parseInt(cleanedKey.substr(0, 1)))) {
+            invalidKey = true
+          }
+
+          if (invalidKey) {
+            cleanedKey = getSafeRouteKey()
+          }
+
           routeKeys[cleanedKey] = key
           return repeat
             ? optional

diff --git a/test/integration/dynamic-routing/pages/b/[123].js b/test/integration/dynamic-routing/pages/b/[123].js
@@ -0,0 +1,13 @@
+export const getServerSideProps = ({ params }) => {
+  console.log({ params })
+
+  return {
+    props: {
+      params,
+    },
+  }
+}
+
+export default function Page(props) {
+  return <p id="props">{JSON.stringify(props)}</p>
+}
diff --git a/test/integration/dynamic-routing/pages/c/[alongparamnameshouldbeallowedeventhoughweird].js b/test/integration/dynamic-routing/pages/c/[alongparamnameshouldbeallowedeventhoughweird].js
@@ -0,0 +1,13 @@
+export const getServerSideProps = ({ params }) => {
+  console.log({ params })
+
+  return {
+    props: {
+      params,
+    },
+  }
+}
+
+export default function Page(props) {
+  return <p id="props">{JSON.stringify(props)}</p>
+}
diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js
@@ -552,6 +552,30 @@ function runTests(dev) {
         rewrites: [],
         redirects: expect.arrayContaining([]),
         dataRoutes: [
+          {
+            dataRouteRegex: `^\\/_next\\/data\\/${escapeRegex(
+              buildId
+            )}\\/b\\/([^\\/]+?)\\.json$`,
+            namedDataRouteRegex: `^/_next/data/${escapeRegex(
+              buildId
+            )}/b/(?<a>[^/]+?)\\.json$`,
+            page: '/b/[123]',
+            routeKeys: {
+              a: '123',
+            },
+          },
+          {
+            dataRouteRegex: `^\\/_next\\/data\\/${escapeRegex(
+              buildId
+            )}\\/c\\/([^\\/]+?)\\.json$`,
+            namedDataRouteRegex: `^/_next/data/${escapeRegex(
+              buildId
+            )}/c/(?<a>[^/]+?)\\.json$`,
+            page: '/c/[alongparamnameshouldbeallowedeventhoughweird]',
+            routeKeys: {
+              a: 'alongparamnameshouldbeallowedeventhoughweird',
+            },
+          },
           {
             namedDataRouteRegex: `^/_next/data/${escapeRegex(
               buildId
@@ -596,6 +620,14 @@ function runTests(dev) {
           },
         ],
         dynamicRoutes: [
+          {
+            namedRegex: '^/b/(?<a>[^/]+?)(?:/)?$',
+            page: '/b/[123]',
+            regex: normalizeRegEx('^\\/b\\/([^\\/]+?)(?:\\/)?$'),
+            routeKeys: {
+              a: '123',
+            },
+          },
           {
             namedRegex: `^/blog/(?<name>[^/]+?)/comment/(?<id>[^/]+?)(?:/)?$`,
             page: '/blog/[name]/comment/[id]',
@@ -607,6 +639,14 @@ function runTests(dev) {
               id: 'id',
             },
           },
+          {
+            namedRegex: '^/c/(?<a>[^/]+?)(?:/)?$',
+            page: '/c/[alongparamnameshouldbeallowedeventhoughweird]',
+            regex: normalizeRegEx('^\\/c\\/([^\\/]+?)(?:\\/)?$'),
+            routeKeys: {
+              a: 'alongparamnameshouldbeallowedeventhoughweird',
+            },
+          },
           {
             namedRegex: '^/catchall\\-dash/(?<helloworld>.+?)(?:/)?$',
             page: '/catchall-dash/[...hello-world]',