dotnet / dotnet/aspnetcore

ChainedPartitionedRateLimiter double-counts when internal limiter uses queue

Open
#60,852 1 comment 1 reaction 0 assignees View on GitHub
area-middleware feature-rate-limit
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

### Description

When creating a ChainedPartionedRateLimiter that uses a FixedWindowRateLimiter/SlidingWindowRateLimiter/TokenBucketRateLimiter followed by a rate limiter configured to allow a queue, requests that are queued by the inner limiter will count as 2 requests for the outer limiter.

This is because after AttemptAcquireCore fails due to a queue on an internal limiter, ChainedPartionedRateLimiter will attempt to return already acquired leases by disposing, but affected limiter leases offer no behaviour to return leases to the pool on disposal. As a result after calling AcquireAsyncCore to wait for the queue, the request will require 2 leases from the outer limiter.

### Reproduction Steps

Program.cs:
```
var builder = WebApplication.CreateBuilder(args);

builder.Services.AddRateLimiter(_ =>
{
_.OnRejected = async (context, cancellationToken) =>
{
if (context.Lease.TryGetMetadata(MetadataName.RetryAfter, out var retryAfter))
{
context.HttpContext.Response.Headers.RetryAfter =
((int)retryAfter.TotalSeconds).ToString(NumberFormatInfo.InvariantInfo);
}

context.HttpContext.Response.StatusCode = StatusCodes.Status429TooManyRequests;
await context.HttpContext.Response.WriteAsync("Too many requests. Please try again later.", cancellationToken);
};
_.GlobalLimiter = PartitionedRateLimiter.CreateChained(
PartitionedRateLimiter.Create(httpContext =>
{
var userAgent = httpContext.Request.Headers.UserAgent.ToString();

return RateLimitPartition.GetFixedWindowLimiter
(userAgent, _ =>
new FixedWindowRateLimiterOptions
{
AutoReplenishment = true,
PermitLimit = 2,
Window = TimeSpan.FromSeconds(10)
});
}),
PartitionedRateLimiter.Create(httpContext =>
{
var userAgent = httpContext.Request.Headers.UserAgent.ToString();

return RateLimitPartition.GetFixedWindowLimiter
(userAgent, _ =>
new FixedWindowRateLimiterOptions
{
AutoReplenishment = true,
PermitLimit = 1,
QueueLimit = 1,
Window = TimeSpan.FromSeconds(10)
});
}));
});

var app = builder.Build();
app.UseRateLimiter();

static string GetTicks() => (DateTime.Now.Ticks & 0x11111).ToString("00000");

app.MapGet("/", () => Results.Ok($"Hello {GetTicks()}"));

app.Run();
```

Above configures a chained rate limiter with 2 fixed window limiters, the outer allowing 2 requests per window and the inner allowing 1 and 1 queued. Expected behaviour would be 2 requests within a window would result in neither failing, with the first completing immediately and the second completing after queueing.

In reality, sending two requests within a 10s window will result in failure with status 429, as the second request is counted twice against the outer limiter, resulting in 3 permits which exceeds the allowance.

### Expected behavior

Queued requests should count as a single request for other rate limiters sharing a ChainedPartionedRateLimiter.

### Actual behavior

Queued requests count as a two requests for rate limiters sharing a ChainedPartionedRateLimiter that come before the queue in the chain.

### Regression?

_No response_

### Known Workarounds

_No response_

### Configuration

.NET SDK 9.0.200

Tested on linux/windows x64, should apply to all configurations.

### Other information

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.