Skip to content

Commit

Permalink
Update routeKeys to handle non-word characters (vercel#12801)
Browse files Browse the repository at this point in the history
This updates the named regexes output in the `routes-manifest` and the associated `routeKeys` to not use any non-word characters as this breaks the named regexes e.g. `"Invalid regular expression: "^/(?<data\-provider\-id>[^/]+?)(?:/)?$"`

x-ref: vercel/vercel#4355
  • Loading branch information
ijjk authored and rokinsky committed Jul 11, 2020
1 parent 389dce1 commit 4af92c2
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 44 deletions.
8 changes: 4 additions & 4 deletions packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ export default async function build(dir: string, conf = null): Promise<void> {

const routesManifestPath = path.join(distDir, ROUTES_MANIFEST)
const routesManifest: any = {
version: 1,
version: 3,
pages404: true,
basePath: config.experimental.basePath,
redirects: redirects.map((r) => buildCustomRoute(r, 'redirect')),
Expand All @@ -302,8 +302,8 @@ export default async function build(dir: string, conf = null): Promise<void> {
return {
page,
regex: routeRegex.re.source,
routeKeys: routeRegex.routeKeys,
namedRegex: routeRegex.namedRegex,
routeKeys: Object.keys(routeRegex.groups),
}
}),
}
Expand Down Expand Up @@ -615,8 +615,8 @@ export default async function build(dir: string, conf = null): Promise<void> {
)

let dataRouteRegex: string
let routeKeys: string[] | undefined
let namedDataRouteRegex: string | undefined
let routeKeys: { [named: string]: string } | undefined

if (isDynamicRoute(page)) {
const routeRegex = getRouteRegex(dataRoute.replace(/\.json$/, ''))
Expand All @@ -629,7 +629,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
/\(\?:\/\)\?\$$/,
'\\.json$'
)
routeKeys = Object.keys(routeRegex.groups)
routeKeys = routeRegex.routeKeys
} else {
dataRouteRegex = new RegExp(
`^${path.posix.join(
Expand Down
23 changes: 18 additions & 5 deletions packages/next/next-server/lib/router/utils/route-regex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export function getRouteRegex(
): {
re: RegExp
namedRegex?: string
routeKeys?: { [named: string]: string }
groups: {
[groupName: string]: { pos: number; repeat: boolean; optional: boolean }
}
Expand Down Expand Up @@ -47,6 +48,8 @@ export function getRouteRegex(
// dead code eliminate for browser since it's only needed
// while generating routes-manifest
if (typeof window === 'undefined') {
const routeKeys: { [named: string]: string } = {}

namedParameterizedRoute = escapedRoute.replace(
/\/\\\[([^/]+?)\\\](?=\/|$)/g,
(_, $1) => {
Expand All @@ -56,18 +59,28 @@ export function getRouteRegex(
.replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1')
.replace(/^\.{3}/, '')

// replace any non-word characters since they can break
// the named regex
const cleanedKey = key.replace(/\W/g, '')

routeKeys[cleanedKey] = key

return isCatchAll
? `/(?<${escapeRegex(key)}>.+?)`
: `/(?<${escapeRegex(key)}>[^/]+?)`
? `/(?<${cleanedKey}>.+?)`
: `/(?<${cleanedKey}>[^/]+?)`
}
)

return {
re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'),
groups,
routeKeys,
namedRegex: `^${namedParameterizedRoute}(?:/)?$`,
}
}

return {
re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'),
groups,
namedRegex: namedParameterizedRoute
? `^${namedParameterizedRoute}(?:/)?$`
: undefined,
}
}
18 changes: 13 additions & 5 deletions packages/next/next-server/lib/router/utils/sorted-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,19 @@ class UrlNode {
}
}

if (slugNames.indexOf(nextSlug) !== -1) {
throw new Error(
`You cannot have the same slug name "${nextSlug}" repeat within a single dynamic path`
)
}
slugNames.forEach((slug) => {
if (slug === nextSlug) {
throw new Error(
`You cannot have the same slug name "${nextSlug}" repeat within a single dynamic path`
)
}

if (slug.replace(/\W/g, '') === nextSegment.replace(/\W/g, '')) {
throw new Error(
`You cannot have the slug names "${slug}" and "${nextSlug}" differ only by non-word symbols within a single dynamic path`
)
}
})

slugNames.push(nextSlug)
}
Expand Down
14 changes: 10 additions & 4 deletions test/integration/custom-routes/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ const runTests = (isDev = false) => {
}

expect(manifest).toEqual({
version: 1,
version: 3,
pages404: true,
basePath: '',
redirects: [
Expand Down Expand Up @@ -872,19 +872,25 @@ const runTests = (isDev = false) => {
namedRegex: '^/another/(?<id>[^/]+?)(?:/)?$',
page: '/another/[id]',
regex: normalizeRegEx('^\\/another\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['id'],
routeKeys: {
id: 'id',
},
},
{
namedRegex: '^/api/dynamic/(?<slug>[^/]+?)(?:/)?$',
page: '/api/dynamic/[slug]',
regex: normalizeRegEx('^\\/api\\/dynamic\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['slug'],
routeKeys: {
slug: 'slug',
},
},
{
namedRegex: '^/blog/(?<post>[^/]+?)(?:/)?$',
page: '/blog/[post]',
regex: normalizeRegEx('^\\/blog\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['post'],
routeKeys: {
post: 'post',
},
},
],
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => 'hi'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => 'hi'
80 changes: 66 additions & 14 deletions test/integration/dynamic-routing/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,14 +515,22 @@ function runTests(dev) {

for (const route of manifest.dynamicRoutes) {
route.regex = normalizeRegEx(route.regex)

// ensure regexes are valid
new RegExp(route.regex)
new RegExp(route.namedRegex)
}

for (const route of manifest.dataRoutes) {
route.dataRouteRegex = normalizeRegEx(route.dataRouteRegex)

// ensure regexes are valid
new RegExp(route.dataRouteRegex)
new RegExp(route.namedDataRouteRegex)
}

expect(manifest).toEqual({
version: 1,
version: 3,
pages404: true,
basePath: '',
headers: [],
Expand All @@ -539,7 +547,9 @@ function runTests(dev) {
)}\\/p1\\/p2\\/all\\-ssg\\/(.+?)\\.json$`
),
page: '/p1/p2/all-ssg/[...rest]',
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -551,7 +561,9 @@ function runTests(dev) {
)}\\/p1\\/p2\\/nested\\-all\\-ssg\\/(.+?)\\.json$`
),
page: '/p1/p2/nested-all-ssg/[...rest]',
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -563,7 +575,9 @@ function runTests(dev) {
)}\\/p1\\/p2\\/predefined\\-ssg\\/(.+?)\\.json$`
),
page: '/p1/p2/predefined-ssg/[...rest]',
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
],
dynamicRoutes: [
Expand All @@ -573,67 +587,105 @@ function runTests(dev) {
regex: normalizeRegEx(
'^\\/blog\\/([^\\/]+?)\\/comment\\/([^\\/]+?)(?:\\/)?$'
),
routeKeys: ['name', 'id'],
routeKeys: {
name: 'name',
id: 'id',
},
},
{
namedRegex: '^/catchall\\-dash/(?<helloworld>.+?)(?:/)?$',
page: '/catchall-dash/[...hello-world]',
regex: normalizeRegEx('^\\/catchall\\-dash\\/(.+?)(?:\\/)?$'),
routeKeys: {
helloworld: 'hello-world',
},
},
{
namedRegex: '^/dash/(?<helloworld>[^/]+?)(?:/)?$',
page: '/dash/[hello-world]',
regex: normalizeRegEx('^\\/dash\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: {
helloworld: 'hello-world',
},
},
{
namedRegex: `^/on\\-mount/(?<post>[^/]+?)(?:/)?$`,
page: '/on-mount/[post]',
regex: normalizeRegEx('^\\/on\\-mount\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['post'],
routeKeys: {
post: 'post',
},
},
{
namedRegex: `^/p1/p2/all\\-ssg/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/all-ssg/[...rest]',
regex: normalizeRegEx('^\\/p1\\/p2\\/all\\-ssg\\/(.+?)(?:\\/)?$'),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/p1/p2/all\\-ssr/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/all-ssr/[...rest]',
regex: normalizeRegEx('^\\/p1\\/p2\\/all\\-ssr\\/(.+?)(?:\\/)?$'),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/p1/p2/nested\\-all\\-ssg/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/nested-all-ssg/[...rest]',
regex: normalizeRegEx(
'^\\/p1\\/p2\\/nested\\-all\\-ssg\\/(.+?)(?:\\/)?$'
),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/p1/p2/predefined\\-ssg/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/predefined-ssg/[...rest]',
regex: normalizeRegEx(
'^\\/p1\\/p2\\/predefined\\-ssg\\/(.+?)(?:\\/)?$'
),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/(?<name>[^/]+?)(?:/)?$`,
page: '/[name]',
regex: normalizeRegEx('^\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['name'],
routeKeys: {
name: 'name',
},
},
{
namedRegex: `^/(?<name>[^/]+?)/comments(?:/)?$`,
page: '/[name]/comments',
regex: normalizeRegEx('^\\/([^\\/]+?)\\/comments(?:\\/)?$'),
routeKeys: ['name'],
routeKeys: {
name: 'name',
},
},
{
namedRegex: `^/(?<name>[^/]+?)/on\\-mount\\-redir(?:/)?$`,
page: '/[name]/on-mount-redir',
regex: normalizeRegEx(
'^\\/([^\\/]+?)\\/on\\-mount\\-redir(?:\\/)?$'
),
routeKeys: ['name'],
routeKeys: {
name: 'name',
},
},
{
namedRegex: `^/(?<name>[^/]+?)/(?<comment>[^/]+?)(?:/)?$`,
page: '/[name]/[comment]',
regex: normalizeRegEx('^\\/([^\\/]+?)\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['name', 'comment'],
routeKeys: {
name: 'name',
comment: 'comment',
},
},
],
})
Expand Down
17 changes: 13 additions & 4 deletions test/integration/getserversideprops/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ const expectedManifestRoutes = () => [
`^\\/_next\\/data\\/${escapeRegex(buildId)}\\/blog\\/([^\\/]+?)\\.json$`
),
page: '/blog/[post]',
routeKeys: ['post'],
routeKeys: {
post: 'post',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -67,7 +69,10 @@ const expectedManifestRoutes = () => [
)}\\/blog\\/([^\\/]+?)\\/([^\\/]+?)\\.json$`
),
page: '/blog/[post]/[comment]',
routeKeys: ['post', 'comment'],
routeKeys: {
post: 'post',
comment: 'comment',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -77,7 +82,9 @@ const expectedManifestRoutes = () => [
`^\\/_next\\/data\\/${escapeRegex(buildId)}\\/catchall\\/(.+?)\\.json$`
),
page: '/catchall/[...path]',
routeKeys: ['path'],
routeKeys: {
path: 'path',
},
},
{
dataRouteRegex: normalizeRegEx(
Expand Down Expand Up @@ -131,7 +138,9 @@ const expectedManifestRoutes = () => [
)}\\/user\\/([^\\/]+?)\\/profile\\.json$`
),
page: '/user/[user]/profile',
routeKeys: ['user'],
routeKeys: {
user: 'user',
},
},
]

Expand Down
Loading

0 comments on commit 4af92c2

Please sign in to comment.