Skip to content

Commit 41604f3

Browse files
fix(core): a non-future server cache directive no longer expires the document permanently
`effectiveTtlSeconds()` subtracted the cache timestamp from the server expiry, so an expiry that was not in the future produced a negative TTL. `CacheHeaderParser.parseExpiresAt` returns `0` for `Cache-Control: no-store` and `no-cache`, the current second for `max-age=0`, and a past epoch for a stale `Expires:` — for `no-store` the result is about -1.7e9. A negative TTL is expired on every read, so `get()` took the synchronous re-fetch branch on the caller's thread every time, forever. The failure backoff could not cover it: that only arms when a fetch *throws*, and an endpoint answering `no-store` successfully clears the backoff and re-arms the expiry on the same call. This was latent while nothing on a verification path read the metadata cache. This change series is what puts it there — verification reads through the cache before every key lookup, and `beforeLookup()` runs before signature verification, so an unauthenticated caller would have set the fetch rate against the authorization server. It is the failure this series exists to remove, reached through a different door. An expiry at or before the cache timestamp is now read as "no preference" and the configured interval governs. go-sdk clamps the equivalent case the same way rather than taking a zero expiry literally. Two tests, and the negative control holds: with the clamp removed both fail and the pre-existing `get_serverExpiresTtl_usesMinOfConfigured...` still passes, which is why it never covered this — it only exercises a *future* server expiry. Also corrects two claims about gate ordering that were not true. The `requireNoUserinfo` javadoc said it runs "last of the four", and the changelog said `wellKnownUrl` "enforces the same four gates as the constructors". The set is the same; the order is not — the three construction sites run fragment, query, scheme, userinfo and `wellKnownUrl` runs fragment, scheme, userinfo, query. An identifier violating two gates can therefore be reported for a different component depending on the entrypoint. Both reject, so only the message differs; unifying the four behind one private gate touches four call sites with different shapes and is left to its own change rather than bolted onto this one. 990 tests, 0 failures.
1 parent 99739a3 commit 41604f3

4 files changed

Lines changed: 120 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2121

2222
### Fixed
2323

24+
- A server cache directive that is not in the future no longer makes the document permanently
25+
expired. `Cache-Control: no-store` and `no-cache` parse to an expiry of `0`, `max-age=0` to the
26+
current second, and a stale `Expires:` to a past one; the effective TTL was computed by
27+
subtracting the cache timestamp from that, so any of them produced a *negative* TTL — about
28+
-1.7e9 for `no-store`. A negative TTL is expired on every read, so every read took the
29+
synchronous re-fetch branch on the caller's thread, and the failure backoff could not help
30+
because it only arms when a fetch throws: an endpoint answering `no-store` successfully cleared
31+
the backoff and re-armed the expiry on the same call. Such an expiry is now treated as no
32+
preference and the configured interval governs. This was latent while nothing on a verification
33+
path read the metadata cache; the change above puts it there, before signature verification, so
34+
an unauthenticated caller would otherwise have set the fetch rate against the authorization
35+
server.
36+
2437
- Authorization server metadata is now re-read under ordinary verification traffic, so
2538
`metadataRefreshSeconds` takes effect on a resource server that only verifies tokens. Such a
2639
server never calls the token, introspection or revocation endpoints, so nothing on its request
@@ -107,7 +120,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
107120
question, not settled by this change, and tracked.
108121

109122
**Migration:** A scheme-relative or relative resource identifier now fails at startup instead of
110-
at the first 401. Prefix the intended scheme. `wellKnownUrl` enforces the same four gates as the
123+
at the first 401. Prefix the intended scheme. `wellKnownUrl` enforces the same four gates (in a
124+
different order, so the component named in the message can differ from a constructor's) as the
111125
constructors, so a caller reaching it directly with a string no constructor saw is refused there
112126
too rather than splicing a malformed identifier into a challenge.
113127

core/src/main/java/ai/authplane/sdk/core/fetching/DocumentCache.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,35 @@ private long failureBackoffSeconds() {
366366
return failureBackoffSeconds(configuredRefreshSeconds);
367367
}
368368

369+
/**
370+
* The TTL actually in force: the configured interval, shortened by a server expiry when the
371+
* server asks for something sooner.
372+
*
373+
* <p>A server expiry at or before the moment the document was cached is treated as **no
374+
* preference** rather than as an expiry, and the configured interval governs. It has to be:
375+
* {@code CacheHeaderParser.parseExpiresAt} returns {@code 0L} for {@code Cache-Control:
376+
* no-store} or {@code no-cache}, {@code now} for {@code max-age=0}, and a past epoch for a
377+
* stale {@code Expires:} — and subtracting {@code cachedAtEpochSeconds} from any of those
378+
* yields a negative TTL. For {@code no-store} that is about -1.7e9.
379+
*
380+
* <p>A negative TTL makes {@code age >= effectiveTtl} true on every read, so {@link #get()}
381+
* takes the synchronous re-fetch branch on the caller's thread every single time, forever. The
382+
* failure backoff does not cover it, because that only arms when a fetch *throws*: an endpoint
383+
* that answers {@code no-store} successfully clears the backoff and re-arms the expiry on the
384+
* same call.
385+
*
386+
* <p>That was harmless while nothing on a verification path read this cache. It stopped being
387+
* harmless when metadata moved onto that path — verification now reads through here before
388+
* every key lookup, and that runs before signature verification, so an unauthenticated caller
389+
* would set the rate. This is the same failure the backoff was added to remove, reached by a
390+
* different door.
391+
*
392+
* <p>go-sdk clamps the equivalent case the same way: a zero expiry falls back to the configured
393+
* default rather than being taken literally.
394+
*/
369395
private long effectiveTtlSeconds() {
370-
long configuredExpiry = cachedAtEpochSeconds + configuredRefreshSeconds;
371-
if (serverExpiresAtSeconds != null) {
396+
if (serverExpiresAtSeconds != null && serverExpiresAtSeconds > cachedAtEpochSeconds) {
397+
long configuredExpiry = cachedAtEpochSeconds + configuredRefreshSeconds;
372398
return Math.min(configuredExpiry, serverExpiresAtSeconds) - cachedAtEpochSeconds;
373399
}
374400
return configuredRefreshSeconds;

core/src/main/java/ai/authplane/sdk/core/prm/ProtectedResourceMetadata.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,15 @@ public static void requireScheme(String resourceUri) {
438438
*
439439
* <p>Called from the same construction boundaries as the sibling gates — {@link
440440
* Builder#build()}, the {@code AuthplaneResource} constructor, and {@code
441-
* AuthplaneClient.resource(...)} — and last of the four, so an identifier that is also
442-
* scheme-relative is reported for the missing scheme, the defect an operator fixes first.
441+
* AuthplaneClient.resource(...)} — after {@link #requireScheme(String)}, so an identifier that
442+
* is also scheme-relative is reported for the missing scheme, the defect an operator fixes
443+
* first.
444+
*
445+
* <p>The four gates run in the same *set* everywhere but not in the same *order*: the three
446+
* construction sites run fragment, query, scheme, userinfo, while {@link #wellKnownUrl(String)}
447+
* runs fragment, scheme, userinfo, query. So an identifier that violates two of them can be
448+
* reported for a different component depending on the entrypoint. Both reject either way; only
449+
* the message differs. Unifying the four behind one private gate is tracked.
443450
*
444451
* @param resourceUri the resource identifier, as configured by the operator
445452
* @throws IllegalArgumentException if the identifier's authority carries a userinfo component

core/src/test/java/ai/authplane/sdk/core/fetching/DocumentCacheTest.java

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,74 @@ void get_triggersBackgroundRefreshAt80PercentTtl() throws Exception {
136136
assertThat(fetchCount.get()).isEqualTo(2);
137137
}
138138

139+
/**
140+
* A server expiry that is not in the future is no expiry at all.
141+
*
142+
* <p>`Cache-Control: no-store` and `no-cache` parse to `0L`, `max-age=0` to `now`, and a stale
143+
* `Expires:` to a past epoch. Subtracting the cache timestamp from any of those gives a
144+
* negative TTL, which makes the document permanently expired: every read takes the synchronous
145+
* re-fetch branch, on the caller's thread. The failure backoff cannot help, because a
146+
* `no-store` endpoint that *answers* clears it and re-arms the expiry on the same call.
147+
*
148+
* <p>This matters now that verification reads through the metadata cache on every key lookup —
149+
* and does so before signature verification, so an unauthenticated caller would set the fetch
150+
* rate against the authorization server.
151+
*/
152+
@Test
153+
void get_serverExpiryNotInTheFuture_fallsBackToTheConfiguredInterval() throws Exception {
154+
// 0L is what no-store and no-cache parse to; -1 stands for a stale Expires: header.
155+
for (long serverExpiry : new long[] {0L, -1L}) {
156+
AtomicInteger fetchCount = new AtomicInteger();
157+
TestClock clock = new TestClock();
158+
DocumentFetcher fetcher =
159+
url -> {
160+
fetchCount.incrementAndGet();
161+
return CompletableFuture.completedFuture(
162+
new FetchResult(DOC_V1, serverExpiry));
163+
};
164+
cache = cacheWith(fetcher, 300, clock);
165+
cache.fetch();
166+
assertThat(fetchCount.get()).isEqualTo(1);
167+
168+
for (int i = 0; i < 5; i++) {
169+
assertThat(cache.get()).isEqualTo(DOC_V1);
170+
}
171+
assertThat(fetchCount.get())
172+
.as(
173+
"server expiry %s must not make the document permanently expired",
174+
serverExpiry)
175+
.isEqualTo(1);
176+
177+
clock.advanceSeconds(301);
178+
cache.get();
179+
assertThat(fetchCount.get())
180+
.as("the configured interval still governs for server expiry %s", serverExpiry)
181+
.isEqualTo(2);
182+
}
183+
}
184+
185+
/**
186+
* A server expiry exactly at the cache timestamp is the max-age=0 case, and behaves the same.
187+
*/
188+
@Test
189+
void get_serverExpiryEqualToCachedAt_fallsBackToTheConfiguredInterval() throws Exception {
190+
AtomicInteger fetchCount = new AtomicInteger();
191+
TestClock clock = new TestClock();
192+
DocumentFetcher fetcher =
193+
url -> {
194+
fetchCount.incrementAndGet();
195+
return CompletableFuture.completedFuture(
196+
new FetchResult(DOC_V1, clock.instant().getEpochSecond()));
197+
};
198+
cache = cacheWith(fetcher, 300, clock);
199+
cache.fetch();
200+
201+
for (int i = 0; i < 5; i++) {
202+
cache.get();
203+
}
204+
assertThat(fetchCount.get()).as("max-age=0 must not cost a fetch per read").isEqualTo(1);
205+
}
206+
139207
@Test
140208
void get_serverExpiresTtl_usesMinOfConfiguredAndServer() throws Exception {
141209
// Server says the document expires 10s from now; the configured TTL is 300s. The

0 commit comments

Comments
 (0)