feat(router): isolate warning to Express routers and support array paths

1. Scope Router.head warning check to expressRouter instances to prevent mutating the global Router.prototype from the standalone router package.
2. Add pathsOverlap utility to properly detect route shadowing when GET is declared with array paths (e.g. ['/a', '/b']).
3. Add regression tests for array route shadowing and prototype isolation.

Author: santusht06

lib/application.js

@@ -21,6 +21,7 @@ var methods = require('./utils').methods;
 var compileETag = require('./utils').compileETag;
 var compileQueryParser = require('./utils').compileQueryParser;
 var compileTrust = require('./utils').compileTrust;
+var pathsOverlap = require('./utils').pathsOverlap;
 var resolve = require('node:path').resolve;
 var once = require('once')
 var Router = require('router');
@@ -477,7 +478,7 @@ methods.forEach(function (method) {
 
     if (method === 'head' && this.router && Array.isArray(this.router.stack)) {
       var hasPrecedingGet = this.router.stack.some(function (layer) {
-        return layer.route && layer.route.path === path && layer.route.methods && layer.route.methods.get;
+        return layer.route && pathsOverlap(layer.route.path, path) && layer.route.methods && layer.route.methods.get;
       });
       if (hasPrecedingGet) {
         process.emitWarning(

lib/express.js

@@ -63,16 +63,24 @@ exports.application = proto;
 exports.request = req;
 exports.response = res;
 
+var utils = require('./utils');
+
 /**
- * Wrap Router.prototype.head to warn if declared after a GET route on the same path.
+ * Express router factory wrapping Router to detect and warn on shadowed HEAD routes.
+ *
+ * @param {Object} [options]
+ * @return {Function} router
+ * @public
  */
 
-if (Router.prototype && typeof Router.prototype.head === 'function') {
-  var origRouterHead = Router.prototype.head;
-  Router.prototype.head = function head(path) {
+function expressRouter(options) {
+  var router = new Router(options);
+  var origHead = router.head;
+
+  router.head = function head(path) {
     if (Array.isArray(this.stack)) {
       var hasPrecedingGet = this.stack.some(function (layer) {
-        return layer.route && layer.route.path === path && layer.route.methods && layer.route.methods.get;
+        return layer.route && utils.pathsOverlap(layer.route.path, path) && layer.route.methods && layer.route.methods.get;
       });
       if (hasPrecedingGet) {
         process.emitWarning(
@@ -81,16 +89,21 @@ if (Router.prototype && typeof Router.prototype.head === 'function') {
         );
       }
     }
-    return origRouterHead.apply(this, arguments);
+    return origHead.apply(this, arguments);
   };
+
+  return router;
 }
 
+expressRouter.Route = Router.Route;
+expressRouter.prototype = Router.prototype;
+
 /**
  * Expose constructors.
  */
 
 exports.Route = Router.Route;
-exports.Router = Router;
+exports.Router = expressRouter;
 
 /**
  * Expose middleware

lib/utils.js

@@ -269,3 +269,27 @@ function parseExtendedQueryString(str) {
     allowPrototypes: true
   });
 }
+
+/**
+ * Check if two route paths overlap (supporting string and array paths).
+ *
+ * @param {String|Array} existingPath
+ * @param {String|Array} newPath
+ * @return {Boolean}
+ * @api private
+ */
+
+exports.pathsOverlap = function pathsOverlap(existingPath, newPath) {
+  if (Array.isArray(existingPath) && Array.isArray(newPath)) {
+    return existingPath.some(function (p) {
+      return newPath.indexOf(p) !== -1;
+    });
+  }
+  if (Array.isArray(existingPath)) {
+    return existingPath.indexOf(newPath) !== -1;
+  }
+  if (Array.isArray(newPath)) {
+    return newPath.indexOf(existingPath) !== -1;
+  }
+  return existingPath === newPath;
+};

test/Router.js

@@ -686,5 +686,54 @@ describe('Router', function () {
         done()
       })
     })
+
+    it('should emit a warning when GET is declared with an array containing the HEAD path', function (done) {
+      var router = new Router()
+      var warnings = []
+
+      function onWarning(warning) {
+        warnings.push(warning)
+      }
+
+      process.on('warning', onWarning)
+
+      router.get(['/a', '/b'], function (req, res) {
+        res.end('get')
+      })
+
+      router.head('/a', function (req, res) {
+        res.end()
+      })
+
+      process.nextTick(function () {
+        process.removeListener('warning', onWarning)
+        assert.strictEqual(warnings.length, 1)
+        assert.strictEqual(warnings[0].name, 'ExpressWarning')
+        assert.ok(warnings[0].message.includes('HEAD route for "/a" declared after GET route will be shadowed'))
+        done()
+      })
+    })
+
+    it('should not mutate the underlying router package prototype', function () {
+      var StandaloneRouter = require('router')
+      var standalone = new StandaloneRouter()
+
+      var warnings = []
+      function onWarning(warning) {
+        warnings.push(warning)
+      }
+
+      process.on('warning', onWarning)
+
+      standalone.get('/standalone', function (req, res) {
+        res.end()
+      })
+      standalone.head('/standalone', function (req, res) {
+        res.end()
+      })
+
+      process.removeListener('warning', onWarning)
+      assert.strictEqual(warnings.length, 0)
+    })
   })
 })

test/app.head.js

@@ -140,4 +140,31 @@ describe('app.head()', function(){
       done()
     })
   })
+
+  it('should emit a warning when GET is declared with an array containing the HEAD path', function (done) {
+    var app = express()
+    var warnings = []
+
+    function onWarning(warning) {
+      warnings.push(warning)
+    }
+
+    process.on('warning', onWarning)
+
+    app.get(['/a', '/b'], function (req, res) {
+      res.send('get')
+    })
+
+    app.head('/a', function (req, res) {
+      res.end()
+    })
+
+    process.nextTick(function () {
+      process.removeListener('warning', onWarning)
+      assert.strictEqual(warnings.length, 1)
+      assert.strictEqual(warnings[0].name, 'ExpressWarning')
+      assert.ok(warnings[0].message.includes('HEAD route for "/a" declared after GET route will be shadowed'))
+      done()
+    })
+  })
 })