express-limiter
express-limiter copied to clipboard
Having a function for lookup seems to only work for the first request
I "consoled" the block about the lookup option being a function:
if (typeof(opts.lookup) === 'function') {
console.log('1:' + opts.lookup);
middleware = function (middleware, req, res, next) {
console.log('2:' + opts.lookup);
return opts.lookup(req, res, opts, function () {
console.log('3:' + opts.lookup);
return middleware(req, res, next)
})
}.bind(this, middleware)
}
Then I booted the server:
Tue Sep 29 2015 12:38:06 GMT-0400 (EDT): 1:function (req, res, opts, next) {
if (req.body && req.body.uuid) {
opts.lookup = ['body.uuid', 'params.uid'];
} else {
opts.lookup = ['connection.remoteAddress', 'params.uid'];
opts.total = 100;
}
return next();
}
Then I ran the first request:
2:function (req, res, opts, next) {
if (req.body && req.body.uuid) {
opts.lookup = ['body.uuid', 'params.uid'];
} else {
opts.lookup = ['connection.remoteAddress', 'params.uid'];
opts.total = 100;
}
return next();
}
3:connection.remoteAddress,params.uid
Then I ran the second request:
2:connection.remoteAddress,params.uid
Tue Sep 29 2015 12:38:23 GMT-0400 (EDT): TypeError: Property 'lookup' of object #<Object> is not a function
Notice the second go console log 2 has been updated to what was console log 3's.
I had to write a work around in my code:
var limiterCreator = require('express-limiter')(app, client);
function recreateLimiter(opts) {
return function() {
var limiter = limiterCreator(_.extend({}, opts));
return limiter.apply(this, arguments);
}
}
Maybe I'm missing something really obvious but the fact that the limiter only gets created once and the original opts object is modified on subsequent calls seems like bad news. Perhaps cloning the opts at the beginning using underscore could mitigate this problem.
i gotcha. i'll have a look to see what we can do about that
@kentmw , @ded : I had the same issue. I create a pull request to fix this issue. (https://github.com/ded/express-limiter/pull/21)
@kentmw : Meanwhile the merge or another fix you may use my fork. ;)
@vamonte: Your code worked. @ded: Why don't you merge @vamonte's PR?
@ded Thank's for the merge. Do you know when you're going to tag the next release? I would like to use this package in an automated build.
@vamonte ++
@vamonte ++ @ded just a reminder to tag the release.
@vamonte ++ @ded We are also looking to use this package in an automated build in the near future, so a new tag would be much appreciated.
New release please @ded
Just ran into this too, any word on a new release @ded?
Is this fixed now, per #21?