dotnet / dotnet/aspnetcore

Rate limiting middleware OnReject assign bug

Open
#49,379 3 comments 0 reactions 0 assignees View on GitHub
area-middleware feature-rate-limit Needs: Attention :wave:
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Describe the bug

Please check the code below(located 130~134 lines of source code):

```csharp
internal sealed partial class RateLimitingMiddleware
{
private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute)
{
policy = enableRateLimitingAttribute?.Policy;
if (policy is not null)
{
thisRequestOnRejected = policy.OnRejected;
}
}
}
```

I think we should check if `Policy.OnReject` is null here, if not null, then use it, else use `OnRejected` from the `Options` if available

### Expected Behavior

```csharp
internal sealed partial class RateLimitingMiddleware
{
private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute)
{
policy = enableRateLimitingAttribute?.Policy;
if (policy?.OnRejected is not null)
{
thisRequestOnRejected = policy.OnRejected;
}
}
}
```

### Steps To Reproduce

```csharp
public class MyRateLimiterPolicy : IRateLimiterPolicy
{
public MyRateLimiterPolicy()
{
}

// keep it null
public Func? OnRejected { get; }

public RateLimitPartition GetPartition(HttpContext httpContext)
{
return RateLimitPartition.GetFixedWindowLimiter("anything", _ => new FixedWindowRateLimiterOptions
{
PermitLimit = 3,
Window = TimeSpan.FromSeconds(60),
QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
QueueLimit = 0
});
}
}

builder.Services.AddRateLimiter(limiterOptions =>
{
limiterOptions.OnRejected = (context, cancellationToken) =>
{
return ValueTask.CompletedTask;
};
}

app.MapGet("LimitTest", async () =>
{
return Results.Ok("Limiter");
}).RequireRateLimiting(new MyRateLimiterPolicy());
```

### Exceptions (if any)

_No response_

### .NET Version

.NET 7, 8

### Anything else?

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