[release/10.0] Don't cache truncated/aborted responses in OutputCacheMiddleware - #68770
Open
DeagleGross wants to merge 1 commit into
Open
Conversation
…otnet#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 dotnet#66877
Contributor
There was a problem hiding this comment.
Pull request overview
Backports a fix to OutputCacheMiddleware (release/10.0) to prevent truncated or aborted responses from being cached or shared with concurrent waiters for the same output-cache key, ensuring waiters re-execute when the in-flight response wasn’t actually cached.
Changes:
- Updates the locking/pending-entry flow to only reuse an in-flight response when
FinalizeCacheBodyAsyncreports it was cached. - Extends
FinalizeCacheBodyAsyncto return a boolean and to skip caching whenRequestAbortedis already canceled. - Adds/extends tests to cover aborted and truncated responses, including locking scenarios.
Show a summary per file
| File | Description |
|---|---|
| src/Middleware/OutputCaching/src/OutputCacheMiddleware.cs | Tracks whether a response was cached and releases pending entries when it wasn’t, plus skips caching on aborted requests. |
| src/Middleware/OutputCaching/test/OutputCacheMiddlewareTests.cs | Adds regression tests for aborted/truncated responses and verifies locking waiters re-execute instead of receiving incomplete responses. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
| await OutputCacheEntryFormatter.StoreAsync(context.CacheKey, context.CachedResponse, context.Tags, context.CachedResponseValidFor, | ||
| _store, _logger, context.HttpContext.RequestAborted); | ||
|
|
||
| return true; |
Contributor
|
Hi @DeagleGross. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport of #68683 to release/10.0. Source merge commit: bbc2649.
Prevents truncated or aborted responses from being stored or shared with requests waiting on the same output-cache key. Waiters now re-execute when the in-flight response was not actually cached.
Validation: 108 OutputCacheMiddlewareTests passed on net10.0.