Skip to content

Commit

Permalink
Fix case where router.use skipped requests routes did not
Browse files Browse the repository at this point in the history
fixes #3037
  • Loading branch information
dougwilson committed Feb 26, 2017
1 parent 8b6dc6c commit 51f5290
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
1 change: 1 addition & 0 deletions History.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions lib/router/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
36 changes: 36 additions & 0 deletions test/Router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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();
Expand Down

0 comments on commit 51f5290

Please sign in to comment.