Ensure has segments are allowed in destination (#23588)

This ensures we gather segments from the experimental has field when validating segments used in the destination to prevent the invalid segments in the destination error from showing incorrectly. This usage has been added to the custom-routes test suite to ensure the segments are passed correctly from the has field. Fixes: #23415 ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added
vercel · Apr 1, 2021 · 65c2216 · 65c2216
1 parent 0d5baf2
commit 65c2216
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 4 deletions.
diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts
@@ -8,6 +8,7 @@ import {
 } from '../next-server/lib/constants'
 import { execOnce } from '../next-server/lib/utils'
 import * as Log from '../build/output/log'
+import { getSafeParamName } from '../next-server/lib/router/utils/prepare-destination'
 
 export type RouteHas =
   | {
@@ -325,6 +326,34 @@ function checkCustomRoutes(
       }
       sourceTokens = tokens
     }
+    const hasSegments = new Set<string>()
+
+    if (route.has) {
+      for (const hasItem of route.has) {
+        if (!hasItem.value && hasItem.key) {
+          hasSegments.add(hasItem.key)
+        }
+
+        if (hasItem.value) {
+          const matcher = new RegExp(`^${hasItem.value}$`)
+          const matches = matcher.exec('')
+
+          if (matches) {
+            if (matches.groups) {
+              Object.keys(matches.groups).forEach((groupKey) => {
+                const safeKey = getSafeParamName(groupKey)
+
+                if (safeKey && matches.groups![groupKey]) {
+                  hasSegments.add(safeKey)
+                }
+              })
+            } else {
+              hasSegments.add(hasItem.key || 'host')
+            }
+          }
+        }
+      }
+    }
 
     // make sure no unnamed patterns are attempted to be used in the
     // destination as this can cause confusion and is not allowed
@@ -369,15 +398,16 @@ function checkCustomRoutes(
             for (const token of destTokens!) {
               if (
                 typeof token === 'object' &&
-                !sourceSegments.has(token.name)
+                !sourceSegments.has(token.name) &&
+                !hasSegments.has(token.name as string)
               ) {
                 invalidDestSegments.add(token.name)
               }
             }
 
             if (invalidDestSegments.size) {
               invalidParts.push(
-                `\`destination\` has segments not in \`source\` (${[
+                `\`destination\` has segments not in \`source\` or \`has\` (${[
                   ...invalidDestSegments,
                 ].join(', ')})`
               )

diff --git a/packages/next/next-server/lib/router/utils/prepare-destination.ts b/packages/next/next-server/lib/router/utils/prepare-destination.ts
@@ -9,7 +9,7 @@ type Params = { [param: string]: any }
 
 // ensure only a-zA-Z are used for param names for proper interpolating
 // with path-to-regexp
-const getSafeParamName = (paramName: string) => {
+export const getSafeParamName = (paramName: string) => {
   let newParamName = ''
 
   for (let i = 0; i < paramName.length; i++) {

diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js
@@ -149,6 +149,16 @@ module.exports = {
           ],
           destination: '/with-params?host=1',
         },
+        {
+          source: '/has-rewrite-5',
+          has: [
+            {
+              type: 'query',
+              key: 'hasParam',
+            },
+          ],
+          destination: '/:hasParam',
+        },
       ],
       beforeFiles: [
         {

diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js
@@ -692,6 +692,22 @@ const runTests = (isDev = false) => {
     expect(res2.status).toBe(404)
   })
 
+  it('should pass has segment for rewrite correctly', async () => {
+    const res1 = await fetchViaHTTP(appPort, '/has-rewrite-5')
+    expect(res1.status).toBe(404)
+
+    const res = await fetchViaHTTP(appPort, '/has-rewrite-5', {
+      hasParam: 'with-params',
+    })
+
+    expect(res.status).toBe(200)
+    const $ = cheerio.load(await res.text())
+
+    expect(JSON.parse($('#query').text())).toEqual({
+      hasParam: 'with-params',
+    })
+  })
+
   it('should match has rewrite correctly before files', async () => {
     const res1 = await fetchViaHTTP(appPort, '/hello')
     expect(res1.status).toBe(200)
@@ -1509,6 +1525,17 @@ const runTests = (isDev = false) => {
               regex: '^\\/has-rewrite-4$',
               source: '/has-rewrite-4',
             },
+            {
+              destination: '/:hasParam',
+              has: [
+                {
+                  key: 'hasParam',
+                  type: 'query',
+                },
+              ],
+              regex: normalizeRegEx('^\\/has-rewrite-5$'),
+              source: '/has-rewrite-5',
+            },
           ],
           fallback: [],
         },

diff --git a/test/integration/invalid-custom-routes/test/index.test.js b/test/integration/invalid-custom-routes/test/index.test.js
@@ -542,7 +542,7 @@ const runTests = () => {
     const stderr = await getStderr()
 
     expect(stderr).toContain(
-      `\`destination\` has segments not in \`source\` (id) for route {"source":"/feedback/:type","destination":"/feedback/:id"}`
+      `\`destination\` has segments not in \`source\` or \`has\` (id) for route {"source":"/feedback/:type","destination":"/feedback/:id"}`
     )
   })
 }