From fdf44a22b07c30a1549278ea17eb8f8e9a9a155f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 2 Aug 2021 17:34:44 -0500 Subject: [PATCH] Enforce source/destination limit for custom routes (#27703) This ensures we enforce a limit for `source`/`destination` values on rewrites, redirects, and headers since these being too long can affect routing performance. Closes: https://github.com/vercel/next.js/issues/27696 --- packages/next/lib/load-custom-routes.ts | 46 ++++++++++++++++--- .../invalid-custom-routes/test/index.test.js | 26 +++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/packages/next/lib/load-custom-routes.ts b/packages/next/lib/load-custom-routes.ts index 57e7290142484..1c1a3bdf0cf80 100644 --- a/packages/next/lib/load-custom-routes.ts +++ b/packages/next/lib/load-custom-routes.ts @@ -131,6 +131,7 @@ function checkHeader(route: Header): string[] { type ParseAttemptResult = { error?: boolean tokens?: pathToRegexp.Token[] + regexStr?: string } function tryParsePath(route: string, handleUrl?: boolean): ParseAttemptResult { @@ -147,7 +148,9 @@ function tryParsePath(route: string, handleUrl?: boolean): ParseAttemptResult { // Make sure we can parse the source properly result.tokens = pathToRegexp.parse(routePath) - pathToRegexp.tokensToRegexp(result.tokens) + + const regex = pathToRegexp.tokensToRegexp(result.tokens) + result.regexStr = regex.source } catch (err) { // If there is an error show our error link but still show original error or a formatted one if we can const errMatches = err.message.match(/at (\d{0,})/) @@ -328,11 +331,16 @@ function checkCustomRoutes( if (typeof route.source === 'string' && route.source.startsWith('/')) { // only show parse error if we didn't already show error // for not being a string - const { tokens, error } = tryParsePath(route.source) + const { tokens, error, regexStr } = tryParsePath(route.source) if (error) { invalidParts.push('`source` parse failed') } + + if (regexStr && regexStr.length > 4096) { + invalidParts.push('`source` exceeds max built length of 4096') + } + sourceTokens = tokens } const hasSegments = new Set() @@ -384,9 +392,14 @@ function checkCustomRoutes( } else { const { tokens: destTokens, + regexStr: destRegexStr, error: destinationParseFailed, } = tryParsePath((route as Rewrite).destination, true) + if (destRegexStr && destRegexStr.length > 4096) { + invalidParts.push('`destination` exceeds max built length of 4096') + } + if (destinationParseFailed) { invalidParts.push('`destination` parse failed') } else { @@ -563,8 +576,13 @@ async function loadRedirects(config: NextConfig) { return [] } let redirects = await config.redirects() + // check before we process the routes and after to ensure + // they are still valid + checkCustomRoutes(redirects, 'redirect') + + redirects = processRoutes(redirects, config, 'redirect') checkCustomRoutes(redirects, 'redirect') - return processRoutes(redirects, config, 'redirect') + return redirects } async function loadRewrites(config: NextConfig) { @@ -594,15 +612,24 @@ async function loadRewrites(config: NextConfig) { } else { afterFiles = _rewrites as any } + // check before we process the routes and after to ensure + // they are still valid + checkCustomRoutes(beforeFiles, 'rewrite') + checkCustomRoutes(afterFiles, 'rewrite') + checkCustomRoutes(fallback, 'rewrite') + + beforeFiles = processRoutes(beforeFiles, config, 'rewrite') + afterFiles = processRoutes(afterFiles, config, 'rewrite') + fallback = processRoutes(fallback, config, 'rewrite') checkCustomRoutes(beforeFiles, 'rewrite') checkCustomRoutes(afterFiles, 'rewrite') checkCustomRoutes(fallback, 'rewrite') return { - beforeFiles: processRoutes(beforeFiles, config, 'rewrite'), - afterFiles: processRoutes(afterFiles, config, 'rewrite'), - fallback: processRoutes(fallback, config, 'rewrite'), + beforeFiles, + afterFiles, + fallback, } } @@ -611,8 +638,13 @@ async function loadHeaders(config: NextConfig) { return [] } let headers = await config.headers() + // check before we process the routes and after to ensure + // they are still valid + checkCustomRoutes(headers, 'header') + + headers = processRoutes(headers, config, 'header') checkCustomRoutes(headers, 'header') - return processRoutes(headers, config, 'header') + return headers } export default async function loadCustomRoutes( diff --git a/test/integration/invalid-custom-routes/test/index.test.js b/test/integration/invalid-custom-routes/test/index.test.js index e161fc11df13a..3140373cea433 100644 --- a/test/integration/invalid-custom-routes/test/index.test.js +++ b/test/integration/invalid-custom-routes/test/index.test.js @@ -25,6 +25,32 @@ const writeConfig = async (routes, type = 'redirects') => { let getStderr const runTests = () => { + it('should error when source and destination length is exceeded', async () => { + await writeConfig( + [ + { + source: `/${Array(4096).join('a')}`, + destination: `/another`, + permanent: false, + }, + { + source: `/`, + destination: `/${Array(4096).join('a')}`, + permanent: false, + }, + ], + 'redirects' + ) + const stderr = await getStderr() + + expect(stderr).toContain( + '`source` exceeds max built length of 4096 for route {"source":"/aaaaaaaaaaaaaaaaaa' + ) + expect(stderr).toContain( + '`destination` exceeds max built length of 4096 for route {"source":"/","destination":"/aaaa' + ) + }) + it('should error during next build for invalid redirects', async () => { await writeConfig( [