StevenThuriot / express-rate-limiter

Rate limiter middleware for express applications
https://www.npmjs.org/package/express-rate-limiter
MIT License
5 stars 4 forks source link

Limit Date Should Not Be Altered During Session #13

Closed carlosrymer closed 8 years ago

carlosrymer commented 8 years ago

We found a bug where the innerTimeLimit option was not being respected. This was due to a line where the limit (which tracks each session per hit in the store module) object's date property was being reset to the current time on every hit.

Instead, the date property should only be set if the limit object has not yet been assigned. That date should then remain unchanged for the duration of the allowed session as defined in the configuration. The change here simply removes resetting that date while the session is ongoing (i.e. while limit is still truthy).

StevenThuriot commented 8 years ago

Good catch... However:

Rather than fully removing it, I think it should still be set if resetInner === true. If you don't set it at that point, the inner test will block off calls until the whole instance resets.

e.g. inner reset is 3 attempts during 5 seconds, outer reset is 60 attempts over a minute. Without setting it, if you attempt 4 times in 5 seconds, you'll be blocked for the full minute. The point of the inner reset was to block the attempts during those 5 seconds and then let them try again, as long as they don't go over the 60 attempts in the minute timespan.

Do you agree?

carlosrymer commented 8 years ago

Good point. I wrongly assumed that limit was being nullified in self.decreaseLimits. Let me make that update.

carlosrymer commented 8 years ago

So it looks like you already fixed it. Should we just decline this PR? Also, do you want to bump the version?

StevenThuriot commented 8 years ago

I was going over the code, trying to remember it all again, so I had already checked it in.

I've merged it anyway so your name is now in the contributers list, and the version needed to be bumped anyway :smile:

Thanks again!

carlosrymer commented 8 years ago

Awesome, your welcome and thanks for the quick turnaround! :)