Skip to content

Commit 51f5290

Browse files
committed
Fix case where router.use skipped requests routes did not
fixes #3037
1 parent 8b6dc6c commit 51f5290

File tree

3 files changed

+43
-5
lines changed

3 files changed

+43
-5
lines changed

History.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ unreleased
22
==========
33

44
* Add debug message when loading view engine
5+
* Fix case where `router.use` skipped requests routes did not
56
* Remove usage of `res._headers` private field
67
- Improves compatibility with Node.js 8 nightly
78
* Skip routing when `req.url` is not set

lib/router/index.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,12 +280,13 @@ proto.handle = function handle(req, res, out) {
280280
}
281281

282282
function trim_prefix(layer, layerError, layerPath, path) {
283-
var c = path[layerPath.length];
284-
if (c && '/' !== c && '.' !== c) return next(layerError);
285-
286-
// Trim off the part of the url that matches the route
287-
// middleware (.use stuff) needs to have the path stripped
288283
if (layerPath.length !== 0) {
284+
// Validate path breaks on a path separator
285+
var c = path[layerPath.length]
286+
if (c && c !== '/' && c !== '.') return next(layerError)
287+
288+
// Trim off the part of the url that matches the route
289+
// middleware (.use stuff) needs to have the path stripped
289290
debug('trim prefix (%s) from url %s', layerPath, req.url);
290291
removed = layerPath;
291292
req.url = protohost + req.url.substr(protohost.length + removed.length);

test/Router.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,24 @@ describe('Router', function(){
347347
assert.equal(count, methods.length);
348348
done();
349349
})
350+
351+
it('should be called for any URL when "*"', function (done) {
352+
var cb = after(4, done)
353+
var router = new Router()
354+
355+
function no () {
356+
throw new Error('should not be called')
357+
}
358+
359+
router.all('*', function (req, res) {
360+
res.end()
361+
})
362+
363+
router.handle({ url: '/', method: 'GET' }, { end: cb }, no)
364+
router.handle({ url: '/foo', method: 'GET' }, { end: cb }, no)
365+
router.handle({ url: 'foo', method: 'GET' }, { end: cb }, no)
366+
router.handle({ url: '*', method: 'GET' }, { end: cb }, no)
367+
})
350368
})
351369

352370
describe('.use', function() {
@@ -363,6 +381,24 @@ describe('Router', function(){
363381
router.use.bind(router, '/', new Date()).should.throw(/requires middleware function.*Date/)
364382
})
365383

384+
it('should be called for any URL', function (done) {
385+
var cb = after(4, done)
386+
var router = new Router()
387+
388+
function no () {
389+
throw new Error('should not be called')
390+
}
391+
392+
router.use(function (req, res) {
393+
res.end()
394+
})
395+
396+
router.handle({ url: '/', method: 'GET' }, { end: cb }, no)
397+
router.handle({ url: '/foo', method: 'GET' }, { end: cb }, no)
398+
router.handle({ url: 'foo', method: 'GET' }, { end: cb }, no)
399+
router.handle({ url: '*', method: 'GET' }, { end: cb }, no)
400+
})
401+
366402
it('should accept array of middleware', function(done){
367403
var count = 0;
368404
var router = new Router();

0 commit comments

Comments
 (0)