From 3463539f26710bd2a23e8ebd490c47d24a2a8fce Mon Sep 17 00:00:00 2001 From: Karim Salem Date: Tue, 25 Aug 2026 01:44:33 -0700 Subject: [PATCH] Don't cache truncated or aborted responses in OutputCacheMiddleware (#68683) OutputCacheMiddleware could store a response whose body was cut short, and share that entry with requests waiting on the same cache key. FinalizeCacheBodyAsync now skips storage when the request was aborted and reports whether the response was cached. The caller releases the pending entry unless it was actually stored, so waiters re-execute instead of receiving a truncated body. Fixes #66877 --- .../src/OutputCacheMiddleware.cs | 22 ++- .../test/OutputCacheMiddlewareTests.cs | 173 ++++++++++++++++++ 2 files changed, 188 insertions(+), 7 deletions(-) diff --git a/src/Middleware/OutputCaching/src/OutputCacheMiddleware.cs b/src/Middleware/OutputCaching/src/OutputCacheMiddleware.cs index 695250703208..01f9c3d01edd 100644 --- a/src/Middleware/OutputCaching/src/OutputCacheMiddleware.cs +++ b/src/Middleware/OutputCaching/src/OutputCacheMiddleware.cs @@ -167,6 +167,8 @@ private async Task InvokeAwaited(HttpContext httpContext, IReadOnlyList - /// Stores the response body + /// Attempts to store the response body. /// - internal async ValueTask FinalizeCacheBodyAsync(OutputCacheContext context) + /// The . + /// true if the response was cached; otherwise false. + internal async ValueTask FinalizeCacheBodyAsync(OutputCacheContext context) { if (context.AllowCacheStorage && context.OutputCacheStream.BufferingEnabled - && context.CachedResponse is not null) + && context.CachedResponse is not null + && !context.HttpContext.RequestAborted.IsCancellationRequested) { // If AllowCacheLookup is false, the cache key was not created CreateCacheKey(context); @@ -442,6 +446,8 @@ internal async ValueTask FinalizeCacheBodyAsync(OutputCacheContext context) await OutputCacheEntryFormatter.StoreAsync(context.CacheKey, context.CachedResponse, context.Tags, context.CachedResponseValidFor, _store, _logger, context.HttpContext.RequestAborted); + + return true; } } else @@ -453,6 +459,8 @@ await OutputCacheEntryFormatter.StoreAsync(context.CacheKey, context.CachedRespo { _logger.ResponseNotCached(); } + + return false; } /// diff --git a/src/Middleware/OutputCaching/test/OutputCacheMiddlewareTests.cs b/src/Middleware/OutputCaching/test/OutputCacheMiddlewareTests.cs index 32c434fb46ab..90b3ed1833d9 100644 --- a/src/Middleware/OutputCaching/test/OutputCacheMiddlewareTests.cs +++ b/src/Middleware/OutputCaching/test/OutputCacheMiddlewareTests.cs @@ -850,6 +850,33 @@ public async Task FinalizeCacheBody_DoNotCache_IfBufferingDisabled() LoggedMessage.ResponseNotCached); } + [Fact] + public async Task FinalizeCacheBody_DoNotCache_IfRequestAborted() + { + var cache = GetStore(); + var sink = new TestSink(); + var middleware = TestUtils.CreateTestMiddleware(testSink: sink, cache: cache); + var context = TestUtils.CreateTestContext(cache: cache); + + middleware.ShimResponseStream(context); + await context.HttpContext.Response.WriteAsync(new string('0', 10)); + + using var entry = new OutputCacheEntry(DateTimeOffset.UtcNow, StatusCodes.Status200OK); + context.CachedResponse = entry; + context.CacheKey = "BaseKey"; + context.CachedResponseValidFor = TimeSpan.FromSeconds(10); + + context.HttpContext.RequestAborted = new CancellationToken(canceled: true); + + var isResponseCached = await middleware.FinalizeCacheBodyAsync(context); + + Assert.Equal(0, cache.SetCount); + Assert.False(isResponseCached); + TestUtils.AssertLoggedMessages( + sink.Writes, + LoggedMessage.ResponseNotCached); + } + [Fact] public async Task FinalizeCacheBody_DoNotCache_IfSizeTooBig() { @@ -1008,6 +1035,69 @@ public async Task Locking_IgnoresNonCacheableResponses() Assert.Equal("Hello2", Encoding.UTF8.GetString(memoryStream2.ToArray())); } + [Fact] + public async Task Locking_IgnoresTruncatedResponses() + { + var responseCounter = 0; + var cache = GetStore(); + + var blocker1 = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var blocker2 = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var memoryStream1 = new MemoryStream(); + var memoryStream2 = new MemoryStream(); + + var options = new OutputCacheOptions(); + options.AddBasePolicy(build => build.Cache()); + + var middleware = TestUtils.CreateTestMiddleware(options: options, cache: cache, next: async c => + { + responseCounter++; + + if (responseCounter == 1) + { + blocker1.SetResult(true); + + // Announce more bytes than are written so the response is truncated + c.Response.ContentLength = 1000; + } + + c.Response.Write("Hello" + responseCounter); + + await blocker2.Task; + }); + + var context1 = TestUtils.CreateTestContext(cache: cache); + context1.HttpContext.Request.Method = "GET"; + context1.HttpContext.Request.Path = "/"; + context1.HttpContext.Response.Body = memoryStream1; + + var context2 = TestUtils.CreateTestContext(cache: cache); + context2.HttpContext.Request.Method = "GET"; + context2.HttpContext.Request.Path = "/"; + context2.HttpContext.Response.Body = memoryStream2; + + var task1 = Task.Run(() => middleware.Invoke(context1.HttpContext)); + + // Wait for context1 to be processed + await blocker1.Task; + + // Start context2 and let it run until it is blocked by the locking feature + var task2 = middleware.Invoke(context2.HttpContext); + Assert.False(task2.IsCompleted); + + // Unblock context1 + blocker2.SetResult(true); + + await Task.WhenAll(task1, task2); + + Assert.Equal(2, responseCounter); + + // Ensure the truncated response was not returned from cache + Assert.Equal("Hello1", Encoding.UTF8.GetString(memoryStream1.ToArray())); + Assert.Equal("Hello2", Encoding.UTF8.GetString(memoryStream2.ToArray())); + } + [Fact] public async Task Locking_ExecuteAllRequestsWhenDisabled() { @@ -1055,6 +1145,89 @@ public async Task Locking_ExecuteAllRequestsWhenDisabled() Assert.Equal(2, responseCounter); } + [Fact] + public async Task AbortedRequest_DoesNotReExecuteRequest() + { + var responseCounter = 0; + var cache = GetStore(); + + var options = new OutputCacheOptions(); + options.AddBasePolicy(build => build.Cache()); + + var middleware = TestUtils.CreateTestMiddleware(options: options, cache: cache, next: c => + { + responseCounter++; + + c.Response.Write("Hello" + responseCounter); + + // Simulates an action result that aborts after a canceled write + c.RequestAborted = new CancellationToken(canceled: true); + + return Task.CompletedTask; + }); + + var memoryStream = new MemoryStream(); + + var context = TestUtils.CreateTestContext(cache: cache); + context.HttpContext.Request.Method = "GET"; + context.HttpContext.Request.Path = "/"; + context.HttpContext.Response.Body = memoryStream; + + await middleware.Invoke(context.HttpContext); + + // The aborted request must not run the pipeline a second time + Assert.Equal(1, responseCounter); + Assert.Equal(0, cache.SetCount); + Assert.Equal("Hello1", Encoding.UTF8.GetString(memoryStream.ToArray())); + } + + [Fact] + public async Task AbortedRequest_IsNotServedToSubsequentRequests() + { + var responseCounter = 0; + var cache = GetStore(); + + var options = new OutputCacheOptions(); + options.AddBasePolicy(build => build.Cache()); + + var middleware = TestUtils.CreateTestMiddleware(options: options, cache: cache, next: c => + { + responseCounter++; + + c.Response.Write("Hello" + responseCounter); + + if (responseCounter == 1) + { + c.RequestAborted = new CancellationToken(canceled: true); + } + + return Task.CompletedTask; + }); + + var memoryStream1 = new MemoryStream(); + + var context1 = TestUtils.CreateTestContext(cache: cache); + context1.HttpContext.Request.Method = "GET"; + context1.HttpContext.Request.Path = "/"; + context1.HttpContext.Response.Body = memoryStream1; + + await middleware.Invoke(context1.HttpContext); + + var memoryStream2 = new MemoryStream(); + + var context2 = TestUtils.CreateTestContext(cache: cache); + context2.HttpContext.Request.Method = "GET"; + context2.HttpContext.Request.Path = "/"; + context2.HttpContext.Response.Body = memoryStream2; + + await middleware.Invoke(context2.HttpContext); + + // The later request runs its own delegate, not the aborted response + Assert.Equal(2, responseCounter); + Assert.Equal("Hello1", Encoding.UTF8.GetString(memoryStream1.ToArray())); + Assert.Equal("Hello2", Encoding.UTF8.GetString(memoryStream2.ToArray())); + } + [Fact] public async Task EmptyCacheKey_IsNotCached() {