fix: contain handler errors after headers sent + harden nested-router state

README.md

@@ -47,5 +47,24 @@ server.listen(3000)
 You can support the maintenance of this project: 
 - PayPal: https://www.paypal.me/kyberneees
 
+# Security notes
+
+- **Errors never crash the process.** A route handler that throws (or rejects)
+  after response headers are already sent is contained: the response is ended or
+  the socket closed, and the server keeps serving. A custom `errorHandler` that
+  itself throws or rejects is also contained with a bare `500`.
+- **Route matching is case-insensitive.** `/Admin`, `/admin` and `/ADMIN` all
+  match a route registered as `/admin`. Reverse proxies, WAFs and auth layers
+  that classify paths case-sensitively must be aligned accordingly.
+- **Mount nested routers with `use()`.** Only `router.use(prefix, subRouter)`
+  rewrites the URL when entering a nested router. A router passed to `get()` or
+  `on()` is treated as a plain middleware and will not match its sub-routes.
+- **Nested routers own unmatched requests.** If a mounted router finds no route
+  for the rewritten URL, it responds `404` itself and the parent chain is not
+  resumed (unlike Express mounted apps, which fall through).
+- **The route cache is memory-bounded.** Matches with parameters, `404`s, and
+  paths longer than 4KB are never cached, and the cache is additionally capped
+  by total size — so attacker-controlled unique paths cannot pin memory.
+
 # More
 - Website and documentation: https://0http.21no.de

lib/next.js

@@ -24,6 +24,41 @@ function restoreNestedUrl (req) {
   }
 }
 
+/**
+ * Contain every error-handler invocation so a routing error can never take the
+ * process down. A handler error after headers are sent makes res.setHeader
+ * throw ERR_HTTP_HEADERS_SENT; a user-supplied errorHandler may throw (sync)
+ * or reject (async) for any reason. All of these previously escaped as
+ * uncaughtException / unhandledRejection and killed the server.
+ */
+function failSafe (res) {
+  if (res.headersSent) {
+    // Response already started: a fresh status is impossible. End it as-is,
+    // falling back to destroying the socket when the stream already finished.
+    try { res.end() } catch (_) { res.destroy() }
+    return undefined
+  }
+  // Last-resort bare 500 for a broken error handler.
+  res.statusCode = 500
+  try { res.setHeader('Content-Type', 'text/plain') } catch (_) {}
+  try { res.end('Internal Server Error') } catch (_) {}
+  return undefined
+}
+
+function dispatchError (errorHandler, err, req, res) {
+  let result
+  try {
+    result = errorHandler(err, req, res)
+  } catch (_) {
+    return failSafe(res)
+  }
+  // An async error handler can reject after returning: contain that too.
+  // Promise.resolve() normalises arbitrary thenables (which may lack .catch).
+  return result && typeof result.then === 'function'
+    ? Promise.resolve(result).catch(() => failSafe(res))
+    : result
+}
+
 function next (middlewares, req, res, index, routers, defaultRoute, errorHandler) {
   // Fast path for end of middleware chain
   if (index >= middlewares.length) {
@@ -37,7 +72,7 @@ function next (middlewares, req, res, index, routers, defaultRoute, errorHandler
   // Create step function - this is called by middleware to continue the chain
   const step = function (err) {
     return err
-      ? errorHandler(err, req, res)
+      ? dispatchError(errorHandler, err, req, res)
       : next(middlewares, req, res, index + 1, routers, defaultRoute, errorHandler)
   }
   // Expose the error handler so nested routers can bubble errors to the parent
@@ -74,25 +109,26 @@ function next (middlewares, req, res, index, routers, defaultRoute, errorHandler
         return result && typeof result.then === 'function'
           ? result.catch(err => {
             restoreNestedUrl(req)
-            return errorHandler(err, req, res)
+            return dispatchError(errorHandler, err, req, res)
           })
           : result
       } catch (err) {
         // Sync error that escaped the nested router's own handling.
         // Restore the parent URL context before invoking the error handler.
         restoreNestedUrl(req)
-        return errorHandler(err, req, res)
+        return dispatchError(errorHandler, err, req, res)
       }
     }
 
     // Regular middleware function
     const result = middleware(req, res, step)
     return result && typeof result.then === 'function'
-      ? result.catch(err => errorHandler(err, req, res))
+      ? result.catch(err => dispatchError(errorHandler, err, req, res))
       : result
   } catch (err) {
-    return errorHandler(err, req, res)
+    return dispatchError(errorHandler, err, req, res)
   }
 }
 
 module.exports = next
+module.exports.dispatchError = dispatchError

lib/router/sequential.js

@@ -3,6 +3,7 @@ const next = require('./../next')
 const { parse } = require('regexparam')
 const { LRUCache: Cache } = require('lru-cache')
 const queryparams = require('./../utils/queryparams')
+const dispatchError = require('./../next').dispatchError
 
 /**
  * Default handlers as constants to avoid creating functions on each router instance.
@@ -14,15 +15,18 @@ const DEFAULT_ROUTE = (req, res) => {
 }
 
 const DEFAULT_ERROR_HANDLER = (err, req, res) => {
-  res.statusCode = 500
-  res.setHeader('Content-Type', 'text/plain')
   // Safe by default: only expose error details in explicit development mode.
   // Production, staging, testing, and unset NODE_ENV all receive sanitized response.
-  if (process.env.NODE_ENV === 'development') {
-    res.end(err.message)
-  } else {
-    res.end('Internal Server Error')
+  if (res.headersSent) {
+    // Headers already flushed (mid-stream failure): a status code can no longer
+    // be applied. End the response as-is, falling back to destroying the socket
+    // when the stream has already finished.
+    try { res.end() } catch (_) { res.destroy() }
+    return
   }
+  res.statusCode = 500
+  res.setHeader('Content-Type', 'text/plain')
+  res.end(process.env.NODE_ENV === 'development' ? err.message : 'Internal Server Error')
 }
 
 /**
@@ -58,13 +62,21 @@ module.exports = (config = {}) => {
   if (cacheSize > 0) {
     cache = new Cache({
       max: cacheSize,
+      maxSize: Math.max(cacheSize * 1024, 1 << 20), // hard byte cap (≥1MB)
+      maxEntrySize: 4096, // never cache absurdly long paths
+      sizeCalculation: (value, key) => key.length + 64,
       updateAgeOnGet: false, // Disable age updates for better performance
       updateAgeOnHas: false
     })
   } else if (cacheSize < 0) {
-    // Reduced from 100k to 50k for better memory efficiency while maintaining performance
+    // Reduced from 100k to 50k for better memory efficiency while maintaining performance.
+    // Byte-bounded so attacker-controlled long paths cannot pin memory even when
+    // they match global middleware or catch-all regex routes (empty params).
     cache = new Cache({
       max: 50000,
+      maxSize: 16 * 1024 * 1024, // 16MB hard cap
+      maxEntrySize: 4096,
+      sizeCalculation: (value, key) => key.length + 64,
       updateAgeOnGet: false,
       updateAgeOnHas: false
     })
@@ -132,16 +144,27 @@ module.exports = (config = {}) => {
    * Uses property deletion instead of undefined assignment for better performance.
    * Optimized to minimize closure creation overhead.
    */
-  const createCleanupMiddleware = (step) => {
-    // Pre-create the cleanup function to avoid repeated function creation
-    return (req, res, next) => {
-      req.url = req.preRouterUrl
-      req.path = req.preRouterPath
-
-      // Use delete for better performance than setting undefined
+  // Restore the request state a nested lookup changed, using values snapshotted
+  // BEFORE the lookup ran. Closure snapshots give correct stack semantics: a
+  // grandchild cannot corrupt a parent's restore (preRouterUrl is a single slot,
+  // so re-reading req after deeper levels run yields undefined).
+  const restoreNestedUrlContext = (req, context) => {
+    if (context.hasUrlContext) {
+      req.url = context.url
+      req.path = context.path
       delete req.preRouterUrl
       delete req.preRouterPath
+    }
+  }
 
+  const restoreNestedContext = (req, context) => {
+    restoreNestedUrlContext(req, context)
+    req.params = context.params
+  }
+
+  const createCleanupMiddleware = (step, context) => {
+    return (req, res, next) => {
+      restoreNestedContext(req, context)
       return step()
     }
   }
@@ -166,7 +189,14 @@ module.exports = (config = {}) => {
 
       if (!match) {
         match = router.find(req.method, req.path)
-        cache.set(reqCacheKey, match)
+        // Parametrized matches have unbounded distinct keys (one per param
+        // value): caching them pins attacker-growable memory and churns hot
+        // static entries out of the LRU. Never cache them, and never cache
+        // 404s either (empty handlers) — junk unmatched paths are the same
+        // memory-pressure vector.
+        if (match.handlers.length && Object.keys(match.params).length === 0) {
+          cache.set(reqCacheKey, match)
+        }
       }
     } else {
       match = router.find(req.method, req.path)
@@ -175,12 +205,21 @@ module.exports = (config = {}) => {
     const { handlers, params } = match
 
     if (handlers.length) {
-      // Optimized middleware array handling
+      // Snapshot the request state this lookup is about to change (URL rewrite +
+      // params). Cleanup and error paths restore from the snapshot — closure
+      // semantics, so deeper nesting levels cannot corrupt an outer restore.
+      const context = {
+        hasUrlContext: req.preRouterUrl !== undefined,
+        url: req.preRouterUrl,
+        path: req.preRouterPath,
+        params: req.params
+      }
+
       let middlewares
       if (step !== undefined) {
         // Create new array only when step middleware is needed
         middlewares = handlers.slice()
-        middlewares.push(createCleanupMiddleware(step))
+        middlewares.push(createCleanupMiddleware(step, context))
       } else {
         middlewares = handlers
       }
@@ -191,29 +230,25 @@ module.exports = (config = {}) => {
       // nested router's own default error handler.
       const activeErrorHandler = step?.errorHandler || errorHandler
 
-      // Wrap the active error handler so URL restoration happens before the
-      // handler is invoked. This fixes state corruption when a nested router
-      // handler throws, calls next(err), or rejects asynchronously.
+      // Wrap the active error handler so request-state restoration happens
+      // before the handler is invoked, and so a throwing handler can never
+      // take the process down.
       const errorHandlerWithCleanup = (err, req, res) => {
-        if (req.preRouterUrl !== undefined) {
-          req.url = req.preRouterUrl
-          req.path = req.preRouterPath
-          delete req.preRouterUrl
-          delete req.preRouterPath
-        }
-        return activeErrorHandler(err, req, res)
+        // Restore URL context only: error handlers are terminal, so keep the
+        // matched route's params available to the handler.
+        restoreNestedUrlContext(req, context)
+        return dispatchError(activeErrorHandler, err, req, res)
       }
 
-      // Optimized parameter assignment with minimal overhead
-      if (!req.params) {
-        // Shallow-copy: the match (and its params) may be served from the LRU
-        // cache and shared across all requests to the same method+path.
-        // Assigning by reference would let a middleware mutation leak between requests.
-        req.params = params ? { ...params } : Object.create(null)
-      } else if (params) {
-        // Manual property copying - optimized for small objects
-        // Pre-compute keys and length to avoid repeated calls
-        Object.assign(req.params, params)
+      // Per-level params: never mutate an upstream params object in place.
+      // Nested routers inherit parent params via a shallow copy, so existing
+      // consumers keep seeing merged params while mutations stay confined to
+      // this level's own object (restored on cleanup/error).
+      const inherited = req.params
+      if (inherited === undefined && Object.keys(params).length === 0) {
+        req.params = Object.create(null)
+      } else {
+        req.params = { ...(inherited || {}), ...(params || {}) }
       }
 
       return next(middlewares, req, res, 0, routers, defaultRoute, errorHandlerWithCleanup)