Handle decoding errors correctly (#5589)

Fixes #4887
Fixes #3612

Also removes http-errors dependency from next-server, leaving a smaller install size

packages/next-server/package.json

@@ -32,7 +32,6 @@
     "fresh": "0.5.2",
     "hoist-non-react-statics": "^3.0.1",
     "htmlescape": "1.1.1",
-    "http-errors": "1.6.2",
     "path-to-regexp": "2.1.0",
     "prop-types": "15.6.2",
     "send": "0.16.1",

packages/next-server/server/lib/path-match.js

@@ -3,7 +3,6 @@
 // So, it'll give us issues when the app has used a newer version of path-to-regexp
 // (When webpack resolving packages)
 var pathToRegexp = require('path-to-regexp')
-var createError = require('http-errors')
 
 module.exports = function (options) {
   options = options || {}
@@ -36,6 +35,8 @@ function decodeParam (param) {
   try {
     return decodeURIComponent(param)
   } catch (_) {
-    throw createError(400, 'failed to decode param "' + param + '"')
+    const err = new Error('failed to decode param')
+    err.code = 'DECODE_FAILED'
+    throw err
   }
 }

packages/next-server/server/next-server.js

@@ -150,11 +150,19 @@ export default class Server {
   }
 
   async run (req, res, parsedUrl) {
+    try {
       const fn = this.router.match(req, res, parsedUrl)
       if (fn) {
         await fn()
         return
       }
+    } catch (err) {
+      if (err.code === 'DECODE_FAILED') {
+        res.statusCode = 400
+        return this.renderError(null, req, res, '/_error', {})
+      }
+      throw err
+    }
 
     if (req.method === 'GET' || req.method === 'HEAD') {
       await this.render404(req, res, parsedUrl)
@@ -201,6 +209,7 @@ export default class Server {
   }
 
   async renderError (err, req, res, pathname, query) {
+    res.setHeader('Cache-Control', 'no-cache, no-store, max-age=0, must-revalidate')
     const html = await this.renderErrorToHTML(err, req, res, pathname, query)
     return sendHTML(req, res, html, req.method, this.renderOpts)
   }
@@ -212,7 +221,6 @@ export default class Server {
   async render404 (req, res, parsedUrl = parseUrl(req.url, true)) {
     const { pathname, query } = parsedUrl
     res.statusCode = 404
-    res.setHeader('Cache-Control', 'no-cache, no-store, max-age=0, must-revalidate')
     return this.renderError(null, req, res, pathname, query)
   }
 

packages/next/package.json

@@ -64,7 +64,6 @@
     "glob": "7.1.2",
     "hoist-non-react-statics": "2.5.5",
     "htmlescape": "1.1.1",
-    "http-errors": "1.6.2",
     "http-status": "1.0.1",
     "launch-editor": "2.2.1",
     "loader-utils": "1.1.0",

test/integration/production/test/index.test.js

@@ -291,6 +291,12 @@ describe('Production Usage', () => {
     expect(serverSideJsBody).toMatch(/404/)
   })
 
+  it('should handle failed param decoding', async () => {
+    const html = await renderViaHTTP(appPort, '/%DE~%C7%1fY/')
+    expect(html).toMatch(/400/)
+    expect(html).toMatch(/Bad Request/)
+  })
+
   dynamicImportTests(context, (p, q) => renderViaHTTP(context.appPort, p, q))
 
   processEnv(context)