Merge in 'release/7.0-rc1' changes

Author: dotnet-bot

src/Middleware/RateLimiting/src/LeaseContext.cs

@@ -7,12 +7,19 @@ namespace Microsoft.AspNetCore.RateLimiting;
 
 internal struct LeaseContext : IDisposable
 {
-    public bool? GlobalRejected { get; init; }
+    public RequestRejectionReason? RequestRejectionReason { get; init; }
 
-    public required RateLimitLease Lease { get; init; }
+    public RateLimitLease? Lease { get; init; }
 
     public void Dispose()
     {
-        Lease.Dispose();
+        Lease?.Dispose();
     }
 }
+
+internal enum RequestRejectionReason
+{
+    EndpointLimiter,
+    GlobalLimiter,
+    RequestCanceled
+}

src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs

@@ -78,20 +78,25 @@ public Task Invoke(HttpContext context)
     private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute)
     {
         using var leaseContext = await TryAcquireAsync(context);
-        if (leaseContext.Lease.IsAcquired)
+        if (leaseContext.Lease?.IsAcquired == true)
         {
             await _next(context);
         }
         else
         {
+            // If the request was canceled, do not call OnRejected, just return.
+            if (leaseContext.RequestRejectionReason == RequestRejectionReason.RequestCanceled)
+            {
+                return;
+            }
             var thisRequestOnRejected = _defaultOnRejected;
             RateLimiterLog.RequestRejectedLimitsExceeded(_logger);
             // OnRejected "wins" over DefaultRejectionStatusCode - we set DefaultRejectionStatusCode first,
             // then call OnRejected in case it wants to do any further modification of the status code.
             context.Response.StatusCode = _rejectionStatusCode;
 
             // If this request was rejected by the endpoint limiter, use its OnRejected if available.
-            if (leaseContext.GlobalRejected == false)
+            if (leaseContext.RequestRejectionReason == RequestRejectionReason.EndpointLimiter)
             {
                 DefaultRateLimiterPolicy? policy;
                 // Use custom policy OnRejected if available, else use OnRejected from the Options if available.
@@ -111,15 +116,16 @@ private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribu
             }
             if (thisRequestOnRejected is not null)
             {
-                await thisRequestOnRejected(new OnRejectedContext() { HttpContext = context, Lease = leaseContext.Lease }, context.RequestAborted);
+                // leaseContext.Lease will only be null when the request was canceled.
+                await thisRequestOnRejected(new OnRejectedContext() { HttpContext = context, Lease = leaseContext.Lease! }, context.RequestAborted);
             }
         }
     }
 
     private ValueTask<LeaseContext> TryAcquireAsync(HttpContext context)
     {
         var leaseContext = CombinedAcquire(context);
-        if (leaseContext.Lease.IsAcquired)
+        if (leaseContext.Lease?.IsAcquired == true)
         {
             return ValueTask.FromResult(leaseContext);
         }
@@ -139,14 +145,14 @@ private LeaseContext CombinedAcquire(HttpContext context)
                 globalLease = _globalLimiter.AttemptAcquire(context);
                 if (!globalLease.IsAcquired)
                 {
-                    return new LeaseContext() { GlobalRejected = true, Lease = globalLease };
+                    return new LeaseContext() { RequestRejectionReason = RequestRejectionReason.GlobalLimiter, Lease = globalLease };
                 }
             }
             endpointLease = _endpointLimiter.AttemptAcquire(context);
             if (!endpointLease.IsAcquired)
             {
                 globalLease?.Dispose();
-                return new LeaseContext() { GlobalRejected = false, Lease = endpointLease };
+                return new LeaseContext() { RequestRejectionReason = RequestRejectionReason.EndpointLimiter, Lease = endpointLease };
             }
         }
         catch (Exception)
@@ -170,21 +176,30 @@ private async ValueTask<LeaseContext> CombinedWaitAsync(HttpContext context, Can
                 globalLease = await _globalLimiter.AcquireAsync(context, cancellationToken: cancellationToken);
                 if (!globalLease.IsAcquired)
                 {
-                    return new LeaseContext() { GlobalRejected = true, Lease = globalLease };
+                    return new LeaseContext() { RequestRejectionReason = RequestRejectionReason.GlobalLimiter, Lease = globalLease };
                 }
             }
             endpointLease = await _endpointLimiter.AcquireAsync(context, cancellationToken: cancellationToken);
             if (!endpointLease.IsAcquired)
             {
                 globalLease?.Dispose();
-                return new LeaseContext() { GlobalRejected = false, Lease = endpointLease };
+                return new LeaseContext() { RequestRejectionReason = RequestRejectionReason.EndpointLimiter, Lease = endpointLease };
             }
         }
-        catch (Exception)
+        catch (Exception ex)
         {
             endpointLease?.Dispose();
             globalLease?.Dispose();
-            throw;
+            // Don't throw if the request was canceled - instead log. 
+            if (ex is OperationCanceledException && context.RequestAborted.IsCancellationRequested)
+            {
+                RateLimiterLog.RequestCanceled(_logger);
+                return new LeaseContext() { RequestRejectionReason = RequestRejectionReason.RequestCanceled };
+            }
+            else
+            {
+                throw;
+            }
         }
 
         return globalLease is null ? new LeaseContext() { Lease = endpointLease } : new LeaseContext() { Lease = new DefaultCombinedLease(globalLease, endpointLease) };
@@ -234,5 +249,8 @@ private static partial class RateLimiterLog
 
         [LoggerMessage(2, LogLevel.Debug, "This endpoint requires a rate limiting policy with name {PolicyName}, but no such policy exists.", EventName = "WarnMissingPolicy")]
         internal static partial void WarnMissingPolicy(ILogger logger, string policyName);
+
+        [LoggerMessage(3, LogLevel.Debug, "The request was canceled.", EventName = "RequestCanceled")]
+        internal static partial void RequestCanceled(ILogger logger);
     }
-}
+}

src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs

@@ -6,12 +6,13 @@
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Logging.Testing;
 using Microsoft.Extensions.Options;
 using Moq;
 
 namespace Microsoft.AspNetCore.RateLimiting;
 
-public class RateLimitingMiddlewareTests : LoggedTest
+public class RateLimitingMiddlewareTests
 {
     [Fact]
     public void Ctor_ThrowsExceptionsWhenNullArgs()
@@ -115,22 +116,35 @@ public async Task RequestRejected_WinsOverDefaultStatusCode()
     }
 
     [Fact]
-    public async Task RequestAborted_ThrowsTaskCanceledException()
+    public async Task RequestAborted_DoesNotThrowTaskCanceledException()
     {
+        var sink = new TestSink(
+            TestSink.EnableWithTypeName<RateLimitingMiddleware>,
+            TestSink.EnableWithTypeName<RateLimitingMiddleware>);
+        var loggerFactory = new TestLoggerFactory(sink, enabled: true);
+
         var options = CreateOptionsAccessor();
         options.Value.GlobalLimiter = new TestPartitionedRateLimiter<HttpContext>(new TestRateLimiter(false));
 
         var middleware = new RateLimitingMiddleware(c =>
             {
                 return Task.CompletedTask;
             },
-            new NullLoggerFactory().CreateLogger<RateLimitingMiddleware>(),
+            loggerFactory.CreateLogger<RateLimitingMiddleware>(),
             options,
             Mock.Of<IServiceProvider>());
 
         var context = new DefaultHttpContext();
         context.RequestAborted = new CancellationToken(true);
-        await Assert.ThrowsAsync<TaskCanceledException>(() => middleware.Invoke(context)).DefaultTimeout();
+        await middleware.Invoke(context);
+        Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode);
+
+        var logMessages = sink.Writes.ToList();
+
+        Assert.Single(logMessages);
+        var message = logMessages.First();
+        Assert.Equal(LogLevel.Debug, message.LogLevel);
+        Assert.Equal("The request was canceled.", message.State.ToString());
     }
 
     [Fact]
@@ -609,4 +623,4 @@ public async Task MultipleEndpointPolicies_LastOneWins()
 
     private IOptions<RateLimiterOptions> CreateOptionsAccessor() => Options.Create(new RateLimiterOptions());
 
-}
+}