artisansdk / ratelimiter

A leaky bucket rate limiter and corresponding middleware with route-level granularity compatible with Laravel.
MIT License
136 stars 17 forks source link

Configuring a global & route limit #19

Open ItsReddi opened 1 year ago

ItsReddi commented 1 year ago

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 commented 1 year ago

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 support@artisanmade.io.

ItsReddi commented 1 year ago

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
...