ded / express-limiter

Rate limiting middleware for Express
MIT License
423 stars 54 forks source link

Having a function for lookup seems to only work for the first request #19

Open kentmw opened 9 years ago

kentmw commented 9 years ago

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.

kentmw commented 9 years ago

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.

ded commented 9 years ago

i gotcha. i'll have a look to see what we can do about that

vamonte commented 8 years ago

@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. ;)

tuanquynet commented 8 years ago

@vamonte: Your code worked. @ded: Why don't you merge @vamonte's PR?

vamonte commented 8 years ago

@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.

kiebzak commented 8 years ago

@vamonte ++

cloud2pilot commented 8 years ago

@vamonte ++ @ded just a reminder to tag the release.

TheSoundDefense commented 8 years ago

@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.

v-kat commented 8 years ago

New release please @ded

nickdaugherty commented 7 years ago

Just ran into this too, any word on a new release @ded?

skeggse commented 5 years ago

Is this fixed now, per #21?