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

Configuring a global & route limit #19

Open
ItsReddi opened this issue Sep 29, 2023 · 2 comments
Open

Configuring a global & route limit #19

ItsReddi opened this issue Sep 29, 2023 · 2 comments

Comments

@ItsReddi
Copy link

ItsReddi commented Sep 29, 2023

I want to configure a global limit and i want to configure Route limits.

I think my problem is about the understanding of:
https://github.com/artisansdk/ratelimiter#how-multiple-buckets-work

So i have setup a default in the kernel:

protected $middlewareGroups = [
        'web' => [
            'throttle:10,1,30',
            \App\Http\Middleware\EncryptCookies::class,
            \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
            \Illuminate\Session\Middleware\StartSession::class,
            \Illuminate\View\Middleware\ShareErrorsFromSession::class,
            \App\Http\Middleware\VerifyCsrfToken::class,
            \Illuminate\Routing\Middleware\SubstituteBindings::class,
        ],

        'api' => [
            //default throttle | allow 600 requests | every second you gain 2 more requests | on exceeding limit, wait for 300 seconds
            //translates to: the full 600 requests are backfilled in 5 minutes you can make up to 1200 requests in 5 minutes
            'throttle:600,2,300',
            // \Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful::class,
            \Illuminate\Routing\Middleware\SubstituteBindings::class,
            
        ],
    ];

And configured:

use Illuminate\Support\Facades\Route;
use ArtisanSdk\RateLimiter\Resolvers\Route as Limiter;

Route::group([
    'prefix' => 'v1',
    'namespace' => 'App\Http\Controllers\Api\V1',
    //'middleware' => ['auth:api'],
    'middleware' => 'throttle:' . Limiter::class . ',5,1,30',
], function () {
    Route::apiResource('tasks', TaskController::class, [
        
    ]);
});

So 5 requests at start, 1/s drain, 30 sec penalty.

The rate limits are enforced correctly 👍
But the returned headers are always the headers, from the global default and not from Route Limit.
That makes it impossible to know for the clients, what limits are on a route.

Response while not limited: (request on /api/v1/tasks)

HTTP/1.1 200 OK
Host: localhost:8400
Date: Fri, 29 Sep 2023 11:32:52 GMT
Connection: close
X-Powered-By: PHP/8.2.10
Content-Type: text/html; charset=UTF-8
Cache-Control: no-cache, private
Date: Fri, 29 Sep 2023 11:32:52 GMT
X-RateLimit-Limit: 600
X-RateLimit-Remaining: 599
Access-Control-Allow-Origin: *

While limited: (request on /api/v1/tasks)

HTTP/1.1 429 Too Many Requests
Host: localhost:8400
Date: Fri, 29 Sep 2023 11:33:46 GMT
Connection: close
X-Powered-By: PHP/8.2.10
X-RateLimit-Limit: 600
X-RateLimit-Remaining: 594
retry-after: 30
x-ratelimit-reset: 1695987256
Cache-Control: no-cache, private
date: Fri, 29 Sep 2023 11:33:46 GMT
Content-Type: text/html; charset=UTF-8
Access-Control-Allow-Origin: *

The retry-after seems to be correct but not Limit and Remaining
So i am unsure if its a bug or i misunderstand the README.
Thanks in advance.

@dalabarge
Copy link
Contributor

Thanks @ItsReddi for using the package and for leaving your issue. I'm looking into reproducing the issue and will respond with a solution if we identify a bug.

I think at first glance, X-RateLimit-Limit is supposed to remain at the upper limit of 600 while X-RateLimit-Remaining is to tell you how many hits you have left in the bucket before hitting the 600 limit. So in your case 599 then 594. The retry-after should be formatted X-RateLimit-Retry-After and should only show up when you are rate limited. The X-RateLimit-Reset I supposed to be the unix timestamp of when your timeout is reset.

Another issue might be your actual configuration and our explanation of multi-bucket usage. We've got some improved documentation we are working on posting which should help clarify issues like this. Give me a bit of time and we'll get this all sorted out for you.

If you need something more urgently reach out to [email protected].

@ItsReddi
Copy link
Author

ItsReddi commented Oct 11, 2023

Thanks for your reply i could set up an reproduction repository / with a vscode .devcontainer if that would help.
More than this i am unsure if we both understood the same issue.
I see the following issue, with the above configuration:

Actual result not hitting the limit on /api/v1/tasks:

...
X-RateLimit-Limit: 600    <--- thats from the global bucket
X-RateLimit-Remaining: 599 <--- thats from the global bucket
...

Expected result not hitting the limit on /api/v1/tasks:

...
X-RateLimit-Limit: 5 <--- should be from the route bucket
X-RateLimit-Remaining: 4 <--- should be from the route bucket
...

Actual result hitting the limit on /api/v1/tasks: (while beeing throttled, after 5 requests)
Here comes a mixed response from both buckets.

...
X-RateLimit-Limit: 600 <--- thats from the global bucket
X-RateLimit-Remaining: 594 <--- thats from the global bucket
retry-after: 30 <--- thats from the route bucket
x-ratelimit-reset: 1695987256 <--- unsure, from where it is, did not calulate
...

Epected result hitting the limit on /api/v1/tasks: (while beeing throttled, after 5 requests)

...
X-RateLimit-Limit: 5 <--- thats from the route bucket
X-RateLimit-Remaining: 0 <--- thats from the route bucket
retry-after: 30 <--- thats from the route bucket
x-ratelimit-reset: 1695987256 <--- thats from the route bucket
...

Furthermore:
I would expect another result if i would be limited due to the global bucket.
For example if i would overflow the global bucket with api/v2 requests, api/v1 requests should return:

...
X-RateLimit-Limit: 600 <--- thats from the global bucket
X-RateLimit-Remaining: 0 <--- thats from the global bucket
retry-after: 300<--- thats from the global bucket
x-ratelimit-reset: 1695987256 <--- thats from the global bucket
...

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

No branches or pull requests

2 participants