diff --git a/packages/next/next-server/lib/router/utils/path-match.ts b/packages/next/next-server/lib/router/utils/path-match.ts index cbf785b5c6510..8633a35cc5fb4 100644 --- a/packages/next/next-server/lib/router/utils/path-match.ts +++ b/packages/next/next-server/lib/router/utils/path-match.ts @@ -2,13 +2,14 @@ import * as pathToRegexp from 'next/dist/compiled/path-to-regexp' export { pathToRegexp } -export const matcherOptions = { +export const matcherOptions: pathToRegexp.TokensToRegexpOptions & + pathToRegexp.ParseOptions = { sensitive: false, delimiter: '/', - decode: decodeParam, } -export const customRouteMatcherOptions = { +export const customRouteMatcherOptions: pathToRegexp.TokensToRegexpOptions & + pathToRegexp.ParseOptions = { ...matcherOptions, strict: true, } @@ -21,11 +22,7 @@ export default (customRoute = false) => { keys, customRoute ? customRouteMatcherOptions : matcherOptions ) - const matcher = pathToRegexp.regexpToFunction( - matcherRegex, - keys, - matcherOptions - ) + const matcher = pathToRegexp.regexpToFunction(matcherRegex, keys) return (pathname: string | null | undefined, params?: any) => { const res = pathname == null ? false : matcher(pathname) @@ -47,13 +44,3 @@ export default (customRoute = false) => { } } } - -function decodeParam(param: string) { - try { - return decodeURIComponent(param) - } catch (_) { - const err: Error & { code?: string } = new Error('failed to decode param') - err.code = 'DECODE_FAILED' - throw err - } -} 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 72c51f1b3adbb..90df77c446c5b 100644 --- a/packages/next/next-server/lib/router/utils/prepare-destination.ts +++ b/packages/next/next-server/lib/router/utils/prepare-destination.ts @@ -97,8 +97,8 @@ export default function prepareDestination( const shouldAddBasePath = destination.startsWith('/') && basePath try { - newUrl = `${shouldAddBasePath ? basePath : ''}${encodeURI( - destinationCompiler(params) + newUrl = `${shouldAddBasePath ? basePath : ''}${destinationCompiler( + params )}` const [pathname, hash] = newUrl.split('#') diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 268a3c9f11d24..b005f719114af 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -4,7 +4,11 @@ import chalk from 'next/dist/compiled/chalk' import { IncomingMessage, ServerResponse } from 'http' import Proxy from 'next/dist/compiled/http-proxy' import { join, relative, resolve, sep } from 'path' -import { parse as parseQs, ParsedUrlQuery } from 'querystring' +import { + parse as parseQs, + stringify as stringifyQs, + ParsedUrlQuery, +} from 'querystring' import { format as formatUrl, parse as parseUrl, UrlWithParsedQuery } from 'url' import { PrerenderManifest } from '../../build' import { @@ -352,11 +356,7 @@ export default class Server { match: route('/static/:path*'), name: 'static catchall', fn: async (req, res, params, parsedUrl) => { - const p = join( - this.dir, - 'static', - ...(params.path || []).map(encodeURIComponent) - ) + const p = join(this.dir, 'static', ...params.path) await this.serveStatic(req, res, p, parsedUrl) return { finished: true, @@ -428,11 +428,7 @@ export default class Server { // re-create page's pathname const pathname = getRouteFromAssetPath( - `/${params.path - // we need to re-encode the params since they are decoded - // by path-match and we are re-building the URL - .map((param: string) => encodeURIComponent(param)) - .join('/')}`, + `/${params.path.join('/')}`, '.json' ) @@ -559,6 +555,14 @@ export default class Server { false, getCustomRouteBasePath(redirectRoute) ) + + const { query } = parsedDestination + delete parsedDestination.query + + parsedDestination.search = stringifyQs(query, undefined, undefined, { + encodeURIComponent: (str: string) => str, + } as any) + const updatedDestination = formatUrl(parsedDestination) res.setHeader('Location', updatedDestination) @@ -597,6 +601,15 @@ export default class Server { // external rewrite, proxy it if (parsedDestination.protocol) { + const { query } = parsedDestination + delete parsedDestination.query + parsedDestination.search = stringifyQs( + query, + undefined, + undefined, + { encodeURIComponent: (str) => str } + ) + const target = formatUrl(parsedDestination) const proxy = new Proxy({ target, @@ -775,7 +788,9 @@ export default class Server { protected generatePublicRoutes(): Route[] { const publicFiles = new Set( - recursiveReadDirSync(this.publicDir).map((p) => p.replace(/\\/g, '/')) + recursiveReadDirSync(this.publicDir).map((p) => + encodeURI(p.replace(/\\/g, '/')) + ) ) return [ @@ -798,8 +813,7 @@ export default class Server { await this.serveStatic( req, res, - // we need to re-encode it since send decodes it - join(this.publicDir, ...pathParts.map(encodeURIComponent)), + join(this.publicDir, ...pathParts), parsedUrl ) return { @@ -1323,6 +1337,11 @@ export default class Server { } } catch (err) { this.logError(err) + + if (err && err.code === 'DECODE_FAILED') { + res.statusCode = 400 + return await this.renderErrorToHTML(err, req, res, pathname, query) + } res.statusCode = 500 return await this.renderErrorToHTML(err, req, res, pathname, query) } diff --git a/packages/next/server/hot-reloader.ts b/packages/next/server/hot-reloader.ts index abeb3872300c9..0885e0c84fd8f 100644 --- a/packages/next/server/hot-reloader.ts +++ b/packages/next/server/hot-reloader.ts @@ -215,7 +215,22 @@ export default class HotReloader { return {} } - const page = denormalizePagePath(`/${params.path.join('/')}`) + let decodedPagePath: string + + try { + decodedPagePath = `/${params.path + .map((param) => decodeURIComponent(param)) + .join('/')}` + } catch (_) { + const err: Error & { code?: string } = new Error( + 'failed to decode param' + ) + err.code = 'DECODE_FAILED' + throw err + } + + const page = denormalizePagePath(decodedPagePath) + if (page === '/_error' || BLOCKED_PAGES.indexOf(page) === -1) { try { await this.ensurePage(page) diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index f8c0042cb255d..8bddcb130f023 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -336,7 +336,17 @@ export default class DevServer extends Server { const path = `/${pathParts.join('/')}` // check for a public file, throwing error if there's a // conflicting page - if (await this.hasPublicFile(path)) { + let decodedPath: string + + try { + decodedPath = decodeURIComponent(path) + } catch (_) { + const err: Error & { code?: string } = new Error('failed to decode param') + err.code = 'DECODE_FAILED' + throw err + } + + if (await this.hasPublicFile(decodedPath)) { if (await this.hasPage(pathname!)) { const err = new Error( `A conflicting public file and page file was found for path ${pathname} https://err.sh/vercel/next.js/conflicting-public-file-page` @@ -660,7 +670,7 @@ export default class DevServer extends Server { res: ServerResponse, pathParts: string[] ): Promise { - const p = pathJoin(this.publicDir, ...pathParts.map(encodeURIComponent)) + const p = pathJoin(this.publicDir, ...pathParts) return this.serveStatic(req, res, p) } diff --git a/test/integration/production/pages/invalid-param/[slug].js b/test/integration/production/pages/invalid-param/[slug].js new file mode 100644 index 0000000000000..9f287061d65fc --- /dev/null +++ b/test/integration/production/pages/invalid-param/[slug].js @@ -0,0 +1,13 @@ +import { useRouter } from 'next/router' + +export default function Page() { + return

hello {useRouter().query}

+} + +export const getServerSideProps = () => { + return { + props: { + hello: 'world', + }, + } +} diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 467eeb42c114d..5a53721540439 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -710,7 +710,7 @@ describe('Production Usage', () => { }) it('should handle failed param decoding', async () => { - const html = await renderViaHTTP(appPort, '/%DE~%C7%1fY/') + const html = await renderViaHTTP(appPort, '/invalid-param/%DE~%C7%1fY/') expect(html).toMatch(/400/) expect(html).toMatch(/Bad Request/) })