Don't cache truncated/aborted responses in OutputCacheMiddleware - #68683
Open
karimsalem1 wants to merge 1 commit into
Open
Don't cache truncated/aborted responses in OutputCacheMiddleware#68683karimsalem1 wants to merge 1 commit into
karimsalem1 wants to merge 1 commit into
Conversation
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
dotnet-policy-service
Bot
added
the
community-contribution
Contributor
|
Thanks for your PR, @karimsalem1. Someone from the team will get assigned to your PR shortly and we'll get it 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
2 participants
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.
Don't cache truncated/aborted responses in OutputCacheMiddleware
Fixes #66877
Fixes #56427
Issue
When a request is aborted,
OutputCacheMiddlewarecan still store the partial response and serve it to subsequent clients until the entry expires.Two safeguards are meant to prevent this, and neither is reliable:
Content-Length. With noContent-Lengthset there is nothing to compare against, and the check passes.OutputCacheStreamdisables buffering on a write failure, but only sees failures raised at its own layer. When another stream is layered above it, the exception is thrown before the write ever reachesOutputCacheStream.Locking makes this worse. With
SetLocking(true), requests waiting on an in-flight response are handed it directly, without the cacheability checks running at all. On that path, even a response carrying aContent-Lengthcan be shared truncated.Fix
httpContext.RequestAbortedis canceled.FinalizeCacheBodyAsyncnow reports whether the response was stored, and the caller releases the pending entry if it wasn't, forcing waiters to execute the pipeline themselves instead of receiving the incomplete entry.The release condition is just
!isResponseCached. The previous!context.AllowCacheStoragecheck is now redundant, sinceFinalizeCacheBodyAsyncwill returnfalsewhen storage is disabled.Tests
Validates scenarios: an aborted response isn't stored, isn't served to later requests, and isn't handed to a request waiting on the locking path. Also ensures the aborted request itself still runs the pipeline exactly once.