dotnet / dotnet/aspnetcore

Response Caching: include retained cache-key bytes in MemoryCacheEntryOptions.Size

Open
#69,214 2 comments 0 reactions 0 assignees View on GitHub
area-middleware feature-response-caching
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.