From 5416d456e802d92fb3521d6ea77b0079a36f278c Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Sat, 4 Jul 2026 16:23:00 +0000 Subject: [PATCH 1/2] fix: honor request cache-control max-age=0 in cache interceptor 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 #5504 Co-Authored-By: Claude Fable 5 --- lib/interceptor/cache.js | 19 ++-- test/interceptors/cache.js | 182 +++++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 7 deletions(-) diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index 5d497fca561..f1c4ef28591 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -34,16 +34,26 @@ const nop = () => {} /** * @param {import('../../types/cache-interceptor.d.ts').default.GetResult} result + * @param {number} age * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives | undefined} cacheControlDirectives * @param {import('../../types/dispatcher.d.ts').default.RequestOptions} opts * @returns {boolean} */ -function needsRevalidation (result, cacheControlDirectives, { headers = {} }) { +function needsRevalidation (result, age, cacheControlDirectives, { headers = {} }) { // Always revalidate requests with the no-cache request directive. if (cacheControlDirectives?.['no-cache']) { return true } + // Always revalidate requests whose age exceeds the max-age request + // directive. Note that max-age=0 must not be ignored (it is falsy), it + // forces revalidation on every request (e.g. fetch's cache: 'no-cache' + // mode sends Cache-Control: max-age=0). + // https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.1.1 + if (cacheControlDirectives?.['max-age'] !== undefined && age >= cacheControlDirectives['max-age']) { + return true + } + // Always revalidate requests with unqualified no-cache response directive. if (result.cacheControlDirectives?.['no-cache'] && !Array.isArray(result.cacheControlDirectives['no-cache'])) { return true @@ -297,14 +307,9 @@ function handleResult ( } const age = Math.round((now - result.cachedAt) / 1000) - if (reqCacheControl?.['max-age'] && age >= reqCacheControl['max-age']) { - // Response is considered expired for this specific request - // https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.1.1 - return dispatch(opts, handler) - } const stale = isStale(result, reqCacheControl, globalOpts.type) - const revalidate = needsRevalidation(result, reqCacheControl, opts) + const revalidate = needsRevalidation(result, age, reqCacheControl, opts) // Check if the response is stale if (stale || revalidate) { diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index d429f8004de..9b9d8817f1c 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -872,6 +872,188 @@ describe('Cache Interceptor', () => { equal(requestsToOrigin, 2) }) + test('max-age=0 revalidates the response (RFC 9111 ยง5.2.1.1)', async () => { + let requestsToOrigin = 0 + let revalidationRequests = 0 + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + if (req.headers['if-none-match']) { + revalidationRequests++ + res.statusCode = 304 + res.end() + } else { + requestsToOrigin++ + res.setHeader('cache-control', 'public, s-maxage=100') + res.setHeader('etag', '"asd"') + res.end('asd') + } + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + server.close() + await client.close() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + + /** + * @type {import('../../types/dispatcher').default.RequestOptions} + */ + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } + + // Send initial request to fill the cache + { + const response = await client.request(request) + strictEqual(requestsToOrigin, 1) + strictEqual(revalidationRequests, 0) + strictEqual(await response.body.text(), 'asd') + } + + // Send second request with max-age=0, a validation request should be + // sent and the cached response served on the 304 + { + const response = await client.request({ + ...request, + headers: { + 'cache-control': 'max-age=0' + } + }) + strictEqual(requestsToOrigin, 1) + strictEqual(revalidationRequests, 1) + strictEqual(response.statusCode, 200) + strictEqual(await response.body.text(), 'asd') + } + + // Send third request w/o max-age, this should be handled by the cache + { + const response = await client.request(request) + strictEqual(requestsToOrigin, 1) + strictEqual(revalidationRequests, 1) + strictEqual(await response.body.text(), 'asd') + } + }) + + test('response fetched due to max-age exceedance updates the cache', async () => { + const clock = FakeTimers.install({ + toFake: ['Date'] + }) + + let requestsToOrigin = 0 + const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { + requestsToOrigin++ + res.setHeader('cache-control', 'public, s-maxage=100') + res.end(`response ${requestsToOrigin}`) + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + clock.uninstall() + server.close() + await client.close() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + + /** + * @type {import('../../types/dispatcher').default.RequestOptions} + */ + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } + + // Send first request to cache the response + { + const response = await client.request(request) + strictEqual(requestsToOrigin, 1) + strictEqual(await response.body.text(), 'response 1') + } + + clock.tick(6000) + + // Send second request with the response's age exceeding the request's + // max-age, the origin's fresh response should be served and stored + { + const response = await client.request({ + ...request, + headers: { + 'cache-control': 'max-age=5' + } + }) + strictEqual(requestsToOrigin, 2) + strictEqual(await response.body.text(), 'response 2') + } + + // Send third request w/o max-age, the fresh response stored by the + // previous request should be served from the cache + { + const response = await client.request(request) + strictEqual(requestsToOrigin, 2) + strictEqual(await response.body.text(), 'response 2') + } + }) + + test('fetch with cache: no-cache revalidates through a composed cache dispatcher', async () => { + const { fetch, Agent } = require('../..') + + let requestsToOrigin = 0 + let revalidationRequests = 0 + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + if (req.headers['if-none-match']) { + revalidationRequests++ + res.statusCode = 304 + res.end() + } else { + requestsToOrigin++ + res.setHeader('cache-control', 'public, max-age=100') + res.setHeader('etag', '"asd"') + res.end('asd') + } + }).listen(0) + + const dispatcher = new Agent().compose(interceptors.cache()) + + after(async () => { + server.close() + await dispatcher.close() + }) + + await once(server, 'listening') + + const origin = `http://localhost:${server.address().port}` + + // Send initial request to fill the cache + { + const response = await fetch(origin, { dispatcher }) + strictEqual(requestsToOrigin, 1) + strictEqual(revalidationRequests, 0) + strictEqual(await response.text(), 'asd') + } + + // The no-cache fetch cache mode appends cache-control: max-age=0, which + // must trigger a validation request + { + const response = await fetch(origin, { dispatcher, cache: 'no-cache' }) + strictEqual(requestsToOrigin, 1) + strictEqual(revalidationRequests, 1) + strictEqual(response.status, 200) + strictEqual(await response.text(), 'asd') + } + }) + test('max-stale', async () => { const clock = FakeTimers.install({ toFake: ['Date'] From 5d349383aabbf185c56b11acae6ef953e5f7e862 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Sat, 4 Jul 2026 21:03:50 +0000 Subject: [PATCH 2/2] docs: shorten cache interceptor comment Addresses review feedback from mcollina on nodejs/undici#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 --- lib/interceptor/cache.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index f1c4ef28591..5dea6824582 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -45,10 +45,8 @@ function needsRevalidation (result, age, cacheControlDirectives, { headers = {} return true } - // Always revalidate requests whose age exceeds the max-age request - // directive. Note that max-age=0 must not be ignored (it is falsy), it - // forces revalidation on every request (e.g. fetch's cache: 'no-cache' - // mode sends Cache-Control: max-age=0). + // Revalidate when age >= max-age. max-age=0 is falsy but must still + // force it (e.g. fetch cache: 'no-cache' sends max-age=0). // https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.1.1 if (cacheControlDirectives?.['max-age'] !== undefined && age >= cacheControlDirectives['max-age']) { return true