Skip to content

Commit cc3b0c2

Browse files
authored
feat(ads-client): respect server max-age in cache TTL resolution (#7344)
* feat(ads-client): respect server max-age in cache TTL resolution Resolve effective TTL by priority via a new EffectiveTtl helper — explicit per-request TTL wins, otherwise the response's Cache-Control: max-age is used, otherwise the configured default_ttl. Previously the layer took the min of all three, which collapsed to the short default_ttl in practice and ignored the server's max-age signal. AC-103 * feat(ads-client): cap effective cache TTL at 7 days Apply a 7-day ceiling to the resolved effective TTL regardless of source. Without it, a misconfigured server max-age (e.g. years) or an oversized caller/default value would persist responses far longer than is reasonable for this cache. AC-103
1 parent ecd8f5c commit cc3b0c2

8 files changed

Lines changed: 215 additions & 50 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313

1414
## ✨ What's New ✨
1515

16+
### Ads Client
17+
* HTTP cache TTL is now resolved by priority — explicit per-request TTL (if any) wins, otherwise the response's `Cache-Control: max-age` is used, otherwise the configured `default_ttl`. The resolved TTL is capped at 7 days as a safety net against misconfigured server values. Previously the layer took the minimum of all three, which effectively ignored the server's `max-age` signal.
18+
1619
### Remote Settings
1720
* Add uptake telemetry support ([#7288](https://github.com/mozilla/application-services/pull/7288))
1821
* Add v2 routes ([#7339](https://github.com/mozilla/application-services/pull/7339))

components/ads-client/docs/usage-javascript.md

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -544,19 +544,16 @@ It reduces redundant network traffic and improves latency for repeated or identi
544544

545545
### Cache Lifecycle
546546

547-
Each network response can be stored in the cache with an associated effective TTL, computed as:
547+
Each network response can be stored in the cache with an associated effective TTL,
548+
resolved by priority (highest to lowest):
548549

549-
```
550-
effective_ttl = min(server_max_age, client_default_ttl, per_request_ttl)
551-
```
552-
553-
where:
554-
555-
- `server_max_age` comes from the HTTP `Cache-Control: max-age=N` header (if present),
556-
- `client_default_ttl` is set in `MozAdsCacheConfig`,
557-
- `per_request_ttl` is an optional override set in `MozAdsRequestCachePolicy`.
550+
1. `per_request_ttl` — caller-provided override on `MozAdsRequestCachePolicy`.
551+
2. `server_max_age` — value of the HTTP `Cache-Control: max-age=N` header on the response.
552+
3. `client_default_ttl` — configured on `MozAdsCacheConfig`.
558553

559-
If the effective TTL resolves to 0 seconds, the response is not cached.
554+
If the effective TTL resolves to 0 seconds (e.g. `Cache-Control: max-age=0`),
555+
the response is not cached. The resolved TTL is capped at 7 days regardless
556+
of source.
560557

561558
### Configuring The Cache
562559

components/ads-client/docs/usage-kotlin.md

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -501,19 +501,16 @@ It reduces redundant network traffic and improves latency for repeated or identi
501501

502502
### Cache Lifecycle
503503

504-
Each network response can be stored in the cache with an associated effective TTL, computed as:
504+
Each network response can be stored in the cache with an associated effective TTL,
505+
resolved by priority (highest to lowest):
505506

506-
```
507-
effective_ttl = min(server_max_age, client_default_ttl, per_request_ttl)
508-
```
509-
510-
where:
511-
512-
- `server_max_age` comes from the HTTP `Cache-Control: max-age=N` header (if present),
513-
- `client_default_ttl` is set in `MozAdsCacheConfig`,
514-
- `per_request_ttl` is an optional override set in `MozAdsRequestCachePolicy`.
507+
1. `per_request_ttl` — caller-provided override on `MozAdsRequestCachePolicy`.
508+
2. `server_max_age` — value of the HTTP `Cache-Control: max-age=N` header on the response.
509+
3. `client_default_ttl` — configured on `MozAdsCacheConfig`.
515510

516-
If the effective TTL resolves to 0 seconds, the response is not cached.
511+
If the effective TTL resolves to 0 seconds (e.g. `Cache-Control: max-age=0`),
512+
the response is not cached. The resolved TTL is capped at 7 days regardless
513+
of source.
517514

518515
### Configuring The Cache
519516

components/ads-client/docs/usage-swift.md

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -501,19 +501,16 @@ It reduces redundant network traffic and improves latency for repeated or identi
501501

502502
### Cache Lifecycle
503503

504-
Each network response can be stored in the cache with an associated effective TTL, computed as:
504+
Each network response can be stored in the cache with an associated effective TTL,
505+
resolved by priority (highest to lowest):
505506

506-
```
507-
effective_ttl = min(server_max_age, client_default_ttl, per_request_ttl)
508-
```
509-
510-
where:
511-
512-
- `server_max_age` comes from the HTTP `Cache-Control: max-age=N` header (if present),
513-
- `client_default_ttl` is set in `MozAdsCacheConfig`,
514-
- `per_request_ttl` is an optional override set in `MozAdsRequestCachePolicy`.
507+
1. `per_request_ttl` — caller-provided override on `MozAdsRequestCachePolicy`.
508+
2. `server_max_age` — value of the HTTP `Cache-Control: max-age=N` header on the response.
509+
3. `client_default_ttl` — configured on `MozAdsCacheConfig`.
515510

516-
If the effective TTL resolves to 0 seconds, the response is not cached.
511+
If the effective TTL resolves to 0 seconds (e.g. `Cache-Control: max-age=0`),
512+
the response is not cached. The resolved TTL is capped at 7 days regardless
513+
of source.
517514

518515
### Configuring The Cache
519516

components/ads-client/src/http_cache.rs

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ mod outcome;
1111
mod request_hash;
1212
mod store;
1313
mod strategy;
14+
mod ttl;
1415

1516
use self::{
1617
builder::HttpCacheBuilder,
@@ -84,13 +85,15 @@ impl HttpCache {
8485
CachePolicy::CacheFirst { ttl } => CacheFirst {
8586
hash,
8687
request,
87-
ttl: ttl.unwrap_or(self.default_ttl),
88+
explicit_ttl: *ttl,
89+
default_ttl: self.default_ttl,
8890
}
8991
.apply(client, &self.store),
9092
CachePolicy::NetworkFirst { ttl } => NetworkFirst {
9193
hash,
9294
request,
93-
ttl: ttl.unwrap_or(self.default_ttl),
95+
explicit_ttl: *ttl,
96+
default_ttl: self.default_ttl,
9497
}
9598
.apply(client, &self.store),
9699
}?;
@@ -311,13 +314,13 @@ mod tests {
311314
}
312315

313316
#[test]
314-
fn ttl_resolution_min_of_server_request_default() {
317+
fn ttl_resolution_explicit_overrides_server_max_age() {
315318
viaduct_dev::init_backend_dev();
316319

317320
let m = mockito::mock("POST", "/ads")
318321
.with_status(200)
319322
.with_header("content-type", "application/json")
320-
.with_header("cache-control", "max-age=1") // Set max age to 1 second
323+
.with_header("cache-control", "max-age=1") // Server says 1 second
321324
.with_body(r#"{"ok":true}"#)
322325
.expect(1)
323326
.create();
@@ -326,24 +329,62 @@ mod tests {
326329
let req = make_post_request();
327330
let hash = RequestHash::new(&req);
328331
let policy = CachePolicy::CacheFirst {
329-
ttl: Some(Duration::from_secs(20)), // 20 second ttl specified vs the cache's default of 300s
332+
ttl: Some(Duration::from_secs(20)), // Caller asked for 20s
330333
};
331334

332335
let client = make_client();
333-
// Store ttl should resolve to 1s as specified by response headers
336+
// Caller's explicit TTL wins over the server's max-age.
334337
let (_, outcomes) = cache.send_with_policy(&client, req, &policy).unwrap();
335338
assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored));
336339

337-
// After ~>1s, cleanup should remove it
340+
// Past the server max-age (1s) but still under the explicit TTL (20s) — entry is still there.
338341
cache.store.get_clock().advance(2);
339342
cache.store.delete_expired_entries().unwrap();
343+
assert!(cache.store.lookup(&hash).unwrap().is_some());
340344

345+
// Past the explicit TTL — entry should be expired.
346+
cache.store.get_clock().advance(20);
347+
cache.store.delete_expired_entries().unwrap();
341348
assert!(cache.store.lookup(&hash).unwrap().is_none());
342349
m.assert();
343350
}
344351

345352
#[test]
346-
fn ttl_resolution_request_overrides_default_when_smaller() {
353+
fn ttl_resolution_uses_server_max_age_when_no_explicit_override() {
354+
viaduct_dev::init_backend_dev();
355+
356+
let _m = mockito::mock("POST", "/ads")
357+
.with_status(200)
358+
.with_header("content-type", "application/json")
359+
.with_header("cache-control", "max-age=2") // Server says 2 seconds
360+
.with_body(r#"{"ok":true}"#)
361+
.expect(1)
362+
.create();
363+
364+
// Configured default is 300s — should be ignored in favor of server max-age.
365+
let cache = make_cache_with_ttl(300);
366+
let req = make_post_request();
367+
let hash = RequestHash::new(&req);
368+
369+
let client = make_client();
370+
let (_, outcomes) = cache
371+
.send_with_policy(&client, req, &CachePolicy::default())
372+
.unwrap();
373+
assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored));
374+
375+
// Still cached at ~1s.
376+
cache.store.get_clock().advance(1);
377+
cache.store.delete_expired_entries().unwrap();
378+
assert!(cache.store.lookup(&hash).unwrap().is_some());
379+
380+
// Expired after exceeding server max-age.
381+
cache.store.get_clock().advance(2);
382+
cache.store.delete_expired_entries().unwrap();
383+
assert!(cache.store.lookup(&hash).unwrap().is_none());
384+
}
385+
386+
#[test]
387+
fn ttl_resolution_explicit_overrides_default() {
347388
viaduct_dev::init_backend_dev();
348389

349390
let m = mockito::mock("POST", "/ads")

components/ads-client/src/http_cache/cache_control.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,8 @@ impl CacheControl {
5151
!self.no_store
5252
}
5353

54-
pub fn effective_ttl(&self, requested_ttl: Duration) -> Duration {
55-
match self.max_age {
56-
Some(s) => std::cmp::min(requested_ttl, Duration::from_secs(s)),
57-
None => requested_ttl,
58-
}
54+
pub fn max_age_duration(&self) -> Option<Duration> {
55+
self.max_age.map(Duration::from_secs)
5956
}
6057
}
6158

components/ads-client/src/http_cache/strategy.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,16 @@
66
use super::cache_control::CacheControl;
77
use super::request_hash::RequestHash;
88
use super::store::HttpCacheStore;
9+
use super::ttl::EffectiveTtl;
910
use super::{CacheOutcome, HttpCacheSendResult};
1011
use std::time::Duration;
1112
use viaduct::{Client, Request};
1213

1314
pub struct CacheFirst {
1415
pub hash: RequestHash,
1516
pub request: Request,
16-
pub ttl: Duration,
17+
pub explicit_ttl: Option<Duration>,
18+
pub default_ttl: Duration,
1719
}
1820

1921
impl CacheFirst {
@@ -28,7 +30,8 @@ impl CacheFirst {
2830
let network = NetworkFirst {
2931
hash: self.hash,
3032
request: self.request,
31-
ttl: self.ttl,
33+
explicit_ttl: self.explicit_ttl,
34+
default_ttl: self.default_ttl,
3235
};
3336
let (response, mut network_outcomes) = network.apply(client, store)?;
3437
outcomes.append(&mut network_outcomes);
@@ -39,15 +42,21 @@ impl CacheFirst {
3942
pub struct NetworkFirst {
4043
pub hash: RequestHash,
4144
pub request: Request,
42-
pub ttl: Duration,
45+
pub explicit_ttl: Option<Duration>,
46+
pub default_ttl: Duration,
4347
}
4448

4549
impl NetworkFirst {
4650
pub fn apply(self, client: &Client, store: &HttpCacheStore) -> HttpCacheSendResult {
4751
let response = client.send_sync(self.request)?;
4852
let cache_control = CacheControl::from(&response);
4953
let outcome = if cache_control.should_cache() {
50-
let ttl = cache_control.effective_ttl(self.ttl);
54+
let ttl = EffectiveTtl {
55+
explicit: self.explicit_ttl,
56+
server_max_age: cache_control.max_age_duration(),
57+
default: self.default_ttl,
58+
}
59+
.resolve();
5160
if ttl.is_zero() {
5261
return Ok((response, vec![CacheOutcome::NoCache]));
5362
}
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
4+
*/
5+
6+
use std::time::Duration;
7+
8+
/// Hard ceiling on any resolved TTL, regardless of source. Guards against a
9+
/// misconfigured server (e.g. `Cache-Control: max-age=315360000`) pinning
10+
/// responses in the cache for far longer than is reasonable.
11+
pub const MAX_TTL: Duration = Duration::from_secs(7 * 24 * 60 * 60);
12+
13+
/// The TTL to use when storing a response in the cache, computed from
14+
/// three possible sources.
15+
///
16+
/// `explicit` comes from the caller, `server_max_age` from the response's
17+
/// `Cache-Control` header, and `default` from the cache's configuration.
18+
pub struct EffectiveTtl {
19+
/// Per-request override provided by the caller, if any.
20+
pub explicit: Option<Duration>,
21+
/// `Cache-Control: max-age` from the server response, if present.
22+
pub server_max_age: Option<Duration>,
23+
/// The cache's configured default TTL.
24+
pub default: Duration,
25+
}
26+
27+
impl EffectiveTtl {
28+
/// Resolve the TTL by priority (highest to lowest):
29+
/// 1. `explicit` — caller-provided per-request override.
30+
/// 2. `server_max_age` — value of `Cache-Control: max-age` on the response.
31+
/// 3. `default` — the cache's configured default.
32+
///
33+
/// The result is capped at [`MAX_TTL`].
34+
pub fn resolve(&self) -> Duration {
35+
let chosen = self
36+
.explicit
37+
.or(self.server_max_age)
38+
.unwrap_or(self.default);
39+
chosen.min(MAX_TTL)
40+
}
41+
}
42+
43+
#[cfg(test)]
44+
mod tests {
45+
use super::*;
46+
47+
#[test]
48+
fn explicit_overrides_server_max_age_and_default() {
49+
let ttl = EffectiveTtl {
50+
explicit: Some(Duration::from_secs(60)),
51+
server_max_age: Some(Duration::from_secs(3600)),
52+
default: Duration::from_secs(300),
53+
}
54+
.resolve();
55+
assert_eq!(ttl, Duration::from_secs(60));
56+
}
57+
58+
#[test]
59+
fn falls_back_to_server_max_age_when_no_explicit() {
60+
let ttl = EffectiveTtl {
61+
explicit: None,
62+
server_max_age: Some(Duration::from_secs(3600)),
63+
default: Duration::from_secs(300),
64+
}
65+
.resolve();
66+
assert_eq!(ttl, Duration::from_secs(3600));
67+
}
68+
69+
#[test]
70+
fn falls_back_to_default_when_no_explicit_and_no_server_max_age() {
71+
let ttl = EffectiveTtl {
72+
explicit: None,
73+
server_max_age: None,
74+
default: Duration::from_secs(300),
75+
}
76+
.resolve();
77+
assert_eq!(ttl, Duration::from_secs(300));
78+
}
79+
80+
#[test]
81+
fn zero_server_max_age_yields_zero() {
82+
// Lets the strategy emit NoCache without a network round-trip.
83+
let ttl = EffectiveTtl {
84+
explicit: None,
85+
server_max_age: Some(Duration::ZERO),
86+
default: Duration::from_secs(300),
87+
}
88+
.resolve();
89+
assert_eq!(ttl, Duration::ZERO);
90+
}
91+
92+
#[test]
93+
fn server_max_age_is_capped_at_max_ttl() {
94+
let ttl = EffectiveTtl {
95+
explicit: None,
96+
server_max_age: Some(Duration::from_secs(365 * 24 * 60 * 60)),
97+
default: Duration::from_secs(300),
98+
}
99+
.resolve();
100+
assert_eq!(ttl, MAX_TTL);
101+
}
102+
103+
#[test]
104+
fn explicit_ttl_is_capped_at_max_ttl() {
105+
let ttl = EffectiveTtl {
106+
explicit: Some(Duration::from_secs(30 * 24 * 60 * 60)),
107+
server_max_age: None,
108+
default: Duration::from_secs(300),
109+
}
110+
.resolve();
111+
assert_eq!(ttl, MAX_TTL);
112+
}
113+
114+
#[test]
115+
fn default_ttl_is_capped_at_max_ttl() {
116+
let ttl = EffectiveTtl {
117+
explicit: None,
118+
server_max_age: None,
119+
default: Duration::from_secs(30 * 24 * 60 * 60),
120+
}
121+
.resolve();
122+
assert_eq!(ttl, MAX_TTL);
123+
}
124+
}

0 commit comments

Comments
 (0)