Skip to content

Commit 5066e53

Browse files
authored
fix(templates): correct response caching of origin-dependent and faulted responses bitfoundation#12733 (bitfoundation#12734)
1 parent 6417922 commit 5066e53

14 files changed

Lines changed: 523 additions & 70 deletions

File tree

src/Templates/Boilerplate/Bit.Boilerplate/.docs/14- Response Caching System.md

Lines changed: 118 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -150,18 +150,29 @@ Output cache still works correctly for multi-language scenarios.
150150
public async ValueTask CacheRequestAsync(OutputCacheContext context, CancellationToken cancellation)
151151
{
152152
var responseCacheAtt = context.HttpContext.GetResponseCacheAttribute();
153-
153+
154154
if (responseCacheAtt is null) return;
155155

156-
// Default: SharedMaxAge = MaxAge if not specified
157-
if (responseCacheAtt.SharedMaxAge == -1)
158-
{
159-
responseCacheAtt.SharedMaxAge = responseCacheAtt.MaxAge;
160-
}
156+
context.AllowLocking = true;
157+
context.EnableOutputCaching = true;
158+
159+
// What the output cache keys on, besides the request path:
160+
context.CacheVaryByRules.QueryKeys = "*";
161+
context.CacheVaryByRules.VaryByHost = true;
162+
context.CacheVaryByRules.HeaderNames = new[] { HeaderNames.Origin, "X-Origin" };
163+
context.CacheVaryByRules.VaryByValues.Add("Culture", CultureInfo.CurrentUICulture.Name);
164+
165+
// Multi-tenant: an authenticated request resolves its tenant from the user's claim rather than from the
166+
// host, and tenant scoped entities are filtered by it, so two tenants on one host must not share an entry.
167+
if (context.HttpContext.User.GetTenantId() is Guid currentTenantId)
168+
context.CacheVaryByRules.VaryByValues.Add("Tenant", currentTenantId.ToString());
161169

162-
var clientCacheTtl = responseCacheAtt.MaxAge; // In-memory + Browser
163-
var edgeCacheTtl = responseCacheAtt.SharedMaxAge; // CDN Edge
164-
var outputCacheTtl = responseCacheAtt.SharedMaxAge; // ASP.NET Core Output Cache
170+
// SharedMaxAge falls back to MaxAge when it isn't set
171+
var sharedMaxAge = responseCacheAtt.SharedMaxAge == -1 ? responseCacheAtt.MaxAge : responseCacheAtt.SharedMaxAge;
172+
173+
var clientCacheTtl = responseCacheAtt.MaxAge; // In-memory + Browser
174+
var edgeCacheTtl = sharedMaxAge; // CDN Edge
175+
var outputCacheTtl = sharedMaxAge; // ASP.NET Core Output Cache
165176
166177
// Disable CDN edge if configured
167178
if (settings.ResponseCaching?.EnableCdnEdgeCaching is false)
@@ -182,14 +193,49 @@ public async ValueTask CacheRequestAsync(OutputCacheContext context, Cancellatio
182193
outputCacheTtl = -1;
183194
}
184195

196+
// The entry is tagged with the request path only, without the query string: purging is done by bare path
197+
// ("/product/5"), while QueryKeys = "*" gives every query string variant its own entry. Tagging those with
198+
// their full PathAndQuery would leave "/product/5?utm_source=x" unpurgeable for the rest of its lifetime.
199+
context.Tags.Add(new Uri(context.HttpContext.Request.GetUri().GetUrlWithoutCulture()).AbsolutePath.ToLowerInvariant());
200+
185201
// ... set cache headers and output cache policy
186202
}
187203
```
188204

205+
**Responses that are never stored:** a response is kept out of every cache unless it is a `200 OK` that hands out no
206+
cookies (the culture cookie is exempt, since the cache varies by culture anyway). That keeps a 404 for a product created
207+
a minute later from surviving on the edge for days, and keeps one caller's cookies from being replayed to everybody else.
208+
This is enforced twice - by an `OnStarting` callback that downgrades `Cache-Control` to `no-store, private` for browsers
209+
and CDNs, and by clearing `AllowCacheStorage` in `ServeResponseAsync` for the output cache.
210+
211+
**Telling shared caches what to vary on:**
212+
213+
The output cache keys on `Origin` and `X-Origin`, so the response advertises them too:
214+
215+
```
216+
Vary: Origin, X-Origin
217+
```
218+
219+
- `Origin` - the CORS middleware runs before the output cache middleware and echoes the caller's origin into
220+
`Access-Control-Allow-Origin`. Without the vary, the first caller's value would be replayed to every other origin and
221+
their browsers would reject it.
222+
- `X-Origin` - the header a Blazor Hybrid / standalone WASM client sends to tell the backend which web app url it is
223+
running under (See `HttpRequestExtensions.GetWebAppUrl`), which can end up embedded in the response.
224+
225+
> **A CDN may ignore `Vary`.** Cloudflare does not consider it in caching decisions unless the header is
226+
> `Accept-Encoding`, or a **Cache Rules → Vary** setting naming `origin` / `x-origin` has been configured on the zone.
227+
> Without that rule the edge keeps a single variant per URL and hands it to callers of every origin. Configure it before
228+
> turning `EnableCdnEdgeCaching` on.
229+
189230
**Important Security Note:**
190231

191232
The `UserAgnostic` property is critical for security. If a response contains user-specific data (e.g., user's name, roles, or tenant information), it **must not** be cached in shared caches (CDN edge or output cache). Setting `UserAgnostic = true` is only safe when the response is identical for all users.
192233

234+
> **Multi-tenant + CDN edge:** the `Tenant` discriminator above is part of the **ASP.NET Core output cache key only** -
235+
> `VaryByValues` never becomes a response header, so a CDN cannot see it. The output cache therefore keeps tenants apart
236+
> correctly, but an edge cache keyed on host + path does not. Until the tenant is part of the URL or the host, treat
237+
> `UserAgnostic = true` together with `EnableCdnEdgeCaching` as unsafe for any response whose body is tenant-filtered.
238+
193239
---
194240

195241
### 3. ResponseCacheService
@@ -215,10 +261,10 @@ public partial class ResponseCacheService
215261
/// </summary>
216262
public async Task PurgeCache(params string[] relativePaths)
217263
{
218-
// Purge from ASP.NET Core output cache
264+
// Purge from ASP.NET Core output cache. Lowercased to match the tag the policy writes.
219265
foreach (var relativePath in relativePaths)
220266
{
221-
await outputCacheStore.EvictByTagAsync(relativePath, default);
267+
await outputCacheStore.EvictByTagAsync(relativePath.ToLowerInvariant(), default);
222268
}
223269

224270
// Purge from Cloudflare CDN
@@ -231,9 +277,9 @@ public partial class ResponseCacheService
231277
public async Task PurgeProductCache(int shortId)
232278
{
233279
await PurgeCache(
234-
"/", // Home page (may list products)
235-
$"/product/{shortId}", // Product detail page
236-
$"/api/ProductView/Get/{shortId}" // Product API endpoint
280+
"/", // Home page (may list products)
281+
$"/product/{shortId}", // Product detail page
282+
$"/api/v1/ProductView/Get/{shortId}" // Product API endpoint
237283
);
238284
}
239285
}
@@ -282,7 +328,7 @@ public string? GetPrimaryMediumImageUrl(Uri absoluteServerAddress)
282328
return HasPrimaryImage is false
283329
? null
284330
: new Uri(absoluteServerAddress,
285-
$"/api/Attachment/GetAttachment/{Id}/{AttachmentKind.ProductPrimaryImageMedium}?v={Version}")
331+
$"/api/v1/Attachment/GetAttachment/{Id}/{AttachmentKind.ProductPrimaryImageMedium}?v={Version}")
286332
.ToString();
287333
}
288334
```
@@ -477,10 +523,44 @@ This prevents accidentally serving User A's data to User B through shared caches
477523
"https://sales.bitplatform.com",
478524
"https://sales.bitplatform.uk"
479525
]
526+
},
527+
// Shared/appsettings.json - shared by the server AND every client (WASM, MAUI, Windows)
528+
"MemoryCache": {
529+
"SizeLimit": 268435456 // 256 MB, in bytes
480530
}
481531
}
482532
```
483533

534+
Both `ResponseCaching` flags default to `false`. Turn `EnableCdnEdgeCaching` on only after reading the `Vary` and
535+
multi-tenant notes above.
536+
537+
---
538+
539+
## The L1 Memory Budget
540+
541+
Layers 1 and 4 (Client In-Memory Cache and Output Cache) both live in the app's single `IMemoryCache`, which is bounded
542+
by `MemoryCache:SizeLimit` in `Shared/appsettings.json` and implemented by `AppMemoryCache`.
543+
544+
**The unit is bytes, not entries.** That matters, because the three kinds of entry are charged differently:
545+
546+
| Entry | Charged | Set by |
547+
|---|---|---|
548+
| Output cache response body | its exact length | `FusionOutputCacheStore` (`AddFusionOutputCache`) |
549+
| Client in-memory cached response | its exact length | `CacheDelegatingHandler` |
550+
| Everything else (FusionCache data entries, 3rd party libraries) | `AppMemoryCache.EstimatedEntrySizeInBytes` (4 KB) | `WithDefaultEntryOptions` / `AppMemoryCache.CreateEntry` |
551+
552+
The flat 4 KB estimate is deliberately generous: charging an entry too much only means fewer of them fit, while charging
553+
too little lets the cache outgrow the limit it exists to enforce. At 256 MB the budget holds roughly 65k estimated
554+
entries, minus whatever the cached response bodies take.
555+
556+
**Why not count entries instead?** Because the output cache stores whole response bodies here. A single pre-rendered
557+
page or attachment would otherwise cost the same one unit as a small dictionary, letting one big response quietly
558+
consume a budget sized for tens of thousands of small ones - or, worse, be silently rejected once the limit was reached,
559+
turning output caching into a no-op with no error anywhere.
560+
561+
If you raise `SizeLimit`, remember the same value ships to the clients: it also bounds the Blazor Hybrid / WASM app's
562+
in-process cache on a phone, not just the server's.
563+
484564
---
485565

486566
## FusionCache Library
@@ -527,13 +607,36 @@ This shows the TTL (in seconds) for each cache layer. Use browser DevTools Netwo
527607

528608
```
529609
Cache-Control: public, max-age=300, s-maxage=3600
610+
Vary: Origin, X-Origin
530611
App-Cache-Response: Output:3600,Edge:3600,Client:300
531612
```
532613

533614
Interpretation:
534615
- `max-age=300`: Browser and in-memory cache for 5 minutes
535616
- `s-maxage=3600`: CDN edge and output cache for 1 hour
536617
- `public`: Can be cached in shared caches (CDN)
618+
- `Vary`: the request headers a shared cache must include in its key
619+
- `Output:-1` (or `Edge:-1` / `Client:-1`) means that layer was disabled for this request - by configuration, by the
620+
caller being authenticated on a non-`UserAgnostic` endpoint, or by the request being a pre-rendered Blazor page
621+
622+
A response that turns out not to be cacheable (anything other than `200 OK`, or one that sets a cookie) is downgraded to
623+
`Cache-Control: no-store, private` on its way out, regardless of what `App-Cache-Response` announced earlier in the
624+
request.
625+
626+
---
627+
628+
### Automated Test
629+
630+
`src/Tests/Features/Caching/ProductResponseCacheTests.cs` exercises the whole loop end to end with
631+
`EnableOutputCaching` and `PrerenderEnabled` both on: it fills the output cache from two directions (the tenant-user
632+
calling the `UserAgnostic` product API, and an anonymous visitor loading the pre-rendered product page), has the
633+
tenant-admin edit the product through `ProductController.Update` and asserts both readers see the change immediately,
634+
then deletes the row straight from the database and asserts both readers keep being served the deleted product - the
635+
proof that the responses are coming from the cache and not the database.
636+
637+
Note it needs `ProductController` (Admin module) and `ProductViewController` (Sales module) at the same time, and
638+
`module` is a single-choice template parameter, so it is excluded from generated projects and only runs against the
639+
template's own source tree.
537640

538641
---
539642

src/Templates/Boilerplate/Bit.Boilerplate/.template.config/template.json

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,11 @@
449449
"**/.git/**",
450450
"**/*.nuspec",
451451
"src/Server/**/Data/Migrations/**",
452-
"src/**/App_Data/**"
452+
"src/**/App_Data/**",
453+
// Needs ProductController (Admin module) and ProductViewController + the product page (Sales
454+
// module) at once, and "module" is a single choice, so it can only ever run against the
455+
// template's own source tree.
456+
"src/Tests/Features/Caching/ProductResponseCacheTests.cs"
453457
]
454458
},
455459
{
@@ -547,7 +551,8 @@
547551
"src/Tests/Features/Identity/PhoneNumberNormalizationUITests.cs",
548552
"src/Tests/Features/Identity/WebAuthnPasswordlessUITests.cs",
549553
"src/Tests/Features/OpenApi/OpenApiScalarIntegrationTests.cs",
550-
"src/Tests/Features/ForceUpdate/ForceUpdateUITests.cs"
554+
"src/Tests/Features/ForceUpdate/ForceUpdateUITests.cs",
555+
"src/Tests/Features/Caching/ProductResponseCacheTests.cs"
551556
]
552557
},
553558
{
@@ -707,4 +712,4 @@
707712
]
708713
}
709714
]
710-
}
715+
}

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Routes.razor

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<AppRouter AppAssembly="@GetType().Assembly"
77
AdditionalAssemblies="@(AssemblyLoadContext.Default.Assemblies.Where(asm => asm.GetName().Name?.Contains("Boilerplate.Client") is true))">
88
@*#if (brouter == true)*@
9-
<ChildContent>
9+
<Routes>
1010
@*#if (module == "Admin")*@
1111
@* Keep-alive: pages that host a BitDataGrid whose add/edit lives on a separate page
1212
(e.g. ProductsPage -> AddOrEditProductPage) keep their grid state - current page,
@@ -16,7 +16,7 @@
1616
<Broute KeepAlive Path="@PageUrls.Products" Component="@typeof(Boilerplate.Client.Core.Components.Pages.Products.ProductsPage)" />
1717
<Broute KeepAlive Path="@("{culture?}" + PageUrls.Products)" Component="@typeof(Boilerplate.Client.Core.Components.Pages.Products.ProductsPage)" />
1818
@*#endif*@
19-
</ChildContent>
19+
</Routes>
2020
@*#endif*@
2121
<Found Context="routeData">
2222
<AppRouteDataPublisher RouteData="@routeData" />

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Infrastructure/Services/HttpMessageHandlers/CacheDelegatingHandler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
5454
LogScopeData = logScopeData.ToDictionary()
5555
}, options: new()
5656
{
57-
Size = 1,
57+
Size = responseContent.Length,
5858
AbsoluteExpirationRelativeToNow = maxAge
5959
});
6060
}

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Features/Identity/IdentityController.ExternalSignIn.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,12 @@ public partial class IdentityController
99
[AutoInject] private ApiServerExceptionHandler serverExceptionHandler = default!;
1010
[AutoInject] private IAuthenticationSchemeProvider authenticationSchemeProvider = default!;
1111

12+
/// <summary>
13+
/// Deliberately not cached: the returned URL embeds the caller's origin, which GetWebAppUrl resolves from the X-Origin
14+
/// header. That header is part of neither the output cache's vary rules nor a CDN's cache key, so a cached response would
15+
/// hand one origin's sign-in URL to a caller coming from another, sending the resulting sign-in link to the wrong origin.
16+
/// </summary>
1217
[HttpGet]
13-
[AppResponseCache(SharedMaxAge = 3600 * 24 * 7, MaxAge = 60 * 5)]
1418
public async Task<string> GetExternalSignInUri(string provider, string? returnUrl = null, int? localHttpPort = null, CancellationToken cancellationToken = default)
1519
{
1620
var uri = Url.Action(nameof(ExternalSignIn), new { provider, returnUrl, localHttpPort, origin = Request.GetWebAppUrl() })!;

0 commit comments

Comments
 (0)