diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index b4f3d14f53b9f..4c1cf29ccf0e0 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -17,6 +17,7 @@ import { fileExists } from '../lib/file-exists' import { findPagesDir } from '../lib/find-pages-dir' import loadCustomRoutes, { getRedirectStatus, + normalizeRouteRegex, Redirect, RouteType, } from '../lib/load-custom-routes' @@ -273,7 +274,7 @@ export default async function build(dir: string, conf = null): Promise { permanent: undefined, } : {}), - regex: routeRegex.source, + regex: normalizeRouteRegex(routeRegex.source), } } @@ -291,7 +292,7 @@ export default async function build(dir: string, conf = null): Promise { const routeRegex = getRouteRegex(page) return { page, - regex: routeRegex.re.source, + regex: normalizeRouteRegex(routeRegex.re.source), routeKeys: routeRegex.routeKeys, namedRegex: routeRegex.namedRegex, } @@ -611,9 +612,8 @@ export default async function build(dir: string, conf = null): Promise { if (isDynamicRoute(page)) { const routeRegex = getRouteRegex(dataRoute.replace(/\.json$/, '')) - dataRouteRegex = routeRegex.re.source.replace( - /\(\?:\\\/\)\?\$$/, - '\\.json$' + dataRouteRegex = normalizeRouteRegex( + routeRegex.re.source.replace(/\(\?:\\\/\)\?\$$/, '\\.json$') ) namedDataRouteRegex = routeRegex.namedRegex!.replace( /\(\?:\/\)\?\$$/, @@ -621,13 +621,15 @@ export default async function build(dir: string, conf = null): Promise { ) routeKeys = routeRegex.routeKeys } else { - dataRouteRegex = new RegExp( - `^${path.posix.join( - '/_next/data', - escapeStringRegexp(buildId), - `${pagePath}.json` - )}$` - ).source + dataRouteRegex = normalizeRouteRegex( + new RegExp( + `^${path.posix.join( + '/_next/data', + escapeStringRegexp(buildId), + `${pagePath}.json` + )}$` + ).source + ) } return { @@ -887,14 +889,17 @@ export default async function build(dir: string, conf = null): Promise { ) finalDynamicRoutes[tbdRoute] = { - routeRegex: getRouteRegex(tbdRoute).re.source, + routeRegex: normalizeRouteRegex(getRouteRegex(tbdRoute).re.source), dataRoute, fallback: ssgFallbackPages.has(tbdRoute) ? `${normalizedRoute}.html` : false, - dataRouteRegex: getRouteRegex( - dataRoute.replace(/\.json$/, '') - ).re.source.replace(/\(\?:\\\/\)\?\$$/, '\\.json$'), + dataRouteRegex: normalizeRouteRegex( + getRouteRegex(dataRoute.replace(/\.json$/, '')).re.source.replace( + /\(\?:\\\/\)\?\$$/, + '\\.json$' + ) + ), } }) const prerenderManifest: PrerenderManifest = { diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index 3a59c454108cf..0f08de04a8867 100644 --- a/packages/next/lib/load-custom-routes.ts +++ b/packages/next/lib/load-custom-routes.ts @@ -29,6 +29,11 @@ export function getRedirectStatus(route: Redirect): number { ) } +export function normalizeRouteRegex(regex: string) { + // clean up un-necessary escaping from regex.source which turns / into \\/ + return regex.replace(/\\\//g, '/') +} + function checkRedirect( route: Redirect ): { invalidParts: string[]; hadInvalidStatus: boolean } { 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]',