framework icon indicating copy to clipboard operation
framework copied to clipboard

Multiple rate limiters using same key fail to behave as expected

Open mwargan opened this issue 3 years ago • 1 comments

  • Laravel Version: 8.83.26
  • PHP Version: 8.1.1
  • Database Driver & Version: MariaDB 10.4

Description:

Using multiple rate-limiters that use the same key does not work as expected. If the key is the same, only the first rate limiter is ever triggered.

Additionally, because both limiters use the same key, the count of remaining requests is decremented by the amount of times the same key is used.

Steps To Reproduce:

Define a simple rate limiter that returns an array of limits, using the same key.

RateLimiter::for('maps', function (Request $request) {
    return [
            Limit::perMinute(3)->by($request->ip()),
            Limit::perDay(30)->by($request->ip())
        ];
});

The above code will only apply the first limit, but the "Remaining" is decremented by two because both use the same key.

An alternative way that I was hoping would work but does not either:

return Limit::perMinute(3)->perDay(30)->by($request->ip());

In the above case, only the second limiting method applies (so perMinute is ignored, but perDay applies). Looking at the source code of the Limit class, it makes total sense and looks like it would require quite a few changes to make it work, so I think that its perhaps not as big an issue as the first case.

mwargan avatar Nov 25 '22 15:11 mwargan

Note as a workaround, you can just define a different key for each limiter, e.g. by($request->ip() . '-minute'). It doesn't feel very clean though :P

mwargan avatar Nov 25 '22 15:11 mwargan

Hey there,

Unfortunately we don't support this version anymore. Please check out our support policy on which versions we are currently supporting. Can you please try to upgrade to the latest version and see if your problem persists? If so, please open up a new issue and we'll help you out.

Thanks!

driesvints avatar Nov 26 '22 11:11 driesvints