Match routes iteratively to prevent stack overflows

fixes #2412

History.md

@@ -4,6 +4,7 @@ unreleased
   * Deprecate leading `:` in `name` for `app.param(name, fn)`
   * Fix `OPTIONS` responses to include the `HEAD` method properly
   * Fix `res.sendFile` not always detecting aborted connection
+  * Match routes iteratively to prevent stack overflows
 
 4.10.8 / 2015-01-13
 ===================

lib/router/index.js

@@ -169,74 +169,104 @@ proto.handle = function(req, res, done) {
       ? null
       : err;
 
-    var layer = stack[idx++];
-
+    // remove added slash
     if (slashAdded) {
       req.url = req.url.substr(1);
       slashAdded = false;
     }
 
+    // restore altered req.url
     if (removed.length !== 0) {
       req.baseUrl = parentUrl;
       req.url = protohost + removed + req.url.substr(protohost.length);
       removed = '';
     }
 
-    if (!layer) {
+    // no more matching layers
+    if (idx >= stack.length) {
       setImmediate(done, layerError);
       return;
     }
 
-    self.match_layer(layer, req, res, function (err, path) {
-      if (err || path === undefined) {
-        return next(layerError || err);
+    // get pathname of request
+    var path = getPathname(req);
+
+    if (path == null) {
+      return done(layerError);
+    }
+
+    // find next matching layer
+    var layer;
+    var match;
+    var route;
+
+    while (match !== true && idx < stack.length) {
+      layer = stack[idx++];
+      match = matchLayer(layer, path);
+      route = layer.route;
+
+      if (typeof match !== 'boolean') {
+        // hold on to layerError
+        layerError = layerError || match;
       }
 
-      // route object and not middleware
-      var route = layer.route;
+      if (match !== true) {
+        continue;
+      }
 
-      // if final route, then we support options
-      if (route) {
-        // we don't run any routes with error first
-        if (layerError) {
-          return next(layerError);
-        }
-
-        var method = req.method;
-        var has_method = route._handles_method(method);
-
-        // build up automatic options response
-        if (!has_method && method === 'OPTIONS') {
-          appendMethods(options, route._options());
-        }
-
-        // don't even bother
-        if (!has_method && method !== 'HEAD') {
-          return next();
-        }
-
-        // we can now dispatch to the route
-        req.route = route;
+      if (!route) {
+        // process non-route handlers normally
+        continue;
       }
 
-      // Capture one-time layer values
-      req.params = self.mergeParams
-        ? mergeParams(layer.params, parentParams)
-        : layer.params;
-      var layerPath = layer.path;
+      if (layerError) {
+        // routes do not match with a pending error
+        match = false;
+        continue;
+      }
 
-      // this should be done for the layer
-      self.process_params(layer, paramcalled, req, res, function (err) {
-        if (err) {
-          return next(layerError || err);
-        }
+      var method = req.method;
+      var has_method = route._handles_method(method);
 
-        if (route) {
-          return layer.handle_request(req, res, next);
-        }
+      // build up automatic options response
+      if (!has_method && method === 'OPTIONS') {
+        appendMethods(options, route._options());
+      }
+
+      // don't even bother matching route
+      if (!has_method && method !== 'HEAD') {
+        match = false;
+        continue;
+      }
+    }
 
-        trim_prefix(layer, layerError, layerPath, path);
-      });
+    // no match
+    if (match !== true) {
+      return done(layerError);
+    }
+
+    // store route for dispatch on change
+    if (route) {
+      req.route = route;
+    }
+
+    // Capture one-time layer values
+    req.params = self.mergeParams
+      ? mergeParams(layer.params, parentParams)
+      : layer.params;
+    var layerPath = layer.path;
+
+    // this should be done for the layer
+    self.process_params(layer, paramcalled, req, res, function (err) {
+      if (err) {
+        return next(layerError || err);
+      }
+
+      if (route) {
+        return layer.handle_request(req, res, next);
+      }
+
+      trim_prefix(layer, layerError, layerPath, path);
     });
   }
 
@@ -273,29 +303,6 @@ proto.handle = function(req, res, done) {
   }
 };
 
-/**
- * Match request to a layer.
- *
- * @api private
- */
-
-proto.match_layer = function match_layer(layer, req, res, done) {
-  var error = null;
-  var path;
-
-  try {
-    path = parseUrl(req).pathname;
-
-    if (!layer.match(path)) {
-      path = undefined;
-    }
-  } catch (err) {
-    error = err;
-  }
-
-  done(error, path);
-};
-
 /**
  * Process any parameters for the layer.
  *
@@ -502,6 +509,15 @@ function appendMethods(list, addition) {
   }
 }
 
+// get pathname of request
+function getPathname(req) {
+  try {
+    return parseUrl(req).pathname;
+  } catch (err) {
+    return undefined;
+  }
+}
+
 // get type for error message
 function gettype(obj) {
   var type = typeof obj;
@@ -515,6 +531,22 @@ function gettype(obj) {
     .replace(objectRegExp, '$1');
 }
 
+/**
+ * Match path to a layer.
+ *
+ * @param {Layer} layer
+ * @param {string} path
+ * @private
+ */
+
+function matchLayer(layer, path) {
+  try {
+    return layer.match(path);
+  } catch (err) {
+    return err;
+  }
+}
+
 // merge params with parent params
 function mergeParams(params, parent) {
   if (typeof parent !== 'object' || !parent) {

test/Router.js

@@ -53,6 +53,21 @@ describe('Router', function(){
     router.handle({ url: '', method: 'GET' }, {}, done);
   });
 
+  it('should not stack overflow with many registered routes', function(done){
+    var handler = function(req, res){ res.end(new Error('wrong handler')) };
+    var router = new Router();
+
+    for (var i = 0; i < 6000; i++) {
+      router.get('/thing' + i, handler)
+    }
+
+    router.get('/', function (req, res) {
+      res.end();
+    });
+
+    router.handle({ url: '/', method: 'GET' }, { end: done });
+  });
+
   describe('.handle', function(){
     it('should dispatch', function(done){
       var router = new Router();