From 51f52901eb1f00571e18d404e10b27310215234c Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Sun, 26 Feb 2017 13:59:47 -0500 Subject: [PATCH] Fix case where router.use skipped requests routes did not fixes #3037 --- History.md | 1 + lib/router/index.js | 11 ++++++----- test/Router.js | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/History.md b/History.md index ed51a7e5b0..bf7764011b 100644 --- a/History.md +++ b/History.md @@ -2,6 +2,7 @@ unreleased ========== * Add debug message when loading view engine + * Fix case where `router.use` skipped requests routes did not * Remove usage of `res._headers` private field - Improves compatibility with Node.js 8 nightly * Skip routing when `req.url` is not set diff --git a/lib/router/index.js b/lib/router/index.js index d2ed86e0bc..917c6002c8 100644 --- a/lib/router/index.js +++ b/lib/router/index.js @@ -280,12 +280,13 @@ proto.handle = function handle(req, res, out) { } function trim_prefix(layer, layerError, layerPath, path) { - var c = path[layerPath.length]; - if (c && '/' !== c && '.' !== c) return next(layerError); - - // Trim off the part of the url that matches the route - // middleware (.use stuff) needs to have the path stripped if (layerPath.length !== 0) { + // Validate path breaks on a path separator + var c = path[layerPath.length] + if (c && c !== '/' && c !== '.') return next(layerError) + + // Trim off the part of the url that matches the route + // middleware (.use stuff) needs to have the path stripped debug('trim prefix (%s) from url %s', layerPath, req.url); removed = layerPath; req.url = protohost + req.url.substr(protohost.length + removed.length); diff --git a/test/Router.js b/test/Router.js index 41cd149e8e..01a6e2c472 100644 --- a/test/Router.js +++ b/test/Router.js @@ -347,6 +347,24 @@ describe('Router', function(){ assert.equal(count, methods.length); done(); }) + + it('should be called for any URL when "*"', function (done) { + var cb = after(4, done) + var router = new Router() + + function no () { + throw new Error('should not be called') + } + + router.all('*', function (req, res) { + res.end() + }) + + router.handle({ url: '/', method: 'GET' }, { end: cb }, no) + router.handle({ url: '/foo', method: 'GET' }, { end: cb }, no) + router.handle({ url: 'foo', method: 'GET' }, { end: cb }, no) + router.handle({ url: '*', method: 'GET' }, { end: cb }, no) + }) }) describe('.use', function() { @@ -363,6 +381,24 @@ describe('Router', function(){ router.use.bind(router, '/', new Date()).should.throw(/requires middleware function.*Date/) }) + it('should be called for any URL', function (done) { + var cb = after(4, done) + var router = new Router() + + function no () { + throw new Error('should not be called') + } + + router.use(function (req, res) { + res.end() + }) + + router.handle({ url: '/', method: 'GET' }, { end: cb }, no) + router.handle({ url: '/foo', method: 'GET' }, { end: cb }, no) + router.handle({ url: 'foo', method: 'GET' }, { end: cb }, no) + router.handle({ url: '*', method: 'GET' }, { end: cb }, no) + }) + it('should accept array of middleware', function(done){ var count = 0; var router = new Router();