diff --git a/src/Middleware/RateLimiting/src/RateLimiterEndpointConventionBuilderExtensions.cs b/src/Middleware/RateLimiting/src/RateLimiterEndpointConventionBuilderExtensions.cs index 3fb4d423e047..32d0f3520dc7 100644 --- a/src/Middleware/RateLimiting/src/RateLimiterEndpointConventionBuilderExtensions.cs +++ b/src/Middleware/RateLimiting/src/RateLimiterEndpointConventionBuilderExtensions.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Runtime.CompilerServices; using Microsoft.AspNetCore.RateLimiting; namespace Microsoft.AspNetCore.Builder; @@ -40,9 +41,18 @@ public static TBuilder RequireRateLimiting(this TBuilde ArgumentNullException.ThrowIfNull(builder); ArgumentNullException.ThrowIfNull(policy); + // Inline policies are not named, so historically they all shared a single null partition-key + // namespace. That let two distinct inline policies collide onto the same limiter whenever their + // GetPartition methods returned equal keys. Derive a stable namespace from the policy instance so + // distinct policy objects are isolated while the same instance reused across endpoints still shares + // a limiter. This is captured once here rather than inside the convention callback so endpoint + // rebuilds don't regenerate it. It is intentionally kept separate from the telemetry-facing + // EnableRateLimitingAttribute.PolicyName so rate-limiting metrics are unaffected. + var policyName = $"__inlinePolicy_{RuntimeHelpers.GetHashCode(policy)}"; + builder.Add(endpointBuilder => { - endpointBuilder.Metadata.Add(new EnableRateLimitingAttribute(new DefaultRateLimiterPolicy(RateLimiterOptions.ConvertPartitioner(null, policy.GetPartition), policy.OnRejected))); + endpointBuilder.Metadata.Add(new EnableRateLimitingAttribute(new DefaultRateLimiterPolicy(RateLimiterOptions.ConvertPartitioner(policyName, policy.GetPartition), policy.OnRejected))); }); return builder; } diff --git a/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs b/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs index 2792d51d6797..a44985b12aa3 100644 --- a/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs +++ b/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs @@ -579,6 +579,61 @@ public async Task PolicyDirectlyOnEndpoint_GetsUsed() Assert.Equal(StatusCodes.Status404NotFound, context.Response.StatusCode); } + [Fact] + public async Task EndpointLimiter_InlinePolicy_DuplicatePartitionKey_NoCollision() + { + // Two distinct inline policy instances that return the same partition key must not share a + // limiter. Before the fix both inline policies used a null partition-key namespace, so whichever + // endpoint ran first created the shared limiter and the other silently reused it. + var options = CreateOptionsAccessor(); + var duplicateKey = "myKey"; + var policy1 = new CountingRateLimiterPolicy(duplicateKey, permitLimit: 1); + var policy2 = new CountingRateLimiterPolicy(duplicateKey, permitLimit: 1); + + var middleware = CreateTestRateLimitingMiddleware(options); + + var endpoint1 = CreateEndpointWithRateLimitPolicy(policy1); + var endpoint2 = CreateEndpointWithRateLimitPolicy(policy2); + + // Exhaust policy1's single permit on endpoint1. + var context = new DefaultHttpContext(); + context.SetEndpoint(endpoint1); + await middleware.Invoke(context).DefaultTimeout(); + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + // endpoint2 uses a different policy instance, so it has its own limiter and its own permit. + context = new DefaultHttpContext(); + context.SetEndpoint(endpoint2); + await middleware.Invoke(context).DefaultTimeout(); + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + } + + [Fact] + public async Task EndpointLimiter_InlinePolicy_SameInstance_SharesLimiter() + { + // The same inline policy instance reused across endpoints should keep sharing a single limiter, + // so per-instance identity (not per-registration) is used for the partition-key namespace. + var options = CreateOptionsAccessor(); + var policy = new CountingRateLimiterPolicy("myKey", permitLimit: 1); + + var middleware = CreateTestRateLimitingMiddleware(options); + + var endpoint1 = CreateEndpointWithRateLimitPolicy(policy); + var endpoint2 = CreateEndpointWithRateLimitPolicy(policy); + + // First endpoint consumes the single shared permit. + var context = new DefaultHttpContext(); + context.SetEndpoint(endpoint1); + await middleware.Invoke(context).DefaultTimeout(); + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + // Second endpoint shares the same limiter, so no permit remains and it is rejected. + context = new DefaultHttpContext(); + context.SetEndpoint(endpoint2); + await middleware.Invoke(context).DefaultTimeout(); + Assert.Equal(StatusCodes.Status503ServiceUnavailable, context.Response.StatusCode); + } + [Fact] public async Task MultipleEndpointPolicies_LastOneWins() { @@ -654,4 +709,51 @@ private RateLimitingMiddleware CreateTestRateLimitingMiddleware(IOptions CreateOptionsAccessor() => Options.Create(new RateLimiterOptions()); + + // An inline policy backed by a stateful limiter that permits a fixed number of acquisitions and then + // rejects. Used to observe whether two endpoints share or isolate their limiter buckets. + private sealed class CountingRateLimiterPolicy : IRateLimiterPolicy + { + private readonly string _key; + private readonly int _permitLimit; + + public CountingRateLimiterPolicy(string key, int permitLimit) + { + _key = key; + _permitLimit = permitLimit; + } + + public Func OnRejected => null; + + public RateLimitPartition GetPartition(HttpContext httpContext) + => RateLimitPartition.Get(_key, _ => new CountingRateLimiter(_permitLimit)); + } + + private sealed class CountingRateLimiter : RateLimiter + { + private int _remaining; + + public CountingRateLimiter(int permitLimit) => _remaining = permitLimit; + + public override TimeSpan? IdleDuration => null; + + public override RateLimiterStatistics GetStatistics() => null; + + protected override RateLimitLease AttemptAcquireCore(int permitCount) + { + if (_remaining >= permitCount) + { + _remaining -= permitCount; + return new TestRateLimitLease(isAcquired: true, null); + } + + return new TestRateLimitLease(isAcquired: false, null); + } + + protected override ValueTask AcquireAsyncCore(int permitCount, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + return new ValueTask(AttemptAcquireCore(permitCount)); + } + } }