From 452b16fea365049094d1ae82701e7ba18530ab74 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 31 Mar 2021 12:52:30 -0500 Subject: [PATCH 1/2] Ensure has segments are allowed in destination --- packages/next/lib/load-custom-routes.ts | 34 +++++++++++++++++-- .../lib/router/utils/prepare-destination.ts | 2 +- test/integration/custom-routes/next.config.js | 10 ++++++ .../custom-routes/test/index.test.js | 27 +++++++++++++++ 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index d518fe16e1523..45ec7d22fe36a 100644 --- 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() + + 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,7 +398,8 @@ 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) } @@ -377,7 +407,7 @@ function checkCustomRoutes( 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 index 9421764487f14..3de8bd4d0f112 100644 --- 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 index cb5c84a40ed0c..769dfa1fbe0bc 100644 --- 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 index 7d0e6a2932f3e..3fe63aa024ab5 100644 --- 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: [], }, From 4305618b3b24e95fca1a6c225e941fc32745eb09 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 31 Mar 2021 12:56:43 -0500 Subject: [PATCH 2/2] update test --- test/integration/invalid-custom-routes/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/invalid-custom-routes/test/index.test.js b/test/integration/invalid-custom-routes/test/index.test.js index d088bbf3848ec..e161fc11df13a 100644 --- 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"}` ) }) }