| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #5510 +/- ##
=======================================
Coverage 93.46% 93.46%
=======================================
Files 110 110
Lines 37455 37458 +3
=======================================
+ Hits 35007 35010 +3
Misses 2448 2448 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
| return true | ||
| } | ||
|
|
||
| // Always revalidate requests whose age exceeds the max-age request |
There was a problem hiding this comment.
This comment is incredibly long. Shorten it
Sorry, something went wrong.
There was a problem hiding this comment.
Shortened in a10a00d — kept the max-age=0/falsy caveat and the RFC 9111 §5.2.1.1 ref. Thanks.
Sorry, something went wrong.
Addresses review feedback from mcollina on nodejs#5510: the max-age revalidation comment was too long. Trimmed to one sentence of rationale while keeping the max-age=0/falsy caveat and RFC 9111 ref. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses review feedback from mcollina on nodejs#5510: the max-age revalidation comment was too long. Trimmed to one sentence of rationale while keeping the max-age=0/falsy caveat and RFC 9111 ref. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The request max-age check in lib/interceptor/cache.js used
if (reqCacheControl?.['max-age'] && age >= reqCacheControl['max-age']) {
which is falsy for max-age=0, so the directive was silently ignored and a
cached response was served with no origin contact. RFC 9111 section 5.2.1.1
requires validation in that case. This also broke
fetch(url, { cache: 'no-cache' }) through a composed cache dispatcher, since
the fetch spec implements that mode by appending Cache-Control: max-age=0.
Additionally, when a nonzero request max-age was exceeded, the old code
bypassed the cache with a plain dispatch(opts, handler) (no CacheHandler),
so the fresh response fetched because of the bypass was never stored and the
next plain request had to contact the origin again.
Fix: treat request max-age exceedance as a revalidation trigger in
needsRevalidation(), feeding it into the existing revalidation flow (which
sends the conditional request, serves the cached body on 304, and stores a
fresh 200 via CacheHandler).
Before (mnot cache-tests): ccreq-ma0 "N no"; after: "Y yes".
Fixes nodejs#5504
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses review feedback from mcollina on nodejs#5510: the max-age revalidation comment was too long. Trimmed to one sentence of rationale while keeping the max-age=0/falsy caveat and RFC 9111 ref. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| Back | FazBrowse Home | New Git URL |
Fixes #5504
The bug
lib/interceptor/cache.js checked the request max-age directive with
For max-age=0 the parsed value 0 is falsy, so the directive was ignored entirely and a cached response was served with no origin contact. RFC 9111 §5.2.1.1 requires validation in that case. This also broke fetch(url, { cache: 'no-cache' }) through a composed cache dispatcher, since the fetch spec implements that mode by appending Cache-Control: max-age=0 (related umbrella: #3847).
Secondary issue in the same block: when a nonzero request max-age was exceeded, the bypass dispatched without a CacheHandler (return dispatch(opts, handler)), so the fresh 200 fetched because of the bypass was never stored — the very next plain request had to contact the origin again.
The fix
Treat request max-age exceedance as a revalidation trigger in needsRevalidation() (alongside the existing request no-cache handling), feeding it into the existing revalidation flow: a conditional request is sent, the cached body is served on 304, and a fresh 200 is stored via the CacheHandler already wrapped by CacheRevalidationHandler. The old falsy-guarded bypass block is removed.
Evidence
Repro from the issue (fresh cached response, then a request with Cache-Control: max-age=0):
Same for fetch(url, { cache: 'no-cache' }) through new Agent().compose(interceptors.cache(...)): before, 1 origin hit (revalidation never sent); after, the validation request is sent.
mnot cache-tests conformance runner (node test/cache-interceptor/cache-tests.mjs):
New tests in test/interceptors/cache.js (all fail on main, pass with this change):
Full cache test files (test/interceptors/cache*.js, test/cache-interceptor/*): 124 tests / 12 suites, 124 pass, 0 fail (baseline on main is 121/121; the +3 are the new tests).
Notes