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

API rate limiting / Cache #15152

Closed
zoltanpeto opened this issue Aug 30, 2016 · 1 comment
Closed

API rate limiting / Cache #15152

zoltanpeto opened this issue Aug 30, 2016 · 1 comment

Comments

@zoltanpeto
Copy link
Contributor

Hi there,

I came accross the following issue, not sure if it's a bug, or I misunderstood something:

We have an API endpoint that has the default throttle middleware applied (60 requests / minute). Our client posts to this endpoint quite frequently using an automated service. It is set now to wait 20 seconds between each request - this means in theory they send 3 requests / minute. However, they are being locked out by rate limiter all the time.
By doing some debugging, I realised that every time when RateLimiter::hit() is called, the cache file containing the attempt counter is renewed.
It looks like the root of the problem is in Illuminate\Cache\FileStore L87:

// Next, we'll extract the number of minutes that are remaining for a cache
// so that we can properly retain the time for things like the increment
// operation that may be performed on the cache. We'll round this out.
$time = ceil(($expire - time()) / 60);

$time will always be 1, if we limit per minute, which means that the client will always reach their limit even is it's set to 1 million (as long as they keep hitting the endpoint at least once every minute).

Do you think this is a legit bug, or is it intentional? What would be the solution to this (apart from increasing $decayMinutes parameter on ThrottleRequests middleware)?

@GrahamCampbell
Copy link
Member

We'd accept a PR to correct our file store on 5.3. The minimum cache time should be 1 second, not 60. That goes for all drivers.

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

2 participants