Skip to content

Commit bc58db8

Browse files
Radu Dandougwilson
authored andcommitted
Fix issue where next('route') in app.param would incorrectly skip values
fixes #2655
1 parent e9c9f95 commit bc58db8

File tree

3 files changed

+67
-1
lines changed

3 files changed

+67
-1
lines changed

History.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
unreleased
2+
==========
3+
4+
* Fix issue where `next('route')` in `app.param` would incorrectly skip values
5+
16
4.12.4 / 2015-05-17
27
===================
38

lib/router/index.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,8 @@ proto.process_params = function(layer, called, req, res, done) {
357357
}
358358

359359
// param previously called with same value or error occurred
360-
if (paramCalled && (paramCalled.error || paramCalled.match === paramVal)) {
360+
if (paramCalled && (paramCalled.match === paramVal
361+
|| (paramCalled.error && paramCalled.error !== 'route'))) {
361362
// restore value
362363
req.params[name] = paramCalled.value;
363364

test/app.param.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,5 +303,65 @@ describe('app', function(){
303303
.get('/user/new')
304304
.expect('get.new', done);
305305
})
306+
307+
it('should not call when values differ on error', function(done) {
308+
var app = express();
309+
var called = 0;
310+
var count = 0;
311+
312+
app.param('user', function(req, res, next, user) {
313+
called++;
314+
if (user === 'foo') throw new Error('err!');
315+
req.user = user;
316+
next();
317+
});
318+
319+
app.get('/:user/bob', function(req, res, next) {
320+
count++;
321+
next();
322+
});
323+
app.get('/foo/:user', function(req, res, next) {
324+
count++;
325+
next();
326+
});
327+
328+
app.use(function(err, req, res, next) {
329+
res.status(500);
330+
res.send([count, called, err.message].join(' '));
331+
});
332+
333+
request(app)
334+
.get('/foo/bob')
335+
.expect(500, '0 1 err!', done)
336+
});
337+
338+
it('should call when values differ when using "next"', function(done) {
339+
var app = express();
340+
var called = 0;
341+
var count = 0;
342+
343+
app.param('user', function(req, res, next, user) {
344+
called++;
345+
if (user === 'foo') return next('route');
346+
req.user = user;
347+
next();
348+
});
349+
350+
app.get('/:user/bob', function(req, res, next) {
351+
count++;
352+
next();
353+
});
354+
app.get('/foo/:user', function(req, res, next) {
355+
count++;
356+
next();
357+
});
358+
app.use(function(req, res) {
359+
res.end([count, called, req.user].join(' '));
360+
});
361+
362+
request(app)
363+
.get('/foo/bob')
364+
.expect('1 2 bob', done);
365+
})
306366
})
307367
})

0 commit comments

Comments
 (0)