express-limiter icon indicating copy to clipboard operation
express-limiter copied to clipboard

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

Open kentmw opened this issue 9 years ago • 11 comments

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 avatar Sep 29 '15 16:09 kentmw

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.

kentmw avatar Sep 29 '15 17:09 kentmw

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

ded avatar Sep 29 '15 18:09 ded

@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 avatar Dec 03 '15 22:12 vamonte

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

tuanquynet avatar Dec 04 '15 19:12 tuanquynet

@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 avatar Dec 06 '15 21:12 vamonte

@vamonte ++

kiebzak avatar Jan 15 '16 14:01 kiebzak

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

cloud2pilot avatar May 02 '16 17:05 cloud2pilot

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

TheSoundDefense avatar Jul 27 '16 19:07 TheSoundDefense

New release please @ded

v-kat avatar Oct 07 '16 17:10 v-kat

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

nickdaugherty avatar Jul 19 '17 23:07 nickdaugherty

Is this fixed now, per #21?

skeggse avatar Nov 20 '18 03:11 skeggse