Response Caching: include retained cache-key bytes in MemoryCacheEntryOptions.Size
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
Description
## Summary
The built-in Response Caching memory store omits the retained storage key from its entry-size estimate. `ResponseCachingOptions.SizeLimit` is expressed in bytes, but `MemoryResponseCache.Set` supplies only the value estimate for both cached responses and cached vary-by rules.
Include each entry's retained key content alongside its existing value estimate so that the configured byte-content budget more closely represents what the middleware retains.
## What is wrong
* Both `MemoryResponseCache.Set` branches pass a string key to the backing cache without adding its content to `MemoryCacheEntryOptions.Size`.
* `EstimateCachedResponseSize` counts response status, headers, and body. `EstimateCachedVaryByRulesySize` counts the rule prefix and rule names. Neither includes the independent storage key.
* Without vary rules, the response is stored under the base key. With vary rules, the rules are stored under the base key and the response under a separate prefix-based varied key. Each stored entry retains its own supplied key; counting rule content does not account for either separate storage key.
## Why it matters (defense in depth)
* The middleware defines its size unit as bytes, but omits a retained, variable-size part of an entry. The declared total can therefore understate the retained key-and-value content.
* Including keys strengthens the accounting used for size-based admission and eviction. It does not make `SizeLimit` a hard process-memory ceiling: the backing cache enforces caller-declared estimates, not measured object graphs.
* This is a byte-accounting correctness and hardening improvement. Object headers, dictionary structures, allocator overhead, and other runtime-dependent costs remain outside the existing content-estimation model.
## Affected code
The source references below identify the inspected implementation:
* [src/Middleware/ResponseCaching/src/MemoryResponseCache.cs:36-68](https://github.com/dotnet/aspnetcore/blob/87bea9349a35ec14195bd74cd35962fbe8dab9f2/src/Middleware/ResponseCaching/src/MemoryResponseCache.cs#L36-L68) - both value-only `Size` assignments and the separate key arguments.
* [src/Middleware/ResponseCaching/src/CacheEntry/CacheEntryHelpers.cs:10-84](https://github.com/dotnet/aspnetcore/blob/87bea9349a35ec14195bd74cd35962fbe8dab9f2/src/Middleware/ResponseCaching/src/CacheEntry/CacheEntryHelpers.cs#L10-L84) - existing response and vary-rule value estimates.
* [src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs:39-53](https://github.com/dotnet/aspnetcore/blob/87bea9349a35ec14195bd74cd35962fbe8dab9f2/src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs#L39-L53) - creates the dedicated cache with `ResponseCachingOptions.SizeLimit`.
* [src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs:364-401](https://github.com/dotnet/aspnetcore/blob/87bea9349a35ec14195bd74cd35962fbe8dab9f2/src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs#L364-L401) - independent rule and response writes.
* [src/Middleware/ResponseCaching/src/ResponseCachingOptions.cs:9-32](https://github.com/dotnet/aspnetcore/blob/87bea9349a35ec14195bd74cd35962fbe8dab9f2/src/Middleware/ResponseCaching/src/ResponseCachingOptions.cs#L9-L32) - byte-based size options and the separate response-body limit.
## Recommended fix
### Selected approach
Add the supplied key's UTF-16 character-content estimate at `MemoryResponseCache.Set`, where both the retained key and value are known. Keep the existing value helpers unchanged and update both `Size` assignments.
For the `CachedResponse` branch:
```csharp
Size = checked(CacheEntryHelpers.EstimateCachedResponseSize(cachedResponse) + (long)(key?.Length ?? 0) * sizeof(char))
```
For the existing vary-rule/fallback branch:
```csharp
Size = checked(CacheEntryHelpers.EstimateCachedVaryByRulesySize(entry as CachedVaryByRules) + (long)(key?.Length ?? 0) * sizeof(char))
```
Promote the key length to `long` before multiplication and keep the combined addition checked. Charge the actual supplied key once per stored entry, rather than reconstructing it from request components or substituting the vary-rule prefix.
Pass the original key unchanged to the backing cache. The null-safe length calculation preserves its existing null-key rejection path; it must not normalize a null key into an empty key. Keep the existing estimator evaluation, fallback cast, value contributions, and expiration assignments.
### Alternatives considered
* Changing both value-estimator signatures would couple value estimation to storage identity without a demonstrated reuse need. The storage boundary already has both inputs.
* Hashing, truncating, or re-encoding keys changes their representation or identity. A separate key-length cap changes eligibility but does not correct the aggregate estimate. Neither is required here.
* An entry-count cap or separate key-memory budget introduces another resource policy and additional admission, replacement, and expiration bookkeeping. Correct the existing byte estimate instead.
* Documentation alone can explain estimate semantics but leaves retained key content uncharged. Documentation should accompany the correction.
### Compatibility, migration, and versioning
* Preserve key generation, key identity, public APIs, value-estimator signatures, default budgets, and the separate response-body limit. No public API baseline update is required.
* Keep rule and response admission independent. A rejected rule does not make response storage transactional, and the writes need not share an absolute expiration deadline. No coupled eviction or cleanup redesign is part of this change.
* At the same `SizeLimit`, corrected accounting can admit fewer entries and reach capacity earlier. Document that capacity effect; do not raise the default limit to compensate.
* Clarify the `ResponseCachingOptions.SizeLimit` XML documentation to describe estimated response/rule sizes including their keys, not total process memory.
* No serialized key-format change or persistent-data migration is needed for this in-process store. Release targeting is a separate triage decision; no backport or supported-version matrix is assumed by this issue.
* Keep this change limited to Response Caching. It does not change Output Caching, CacheTagHelper, or other cache stores.
## Acceptance criteria
* [ ] Both response and vary-rule storage paths include the actual supplied key's UTF-16 content in addition to their existing value estimate.
* [ ] Fixed-value tests vary only key length and cover exact-limit admission and over-limit rejection independently for both entry types.
* [ ] Tests verify UTF-16 code-unit accounting, including BMP and surrogate-pair text, without UTF-8 conversion or key changes.
* [ ] Empty keys, empty rule values, and nonempty response/rule value contributions retain the expected accounting. Null keys retain the backing cache's rejection behavior, and the existing fallback value path is preserved.
* [ ] Aggregate mixed-entry capacity and equal-key replacement account for each logical entry correctly. Expiration behavior remains unchanged; tests do not depend on asynchronous compaction order or coupled rule/response expiry.
* [ ] Middleware tests using a real bounded cache verify successful response delivery when a non-varied response is rejected, when rules are rejected but the varied response is admitted, and when rules are admitted but the varied response is rejected. Admission remains independent.
* [ ] Key-length multiplication widens to `long` before arithmetic, and combined-estimate overflow is detected without requiring large allocations in tests.
* [ ] The existing Response Caching test suite and relevant build/analyzer checks pass. Test doubles that ignore `MemoryCacheEntryOptions.Size` are not used to establish capacity correctness.
* [ ] Public APIs, key identity, defaults, body limits, and expiration semantics remain unchanged apart from the intended size-based admission effect. XML documentation and release-note handling describe the estimate and capacity change.
## Related work
Related to #67911, which tracks the analogous omission in Output Caching. It targets a separate store and option type; fixing it does not fix Response Caching. This issue tracks the Response Caching correction independently.
Contributor guide
Research direction
Start in src/Middleware/ResponseCaching/src/MemoryResponseCache.cs and review the existing estimators in src/Middleware/ResponseCaching/src/CacheEntry/CacheEntryHelpers.cs, then inspect the Response Caching test suite. Verify both cache-entry paths account for the supplied key's UTF-16 length with checked arithmetic, add admission and mixed-entry tests, and update the ResponseCachingOptions XML documentation while preserving key identity, expiration, and independent writes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100