Update status code for normalize error (#36580)

This updates to show a 400 (bad request) when an invalid path is sent to Next.js similar to our decode failure handling. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Closes: #36555 Co-authored-by: Tim Neutkens <[email protected]>
vercel · May 1, 2022 · bd3dfe1 · bd3dfe1
1 parent 7a09f88
commit bd3dfe1
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 14 deletions.
diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
@@ -11,7 +11,12 @@ import type { ParsedUrlQuery } from 'querystring'
 import type { RenderOpts, RenderOptsPartial } from './render'
 import type { ResponseCacheEntry, ResponseCacheValue } from './response-cache'
 import type { UrlWithParsedQuery } from 'url'
-import type { CacheFs } from '../shared/lib/utils'
+import {
+  CacheFs,
+  NormalizeError,
+  DecodeError,
+  normalizeRepeatedSlashes,
+} from '../shared/lib/utils'
 import type { PreviewData } from 'next/types'
 import type { PagesManifest } from '../build/webpack/plugins/pages-manifest-plugin'
 import type { BaseNextRequest, BaseNextResponse } from './base-http'
@@ -39,7 +44,6 @@ import {
   checkIsManualRevalidate,
 } from './api-utils'
 import * as envConfig from '../shared/lib/runtime-config'
-import { DecodeError, normalizeRepeatedSlashes } from '../shared/lib/utils'
 import { isTargetLikeServerless } from './utils'
 import Router from './router'
 import { getPathMatch } from '../shared/lib/router/utils/path-match'
@@ -564,7 +568,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
             ])
           }
         } catch (err) {
-          if (err instanceof DecodeError) {
+          if (err instanceof DecodeError || err instanceof NormalizeError) {
             res.statusCode = 400
             return this.renderError(null, req, res, '/_error', {})
           }
@@ -612,7 +616,8 @@ export default abstract class Server<ServerOptions extends Options = Options> {
     } catch (err: any) {
       if (
         (err && typeof err === 'object' && err.code === 'ERR_INVALID_URL') ||
-        err instanceof DecodeError
+        err instanceof DecodeError ||
+        err instanceof NormalizeError
       ) {
         res.statusCode = 400
         return this.renderError(null, req, res, '/_error', {})
@@ -985,7 +990,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
         return
       }
     } catch (err) {
-      if (err instanceof DecodeError) {
+      if (err instanceof DecodeError || err instanceof NormalizeError) {
         res.statusCode = 400
         return this.renderError(null, req, res, '/_error', {})
       }
@@ -1745,7 +1750,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
       if (err instanceof NoFallbackError && bubbleNoFallback) {
         throw err
       }
-      if (err instanceof DecodeError) {
+      if (err instanceof DecodeError || err instanceof NormalizeError) {
         res.statusCode = 400
         return await this.renderErrorToResponse(ctx, err)
       }

diff --git a/packages/next/shared/lib/page-path/normalize-page-path.ts b/packages/next/shared/lib/page-path/normalize-page-path.ts
@@ -1,6 +1,7 @@
 import { ensureLeadingSlash } from './ensure-leading-slash'
 import { isDynamicRoute } from '../router/utils'
 import { posix } from '../isomorphic/path'
+import { NormalizeError } from '../utils'
 
 /**
  * Takes a page and transforms it into its file counterpart ensuring that the
@@ -22,7 +23,7 @@ export function normalizePagePath(page: string): string {
 
   const resolvedPage = posix.normalize(normalized)
   if (resolvedPage !== normalized) {
-    throw new Error(
+    throw new NormalizeError(
       `Requested and resolved page mismatch: ${normalized} ${resolvedPage}`
     )
   }

diff --git a/packages/next/shared/lib/utils.ts b/packages/next/shared/lib/utils.ts
@@ -397,6 +397,7 @@ export const ST =
   typeof performance.measure === 'function'
 
 export class DecodeError extends Error {}
+export class NormalizeError extends Error {}
 
 export interface CacheFs {
   readFile(f: string): Promise<string>

diff --git a/test/integration/production/test/security.js b/test/integration/production/test/security.js
@@ -5,12 +5,7 @@ import { readFileSync } from 'fs'
 import http from 'http'
 import url from 'url'
 import { join } from 'path'
-import {
-  renderViaHTTP,
-  getBrowserBodyText,
-  waitFor,
-  fetchViaHTTP,
-} from 'next-test-utils'
+import { getBrowserBodyText, waitFor, fetchViaHTTP } from 'next-test-utils'
 import { recursiveReadDir } from 'next/dist/lib/recursive-readdir'
 import { homedir } from 'os'
 
@@ -73,8 +68,10 @@ module.exports = (context) => {
       ]
 
       for (const path of pathsToCheck) {
-        const data = await renderViaHTTP(context.appPort, path)
+        const res = await fetchViaHTTP(context.appPort, path)
+        const data = await res.text()
         expect(data.includes('cool-version')).toBeFalsy()
+        expect([400, 404].includes(res.status)).toBeTruthy()
       }
     })