Skip to content

Commit 6d98b4f

Browse files
ijjkijjk
authored
Ensure invalid URLs respond with 400 correctly (#32092)
This ensures we catch any errors in `handleRequest` so that we can respond with a 400 for invalid requests. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Fixes: https://github.com/vercel/next.js/issues/32075 Closes: #32080
1 parent 1c199a5 commit 6d98b4f

File tree

3 files changed

+198
-163
lines changed

3 files changed

+198
-163
lines changed

‎packages/next/build/swc/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { platform, arch } from 'os'
22
import { platformArchTriples } from '@napi-rs/triples'
3-
import Log from '../output/log'
3+
import * as Log from '../output/log'
44

55
const ArchName = arch()
66
const PlatformName = platform()

‎packages/next/server/next-server.ts

+170-162
Original file line numberDiff line numberDiff line change
@@ -359,206 +359,214 @@ export default class Server {
359359
res: ServerResponse,
360360
parsedUrl?: NextUrlWithParsedQuery
361361
): Promise<void> {
362-
const urlParts = (req.url || '').split('?')
363-
const urlNoQuery = urlParts[0]
364-
365-
if (urlNoQuery?.match(/(\\|\/\/)/)) {
366-
const cleanUrl = normalizeRepeatedSlashes(req.url!)
367-
res.setHeader('Location', cleanUrl)
368-
res.setHeader('Refresh', `0;url=${cleanUrl}`)
369-
res.statusCode = 308
370-
res.end(cleanUrl)
371-
return
372-
}
362+
try {
363+
const urlParts = (req.url || '').split('?')
364+
const urlNoQuery = urlParts[0]
365+
366+
if (urlNoQuery?.match(/(\\|\/\/)/)) {
367+
const cleanUrl = normalizeRepeatedSlashes(req.url!)
368+
res.setHeader('Location', cleanUrl)
369+
res.setHeader('Refresh', `0;url=${cleanUrl}`)
370+
res.statusCode = 308
371+
res.end(cleanUrl)
372+
return
373+
}
373374

374-
setLazyProp({ req: req as any }, 'cookies', getCookieParser(req.headers))
375+
setLazyProp({ req: req as any }, 'cookies', getCookieParser(req.headers))
375376

376-
// Parse url if parsedUrl not provided
377-
if (!parsedUrl || typeof parsedUrl !== 'object') {
378-
parsedUrl = parseUrl(req.url!, true)
379-
}
377+
// Parse url if parsedUrl not provided
378+
if (!parsedUrl || typeof parsedUrl !== 'object') {
379+
parsedUrl = parseUrl(req.url!, true)
380+
}
380381

381-
// Parse the querystring ourselves if the user doesn't handle querystring parsing
382-
if (typeof parsedUrl.query === 'string') {
383-
parsedUrl.query = parseQs(parsedUrl.query)
384-
}
382+
// Parse the querystring ourselves if the user doesn't handle querystring parsing
383+
if (typeof parsedUrl.query === 'string') {
384+
parsedUrl.query = parseQs(parsedUrl.query)
385+
}
385386

386-
// When there are hostname and port we build an absolute URL
387-
const initUrl =
388-
this.hostname && this.port
389-
? `http://${this.hostname}:${this.port}${req.url}`
390-
: req.url
387+
// When there are hostname and port we build an absolute URL
388+
const initUrl =
389+
this.hostname && this.port
390+
? `http://${this.hostname}:${this.port}${req.url}`
391+
: req.url
391392

392-
addRequestMeta(req, '__NEXT_INIT_URL', initUrl)
393-
addRequestMeta(req, '__NEXT_INIT_QUERY', { ...parsedUrl.query })
393+
addRequestMeta(req, '__NEXT_INIT_URL', initUrl)
394+
addRequestMeta(req, '__NEXT_INIT_QUERY', { ...parsedUrl.query })
394395

395-
const url = parseNextUrl({
396-
headers: req.headers,
397-
nextConfig: this.nextConfig,
398-
url: req.url?.replace(/^\/+/, '/'),
399-
})
396+
const url = parseNextUrl({
397+
headers: req.headers,
398+
nextConfig: this.nextConfig,
399+
url: req.url?.replace(/^\/+/, '/'),
400+
})
400401

401-
if (url.basePath) {
402-
req.url = replaceBasePath(req.url!, this.nextConfig.basePath)
403-
addRequestMeta(req, '_nextHadBasePath', true)
404-
}
402+
if (url.basePath) {
403+
req.url = replaceBasePath(req.url!, this.nextConfig.basePath)
404+
addRequestMeta(req, '_nextHadBasePath', true)
405+
}
405406

406-
if (
407-
this.minimalMode &&
408-
req.headers['x-matched-path'] &&
409-
typeof req.headers['x-matched-path'] === 'string'
410-
) {
411-
const reqUrlIsDataUrl = req.url?.includes('/_next/data')
412-
const matchedPathIsDataUrl =
413-
req.headers['x-matched-path']?.includes('/_next/data')
414-
const isDataUrl = reqUrlIsDataUrl || matchedPathIsDataUrl
415-
416-
let parsedPath = parseUrl(
417-
isDataUrl ? req.url! : (req.headers['x-matched-path'] as string),
418-
true
419-
)
407+
if (
408+
this.minimalMode &&
409+
req.headers['x-matched-path'] &&
410+
typeof req.headers['x-matched-path'] === 'string'
411+
) {
412+
const reqUrlIsDataUrl = req.url?.includes('/_next/data')
413+
const matchedPathIsDataUrl =
414+
req.headers['x-matched-path']?.includes('/_next/data')
415+
const isDataUrl = reqUrlIsDataUrl || matchedPathIsDataUrl
416+
417+
let parsedPath = parseUrl(
418+
isDataUrl ? req.url! : (req.headers['x-matched-path'] as string),
419+
true
420+
)
420421

421-
let matchedPathname = parsedPath.pathname!
422+
let matchedPathname = parsedPath.pathname!
422423

423-
let matchedPathnameNoExt = isDataUrl
424-
? matchedPathname.replace(/\.json$/, '')
425-
: matchedPathname
424+
let matchedPathnameNoExt = isDataUrl
425+
? matchedPathname.replace(/\.json$/, '')
426+
: matchedPathname
426427

427-
if (this.nextConfig.i18n) {
428-
const localePathResult = normalizeLocalePath(
429-
matchedPathname || '/',
430-
this.nextConfig.i18n.locales
431-
)
428+
if (this.nextConfig.i18n) {
429+
const localePathResult = normalizeLocalePath(
430+
matchedPathname || '/',
431+
this.nextConfig.i18n.locales
432+
)
432433

433-
if (localePathResult.detectedLocale) {
434-
parsedUrl.query.__nextLocale = localePathResult.detectedLocale
434+
if (localePathResult.detectedLocale) {
435+
parsedUrl.query.__nextLocale = localePathResult.detectedLocale
436+
}
435437
}
436-
}
437-
438-
if (isDataUrl) {
439-
matchedPathname = denormalizePagePath(matchedPathname)
440-
matchedPathnameNoExt = denormalizePagePath(matchedPathnameNoExt)
441-
}
442438

443-
const pageIsDynamic = isDynamicRoute(matchedPathnameNoExt)
444-
const combinedRewrites: Rewrite[] = []
439+
if (isDataUrl) {
440+
matchedPathname = denormalizePagePath(matchedPathname)
441+
matchedPathnameNoExt = denormalizePagePath(matchedPathnameNoExt)
442+
}
445443

446-
combinedRewrites.push(...this.customRoutes.rewrites.beforeFiles)
447-
combinedRewrites.push(...this.customRoutes.rewrites.afterFiles)
448-
combinedRewrites.push(...this.customRoutes.rewrites.fallback)
444+
const pageIsDynamic = isDynamicRoute(matchedPathnameNoExt)
445+
const combinedRewrites: Rewrite[] = []
449446

450-
const utils = getUtils({
451-
pageIsDynamic,
452-
page: matchedPathnameNoExt,
453-
i18n: this.nextConfig.i18n,
454-
basePath: this.nextConfig.basePath,
455-
rewrites: combinedRewrites,
456-
})
447+
combinedRewrites.push(...this.customRoutes.rewrites.beforeFiles)
448+
combinedRewrites.push(...this.customRoutes.rewrites.afterFiles)
449+
combinedRewrites.push(...this.customRoutes.rewrites.fallback)
457450

458-
try {
459-
// ensure parsedUrl.pathname includes URL before processing
460-
// rewrites or they won't match correctly
461-
if (this.nextConfig.i18n && !url.locale?.path.detectedLocale) {
462-
parsedUrl.pathname = `/${url.locale?.locale}${parsedUrl.pathname}`
463-
}
464-
utils.handleRewrites(req, parsedUrl)
451+
const utils = getUtils({
452+
pageIsDynamic,
453+
page: matchedPathnameNoExt,
454+
i18n: this.nextConfig.i18n,
455+
basePath: this.nextConfig.basePath,
456+
rewrites: combinedRewrites,
457+
})
465458

466-
// interpolate dynamic params and normalize URL if needed
467-
if (pageIsDynamic) {
468-
let params: ParsedUrlQuery | false = {}
459+
try {
460+
// ensure parsedUrl.pathname includes URL before processing
461+
// rewrites or they won't match correctly
462+
if (this.nextConfig.i18n && !url.locale?.path.detectedLocale) {
463+
parsedUrl.pathname = `/${url.locale?.locale}${parsedUrl.pathname}`
464+
}
465+
utils.handleRewrites(req, parsedUrl)
469466

470-
Object.assign(parsedUrl.query, parsedPath.query)
471-
const paramsResult = utils.normalizeDynamicRouteParams(
472-
parsedUrl.query
473-
)
467+
// interpolate dynamic params and normalize URL if needed
468+
if (pageIsDynamic) {
469+
let params: ParsedUrlQuery | false = {}
474470

475-
if (paramsResult.hasValidParams) {
476-
params = paramsResult.params
477-
} else if (req.headers['x-now-route-matches']) {
478-
const opts: Record<string, string> = {}
479-
params = utils.getParamsFromRouteMatches(
480-
req,
481-
opts,
482-
parsedUrl.query.__nextLocale || ''
471+
Object.assign(parsedUrl.query, parsedPath.query)
472+
const paramsResult = utils.normalizeDynamicRouteParams(
473+
parsedUrl.query
483474
)
484475

485-
if (opts.locale) {
486-
parsedUrl.query.__nextLocale = opts.locale
476+
if (paramsResult.hasValidParams) {
477+
params = paramsResult.params
478+
} else if (req.headers['x-now-route-matches']) {
479+
const opts: Record<string, string> = {}
480+
params = utils.getParamsFromRouteMatches(
481+
req,
482+
opts,
483+
parsedUrl.query.__nextLocale || ''
484+
)
485+
486+
if (opts.locale) {
487+
parsedUrl.query.__nextLocale = opts.locale
488+
}
489+
} else {
490+
params = utils.dynamicRouteMatcher!(matchedPathnameNoExt)
491+
}
492+
493+
if (params) {
494+
params = utils.normalizeDynamicRouteParams(params).params
495+
496+
matchedPathname = utils.interpolateDynamicPath(
497+
matchedPathname,
498+
params
499+
)
500+
req.url = utils.interpolateDynamicPath(req.url!, params)
487501
}
488-
} else {
489-
params = utils.dynamicRouteMatcher!(matchedPathnameNoExt)
490-
}
491502

492-
if (params) {
493-
params = utils.normalizeDynamicRouteParams(params).params
503+
if (reqUrlIsDataUrl && matchedPathIsDataUrl) {
504+
req.url = formatUrl({
505+
...parsedPath,
506+
pathname: matchedPathname,
507+
})
508+
}
494509

495-
matchedPathname = utils.interpolateDynamicPath(
496-
matchedPathname,
497-
params
498-
)
499-
req.url = utils.interpolateDynamicPath(req.url!, params)
510+
Object.assign(parsedUrl.query, params)
511+
utils.normalizeVercelUrl(req, true)
500512
}
501-
502-
if (reqUrlIsDataUrl && matchedPathIsDataUrl) {
503-
req.url = formatUrl({
504-
...parsedPath,
505-
pathname: matchedPathname,
506-
})
513+
} catch (err) {
514+
if (err instanceof DecodeError) {
515+
res.statusCode = 400
516+
return this.renderError(null, req, res, '/_error', {})
507517
}
508-
509-
Object.assign(parsedUrl.query, params)
510-
utils.normalizeVercelUrl(req, true)
511-
}
512-
} catch (err) {
513-
if (err instanceof DecodeError) {
514-
res.statusCode = 400
515-
return this.renderError(null, req, res, '/_error', {})
518+
throw err
516519
}
517-
throw err
518-
}
519520

520-
parsedUrl.pathname = `${this.nextConfig.basePath || ''}${
521-
matchedPathname === '/' && this.nextConfig.basePath
522-
? ''
523-
: matchedPathname
524-
}`
525-
url.pathname = parsedUrl.pathname
526-
}
521+
parsedUrl.pathname = `${this.nextConfig.basePath || ''}${
522+
matchedPathname === '/' && this.nextConfig.basePath
523+
? ''
524+
: matchedPathname
525+
}`
526+
url.pathname = parsedUrl.pathname
527+
}
527528

528-
addRequestMeta(req, '__nextHadTrailingSlash', url.locale?.trailingSlash)
529-
if (url.locale?.domain) {
530-
addRequestMeta(req, '__nextIsLocaleDomain', true)
531-
}
529+
addRequestMeta(req, '__nextHadTrailingSlash', url.locale?.trailingSlash)
530+
if (url.locale?.domain) {
531+
addRequestMeta(req, '__nextIsLocaleDomain', true)
532+
}
532533

533-
if (url.locale?.path.detectedLocale) {
534-
req.url = formatUrl(url)
535-
addRequestMeta(req, '__nextStrippedLocale', true)
536-
if (url.pathname === '/api' || url.pathname.startsWith('/api/')) {
537-
return this.render404(req, res, parsedUrl)
534+
if (url.locale?.path.detectedLocale) {
535+
req.url = formatUrl(url)
536+
addRequestMeta(req, '__nextStrippedLocale', true)
537+
if (url.pathname === '/api' || url.pathname.startsWith('/api/')) {
538+
return this.render404(req, res, parsedUrl)
539+
}
538540
}
539-
}
540541

541-
if (!this.minimalMode || !parsedUrl.query.__nextLocale) {
542-
if (url?.locale?.locale) {
543-
parsedUrl.query.__nextLocale = url.locale.locale
542+
if (!this.minimalMode || !parsedUrl.query.__nextLocale) {
543+
if (url?.locale?.locale) {
544+
parsedUrl.query.__nextLocale = url.locale.locale
545+
}
544546
}
545-
}
546547

547-
if (url?.locale?.defaultLocale) {
548-
parsedUrl.query.__nextDefaultLocale = url.locale.defaultLocale
549-
}
548+
if (url?.locale?.defaultLocale) {
549+
parsedUrl.query.__nextDefaultLocale = url.locale.defaultLocale
550+
}
550551

551-
if (url.locale?.redirect) {
552-
res.setHeader('Location', url.locale.redirect)
553-
res.statusCode = TEMPORARY_REDIRECT_STATUS
554-
res.end()
555-
return
556-
}
552+
if (url.locale?.redirect) {
553+
res.setHeader('Location', url.locale.redirect)
554+
res.statusCode = TEMPORARY_REDIRECT_STATUS
555+
res.end()
556+
return
557+
}
557558

558-
res.statusCode = 200
559-
try {
559+
res.statusCode = 200
560560
return await this.run(req, res, parsedUrl)
561-
} catch (err) {
561+
} catch (err: any) {
562+
if (
563+
(err && typeof err === 'object' && err.code === 'ERR_INVALID_URL') ||
564+
err instanceof DecodeError
565+
) {
566+
res.statusCode = 400
567+
return this.renderError(null, req, res, '/_error', {})
568+
}
569+
562570
if (this.minimalMode || this.renderOpts.dev) {
563571
throw err
564572
}

0 commit comments

Comments
 (0)