Make RateLimitingMiddleware endpoint aware #42417
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using Microsoft.AspNetCore.Builder; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.EntityFrameworkCore; | ||
| using System.Threading.RateLimiting; | ||
| using Microsoft.AspNetCore.RateLimiting; | ||
| using RateLimitingSample; | ||
| using Microsoft.Extensions.Logging.Abstractions; | ||
|
|
||
| var builder = WebApplication.CreateBuilder(args); | ||
| builder.Services.AddDbContext<TodoDb>(opt => opt.UseInMemoryDatabase("TodoList")); | ||
| builder.Services.AddDatabaseDeveloperPageExceptionFilter(); | ||
| // Inject an ILogger<SampleRateLimiterPolicy> | ||
| builder.Services.AddLogging(); | ||
|
|
||
| var app = builder.Build(); | ||
|
|
||
| var todoName = "todoPolicy"; | ||
| var completeName = "completePolicy"; | ||
| var helloName = "helloPolicy"; | ||
|
|
||
| // Define endpoint limiters and a global limiter. | ||
| var options = new RateLimiterOptions() | ||
|
There was a problem hiding this comment. Why isn't this using services? There was a problem hiding this comment. Updated the sample to inject an There was a problem hiding this comment. I was asking why the options was being newed up instead of a call to There was a problem hiding this comment. We removed that extension in the last API review: #37384 (comment), in favor of just the There was a problem hiding this comment. OK that doesn't look ideal at all. It doesn't look like any of the other middleware that is configured with this level of policy. @Tratcher @halter73 @BrennanConroy What was the thinking here? There was a problem hiding this comment. I think we need a do over here to align with the patterns we have in the rest of the stack. The newly authored output caching middleware also follows this pattern There was a problem hiding this comment. There are a few other places we do it this way: But I think long term it's probably better to switch to There was a problem hiding this comment. @davidfowl You're late to the party! You should read my thoughts on this at #41655 (comment) when @Elfocrash made the same point if you haven't yet. We're way more consistent with this options pattern (or should I say lack of options pattern) for middleware with no services than I would have thought. We're rarely this consistent with anything else 🤣.
Completely agree. I do understand no matter how consistent we are with not using the options pattern in this specific scenario, it's rare to use middleware without any default services but with interesting options must people will want to configure. And this does feel awkward when you're more used to configuring options for middleware that consumes services. Is it likely that There was a problem hiding this comment. None of those options are this complex. There's no policy, no methods, just properties on pocos. |
||
| .AddTokenBucketLimiter(todoName, new TokenBucketRateLimiterOptions(1, QueueProcessingOrder.OldestFirst, 1, TimeSpan.FromSeconds(10), 1)) | ||
| .AddPolicy<string>(completeName, new SampleRateLimiterPolicy(NullLogger<SampleRateLimiterPolicy>.Instance)) | ||
| .AddPolicy<string, SampleRateLimiterPolicy>(helloName); | ||
| // The global limiter will be a concurrency limiter with a max permit count of 10 and a queue depth of 5. | ||
| options.GlobalLimiter = PartitionedRateLimiter.Create<HttpContext, string>(context => | ||
| { | ||
| return RateLimitPartition.CreateConcurrencyLimiter<string>("globalLimiter", key => new ConcurrencyLimiterOptions(10, QueueProcessingOrder.NewestFirst, 5)); | ||
| }); | ||
| app.UseRateLimiter(options); | ||
|
|
||
| // The limiter on this endpoint allows 1 request every 5 seconds | ||
| app.MapGet("/", () => "Hello World!").RequireRateLimiting(helloName); | ||
|
|
||
| // Requests to this endpoint will be processed in 10 second intervals | ||
| app.MapGet("/todoitems", async (TodoDb db) => | ||
| await db.Todos.ToListAsync()) | ||
| .RequireRateLimiting(todoName); | ||
|
|
||
| // The limiter on this endpoint allows 1 request every 5 seconds | ||
| app.MapGet("/todoitems/complete", async (TodoDb db) => | ||
| await db.Todos.Where(t => t.IsComplete).ToListAsync()) | ||
| .RequireRateLimiting(completeName); | ||
|
|
||
| app.MapGet("/todoitems/{id}", async (int id, TodoDb db) => | ||
| await db.Todos.FindAsync(id) | ||
| is Todo todo | ||
| ? Results.Ok(todo) | ||
| : Results.NotFound()); | ||
|
|
||
| app.MapPost("/todoitems", async (Todo todo, TodoDb db) => | ||
| { | ||
| db.Todos.Add(todo); | ||
| await db.SaveChangesAsync(); | ||
|
|
||
| return Results.Created($"/todoitems/{todo.Id}", todo); | ||
| }); | ||
|
|
||
| app.MapPut("/todoitems/{id}", async (int id, Todo inputTodo, TodoDb db) => | ||
| { | ||
| var todo = await db.Todos.FindAsync(id); | ||
|
|
||
| if (todo is null) | ||
| { | ||
| return Results.NotFound(); | ||
| } | ||
|
|
||
| todo.Name = inputTodo.Name; | ||
| todo.IsComplete = inputTodo.IsComplete; | ||
|
|
||
| await db.SaveChangesAsync(); | ||
|
|
||
| return Results.NoContent(); | ||
| }); | ||
|
|
||
| app.MapDelete("/todoitems/{id}", async (int id, TodoDb db) => | ||
| { | ||
| if (await db.Todos.FindAsync(id) is Todo todo) | ||
| { | ||
| db.Todos.Remove(todo); | ||
| await db.SaveChangesAsync(); | ||
| return Results.Ok(todo); | ||
| } | ||
|
|
||
| return Results.NotFound(); | ||
| }); | ||
|
|
||
| app.Run(); | ||
|
|
||
| class Todo | ||
| { | ||
| public int Id { get; set; } | ||
| public string? Name { get; set; } | ||
| public bool IsComplete { get; set; } | ||
| } | ||
|
|
||
| class TodoDb : DbContext | ||
| { | ||
| public TodoDb(DbContextOptions<TodoDb> options) | ||
| : base(options) { } | ||
|
|
||
| public DbSet<Todo> Todos => Set<Todo>(); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| { | ||
| "$schema": "https://json.schemastore.org/launchsettings.json", | ||
| "iisSettings": { | ||
| "windowsAuthentication": false, | ||
| "anonymousAuthentication": true, | ||
| "iisExpress": { | ||
| "applicationUrl": "http://localhost:8855", | ||
| "sslPort": 44312 | ||
| } | ||
| }, | ||
| "profiles": { | ||
| "RateLimitingSample": { | ||
| "commandName": "Project", | ||
| "dotnetRunMessages": true, | ||
| "launchBrowser": true, | ||
| "launchUrl": "swagger", | ||
| "applicationUrl": "https://localhost:7036;http://localhost:5085", | ||
| "environmentVariables": { | ||
| "ASPNETCORE_ENVIRONMENT": "Development" | ||
| } | ||
| }, | ||
| "IIS Express": { | ||
| "commandName": "IISExpress", | ||
| "launchBrowser": true, | ||
| "launchUrl": "swagger", | ||
| "environmentVariables": { | ||
| "ASPNETCORE_ENVIRONMENT": "Development" | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk.Web"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework> | ||
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Reference Include="Microsoft.AspNetCore" /> | ||
| <Reference Include="Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore" /> | ||
| <Reference Include="Microsoft.AspNetCore.HttpsPolicy" /> | ||
| <Reference Include="Microsoft.AspNetCore.Http.Results" /> | ||
| <Reference Include="Microsoft.AspNetCore.Mvc" /> | ||
| <Reference Include="Microsoft.AspNetCore.RateLimiting" /> | ||
| <Reference Include="Microsoft.EntityFrameworkCore.InMemory" /> | ||
| <Reference Include="Microsoft.Extensions.DependencyInjection" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Threading.RateLimiting; | ||
| using Microsoft.AspNetCore.RateLimiting; | ||
|
|
||
| namespace RateLimitingSample; | ||
|
|
||
| public class SampleRateLimiterPolicy : IRateLimiterPolicy<string> | ||
| { | ||
| private Func<OnRejectedContext, CancellationToken, ValueTask>? _onRejected; | ||
|
|
||
| public SampleRateLimiterPolicy(ILogger<SampleRateLimiterPolicy> logger) | ||
| { | ||
| _onRejected = (context, token) => | ||
| { | ||
| context.HttpContext.Response.StatusCode = 429; | ||
| logger.LogInformation($"Request rejected by {nameof(SampleRateLimiterPolicy)}"); | ||
| return ValueTask.CompletedTask; | ||
| }; | ||
| } | ||
|
|
||
| public Func<OnRejectedContext, CancellationToken, ValueTask>? OnRejected { get => _onRejected; } | ||
|
|
||
| // Use a sliding window limiter allowing 1 request every 10 seconds | ||
| public RateLimitPartition<string> GetPartition(HttpContext httpContext) | ||
| { | ||
| return RateLimitPartition.CreateSlidingWindowLimiter<string>(string.Empty, key => new SlidingWindowRateLimiterOptions(1, QueueProcessingOrder.OldestFirst, 1, TimeSpan.FromSeconds(5), 1)); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "Logging": { | ||
| "LogLevel": { | ||
| "Default": "Information", | ||
| "Microsoft.AspNetCore": "Warning" | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "Logging": { | ||
| "LogLevel": { | ||
| "Default": "Information", | ||
| "Microsoft.AspNetCore": "Warning" | ||
| } | ||
| }, | ||
| "AllowedHosts": "*" | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Threading.RateLimiting; | ||
|
|
||
| namespace Microsoft.AspNetCore.RateLimiting; | ||
|
|
||
| internal sealed class DefaultCombinedLease : RateLimitLease | ||
wtgodbe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| private readonly RateLimitLease _globalLease; | ||
| private readonly RateLimitLease _endpointLease; | ||
| private HashSet<string>? _metadataNames; | ||
|
|
||
| public DefaultCombinedLease(RateLimitLease globalLease, RateLimitLease endpointLease) | ||
| { | ||
| _globalLease = globalLease; | ||
| _endpointLease = endpointLease; | ||
| } | ||
|
|
||
| public override bool IsAcquired => true; | ||
|
|
||
| public override IEnumerable<string> MetadataNames | ||
| { | ||
| get | ||
| { | ||
| if (_metadataNames is null) | ||
| { | ||
| _metadataNames = new HashSet<string>(); | ||
| foreach (var metadataName in _globalLease.MetadataNames) | ||
| { | ||
| _metadataNames.Add(metadataName); | ||
| } | ||
| foreach (var metadataName in _endpointLease.MetadataNames) | ||
| { | ||
| _metadataNames.Add(metadataName); | ||
| } | ||
| } | ||
| return _metadataNames; | ||
| } | ||
| } | ||
|
|
||
| public override bool TryGetMetadata(string metadataName, out object? metadata) | ||
| { | ||
| // Use the first metadata item of a given name, ignore duplicates, we can't reliably merge arbitrary metadata | ||
| // Creating an object[] if there are multiple of the same metadataName could work, but makes consumption of metadata messy | ||
| // and makes MetadataName.Create<T>(...) uses no longer work | ||
| if (_endpointLease.TryGetMetadata(metadataName, out metadata)) | ||
| { | ||
| return true; | ||
| } | ||
| if (_globalLease.TryGetMetadata(metadataName, out metadata)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| metadata = null; | ||
| return false; | ||
| } | ||
|
|
||
| protected override void Dispose(bool disposing) | ||
| { | ||
| List<Exception>? exceptions = null; | ||
|
|
||
| // Dispose endpoint lease first, then global lease (reverse order of when they were acquired) | ||
| // Avoids issues where dispose might unblock a queued acquire and then the acquire fails when acquiring the next limiter. | ||
| // When disposing in reverse order there wont be any issues of unblocking an acquire that affects acquires on limiters in the chain after it | ||
| try | ||
| { | ||
| _endpointLease.Dispose(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| exceptions ??= new List<Exception>(); | ||
| exceptions.Add(ex); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| _globalLease.Dispose(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| exceptions ??= new List<Exception>(1); | ||
| exceptions.Add(ex); | ||
| } | ||
|
|
||
| if (exceptions is not null) | ||
| { | ||
| if (exceptions.Count == 1) | ||
| { | ||
| throw exceptions[0]; | ||
| } | ||
| else | ||
| { | ||
| throw new AggregateException(exceptions); | ||
| } | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to allow for the
requiredkeyword I use in many places in this PR