From b02b1feb8364541a4153b4b6fd5078f1e15b1722 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 8 Jul 2020 12:20:11 -0500 Subject: [PATCH] Make sure routeKeys are PCRE compliant --- .../lib/router/utils/route-regex.ts | 36 ++++++++++++++++- .../dynamic-routing/pages/b/[123].js | 13 ++++++ ...aramnameshouldbeallowedeventhoughweird].js | 13 ++++++ .../dynamic-routing/test/index.test.js | 40 +++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 test/integration/dynamic-routing/pages/b/[123].js create mode 100644 test/integration/dynamic-routing/pages/c/[alongparamnameshouldbeallowedeventhoughweird].js diff --git a/packages/next/next-server/lib/router/utils/route-regex.ts b/packages/next/next-server/lib/router/utils/route-regex.ts index 7d75fbcc83a70..b0f74bef7480a 100644 --- 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 new file mode 100644 index 0000000000000..19f255609b632 --- /dev/null +++ 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

{JSON.stringify(props)}

+} diff --git a/test/integration/dynamic-routing/pages/c/[alongparamnameshouldbeallowedeventhoughweird].js b/test/integration/dynamic-routing/pages/c/[alongparamnameshouldbeallowedeventhoughweird].js new file mode 100644 index 0000000000000..19f255609b632 --- /dev/null +++ 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

{JSON.stringify(props)}

+} diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index 8b76d5c70b6df..aec27ecce67b9 100644 --- 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/(?[^/]+?)\\.json$`, + page: '/b/[123]', + routeKeys: { + a: '123', + }, + }, + { + dataRouteRegex: `^\\/_next\\/data\\/${escapeRegex( + buildId + )}\\/c\\/([^\\/]+?)\\.json$`, + namedDataRouteRegex: `^/_next/data/${escapeRegex( + buildId + )}/c/(?[^/]+?)\\.json$`, + page: '/c/[alongparamnameshouldbeallowedeventhoughweird]', + routeKeys: { + a: 'alongparamnameshouldbeallowedeventhoughweird', + }, + }, { namedDataRouteRegex: `^/_next/data/${escapeRegex( buildId @@ -596,6 +620,14 @@ function runTests(dev) { }, ], dynamicRoutes: [ + { + namedRegex: '^/b/(?[^/]+?)(?:/)?$', + page: '/b/[123]', + regex: normalizeRegEx('^\\/b\\/([^\\/]+?)(?:\\/)?$'), + routeKeys: { + a: '123', + }, + }, { namedRegex: `^/blog/(?[^/]+?)/comment/(?[^/]+?)(?:/)?$`, page: '/blog/[name]/comment/[id]', @@ -607,6 +639,14 @@ function runTests(dev) { id: 'id', }, }, + { + namedRegex: '^/c/(?[^/]+?)(?:/)?$', + page: '/c/[alongparamnameshouldbeallowedeventhoughweird]', + regex: normalizeRegEx('^\\/c\\/([^\\/]+?)(?:\\/)?$'), + routeKeys: { + a: 'alongparamnameshouldbeallowedeventhoughweird', + }, + }, { namedRegex: '^/catchall\\-dash/(?.+?)(?:/)?$', page: '/catchall-dash/[...hello-world]',