Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple rate limiters using same key fail to behave as expected #46290

Closed
ceejayoz opened this issue Feb 27, 2023 · 3 comments
Closed

Multiple rate limiters using same key fail to behave as expected #46290

ceejayoz opened this issue Feb 27, 2023 · 3 comments

Comments

@ceejayoz
Copy link
Contributor

  • Laravel Version: 10.1.5
  • PHP Version: 8.2.1
  • Database Driver & Version: Redis 6.x

Description:

Reopening #45088 (closed because issue was originally filed under Laravel 8.x). This issue persists in Laravel 10.

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.

@rodrigopedra
Copy link
Contributor

Can you try using different keys as a workaround?

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

Internally, the Rate Limiter uses the Cache Store to store its individual limiters, so I don't think a workaround is easy.

Maybe adding the $maxAttempts and $decayMinutes to the key limiter under the hood, could solve the problem.

@ceejayoz
Copy link
Contributor Author

@rodrigopedra Yes; I'm using that workaround (#45088 (comment)) having just encountered the issue for the first time. If it's challenging to fix, it probably warrants a note in the docs; it took me quite some time to realize there was something framework-level happening.

@driesvints
Copy link
Member

driesvints commented Mar 1, 2023

This is expected right now and unfortunately there's no way around this. We'd appreciate a PR to the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants