Merge pull request #1868 from dpatti/smarter-router-auto-options

Automatic OPTIONS response breaks with multiple routers

lib/router/index.js

@@ -104,7 +104,7 @@ Router.prototype._dispatch = function(req, res, next){
     req.route = route = self.matchRequest(req, i);
 
     // implied OPTIONS
-    if (!route && 'OPTIONS' == req.method) return self._options(req, res);
+    if (!route && 'OPTIONS' == req.method) return self._options(req, res, next);
 
     // no route
     if (!route) return next(err);
@@ -181,9 +181,10 @@ Router.prototype._dispatch = function(req, res, next){
  * @api private
  */
 
-Router.prototype._options = function(req, res){
+Router.prototype._options = function(req, res, next){
   var path = parse(req).pathname
     , body = this._optionsFor(path).join(',');
+  if (!body) return next();
   res.set('Allow', body).send(body);
 };
 

test/app.options.js

@@ -15,6 +15,30 @@ describe('OPTIONS', function(){
     .expect('GET,PUT')
     .expect('Allow', 'GET,PUT', done);
   })
+
+  it('should not respond if the path is not defined', function(done){
+    var app = express();
+
+    app.get('/users', function(req, res){});
+
+    request(app)
+    .options('/other')
+    .expect(404, done);
+  })
+
+  it('should forward requests down the middleware chain', function(done){
+    var app = express();
+    var router = new express.Router();
+
+    router.get('/users', function(req, res){});
+    app.use(router.middleware);
+    app.get('/other', function(req, res){});
+
+    request(app)
+    .options('/other')
+    .expect('GET')
+    .expect('Allow', 'GET', done);
+  })
 })
 
 describe('app.options()', function(){