From 0797def65392c03d61fea1179c2abcd89e80246e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Sun, 13 Jul 2025 20:20:39 +0200 Subject: [PATCH 01/11] cache: fix heuristic caching 307 responses must not be cached if they do not have an explicit caching directive. Final responses with an explicit caching directive can be cached even if their status code is not heuristically cacheable status codes. fix part of #4334 --- lib/handler/cache-handler.js | 11 +++-- test/interceptors/cache.js | 85 ++++++++++++++++++++---------------- 2 files changed, 55 insertions(+), 41 deletions(-) diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 937790aca12..4d630c7f035 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -241,10 +241,13 @@ class CacheHandler { * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives */ function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives) { - // Allow caching for status codes 200 and 307 (original behavior) - // Also allow caching for other status codes that are heuristically cacheable - // when they have explicit cache directives - if (statusCode !== 200 && statusCode !== 307 && !HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode)) { + // Status code must be final. + if (statusCode < 200) { + return false + } + // Responses with neither status codes that are heuristically cacheable, nor explicit caching directives, + // are not cacheable. + if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !cacheControlDirectives && !resHeaders['expires']) { return false } diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 21f84b562dc..508ce8316d3 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1437,8 +1437,11 @@ describe('Cache Interceptor', () => { }) }) + // Partial list. const cacheableStatusCodes = [ { code: 204, body: '' }, + { code: 302, body: 'Found' }, + { code: 307, body: 'Temporary Redirect' }, { code: 404, body: 'Not Found' }, { code: 410, body: 'Gone' } ] @@ -1453,7 +1456,7 @@ describe('Cache Interceptor', () => { res.end(body) }).listen(0) - const client = new Client(`http://localhost:${server.address().port}`) + const client = new Client(`http://localhost:${server.address().port}`, { maxRedirections: 0 }) .compose(interceptors.cache()) after(async () => { @@ -1489,47 +1492,55 @@ describe('Cache Interceptor', () => { }) } - test('does not cache non-heuristically cacheable error status codes', async () => { - let requestsToOrigin = 0 - const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { - requestsToOrigin++ - res.statusCode = 418 // I'm a teapot - not in heuristically cacheable list - res.setHeader('cache-control', 'public, max-age=60') - res.end('I am a teapot') - }).listen(0) + // Partial list. + const nonHeuristicallyCacheableStatusCodes = [ + { code: 201, body: 'Created' }, + { code: 307, body: 'Temporary Redirect' }, + { code: 418, body: 'I am a teapot' } + ] - const client = new Client(`http://localhost:${server.address().port}`) - .compose(interceptors.cache()) + for (const { code, body } of nonHeuristicallyCacheableStatusCodes) { + test(`does not cache non-heuristically cacheable status ${code} without explicit directive`, async () => { + let requestsToOrigin = 0 + const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { + requestsToOrigin++ + res.statusCode = code + res.end(body) + }).listen(0) - after(async () => { - server.close() - await client.close() - }) + const client = new Client(`http://localhost:${server.address().port}`, { maxRedirections: 0 }) + .compose(interceptors.cache({ cacheByDefault: 60 })) - await once(server, 'listening') + after(async () => { + server.close() + await client.close() + }) - equal(requestsToOrigin, 0) + await once(server, 'listening') - const request = { - origin: 'localhost', - method: 'GET', - path: '/' - } + equal(requestsToOrigin, 0) - // First request should hit the origin - { - const res = await client.request(request) - equal(requestsToOrigin, 1) - equal(res.statusCode, 418) - strictEqual(await res.body.text(), 'I am a teapot') - } + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } - // Second request should also hit the origin (not cached) - { - const res = await client.request(request) - equal(requestsToOrigin, 2) // Should be 2 (not cached) - equal(res.statusCode, 418) - strictEqual(await res.body.text(), 'I am a teapot') - } - }) + // First request should hit the origin + { + const res = await client.request(request) + equal(requestsToOrigin, 1) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + + // Second request should also hit the origin (not cached) + { + const res = await client.request(request) + equal(requestsToOrigin, 2) // Should be 2 (not cached) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + }) + } }) From 00d1d4568b62d539640cd41dfa04173c0f9b9eb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Mon, 14 Jul 2025 15:04:25 +0200 Subject: [PATCH 02/11] fix caching of not understood statuses finish fixing #4334 --- lib/handler/cache-handler.js | 13 ++++- test/interceptors/cache.js | 110 +++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 4d630c7f035..7ad5450271e 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -15,6 +15,15 @@ const HEURISTICALLY_CACHEABLE_STATUS_CODES = [ 200, 203, 204, 206, 300, 301, 308, 404, 405, 410, 414, 501 ] +// Status codes which semantic is not handled by the cache +// https://datatracker.ietf.org/doc/html/rfc9111#section-3 +// This list should not grow beyond 206 and 304 unless the RFC is updated +// by a newer one including more. Please introduce another list if +// implementing caching of responses with the 'must-understand' directive. +const NOT_UNDERSTOOD_STATUS_CODES = [ + 206, 304 +] + const MAX_RESPONSE_AGE = 2147483647000 /** @@ -241,8 +250,8 @@ class CacheHandler { * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives */ function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives) { - // Status code must be final. - if (statusCode < 200) { + // Status code must be final and understood. + if (statusCode < 200 || NOT_UNDERSTOOD_STATUS_CODES.includes(statusCode)) { return false } // Responses with neither status codes that are heuristically cacheable, nor explicit caching directives, diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 508ce8316d3..6c1cb1eafa7 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1543,4 +1543,114 @@ describe('Cache Interceptor', () => { } }) } + + test('discriminates caching of range requests, or does not cache them', async () => { + let requestsToOrigin = 0 + const body = 'Fake range request response' + const code = 206 + const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { + requestsToOrigin++ + res.statusCode = code + res.setHeader('cache-control', 'public, max-age=60') + res.end(body) + }).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') + + equal(requestsToOrigin, 0) + + const request = { + origin: 'localhost', + method: 'GET', + path: '/', + headers: { + range: 'bytes=10-' + } + } + + // First request should hit the origin + { + const res = await client.request(request) + equal(requestsToOrigin, 1) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + + // Second request with different range should hit the origin too + request.headers.range = 'bytes=5-' + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + }) + + test('discriminates caching of conditionnal requests, or does not cache them', async () => { + let requestsToOrigin = 0 + const body = '' + const code = 304 + const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { + requestsToOrigin++ + res.statusCode = code + res.setHeader('cache-control', 'public, max-age=60') + res.end(body) + }).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') + + equal(requestsToOrigin, 0) + + const request = { + origin: 'localhost', + method: 'GET', + path: '/', + headers: { + 'if-modified-since': new Date().toUTCString(), + 'if-none-match': 'some-etag' + } + } + + // First request should hit the origin + { + const res = await client.request(request) + equal(requestsToOrigin, 1) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + + // Second request with different etag should hit the origin too + request.headers['if-none-match'] = 'another-etag' + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + + // Third request with different since should hit the origin too + request.headers['if-modified-since'] = new Date(0).toUTCString() + { + const res = await client.request(request) + equal(requestsToOrigin, 3) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + }) }) From 1c749758ffa992712c90aafdb0b236ab0f7fa110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Tue, 15 Jul 2025 17:47:10 +0200 Subject: [PATCH 03/11] Split conditionnal request test in two --- test/interceptors/cache.js | 48 ++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 6c1cb1eafa7..f09c5d69618 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1594,7 +1594,7 @@ describe('Cache Interceptor', () => { } }) - test('discriminates caching of conditionnal requests, or does not cache them', async () => { + test('discriminates caching of conditionnal requests (if-none-match), or does not cache them', async () => { let requestsToOrigin = 0 const body = '' const code = 304 @@ -1622,7 +1622,6 @@ describe('Cache Interceptor', () => { method: 'GET', path: '/', headers: { - 'if-modified-since': new Date().toUTCString(), 'if-none-match': 'some-etag' } } @@ -1643,12 +1642,53 @@ describe('Cache Interceptor', () => { equal(res.statusCode, code) strictEqual(await res.body.text(), body) } + }) + + test('discriminates caching of conditionnal requests (if-modified-since), or does not cache them', async () => { + let requestsToOrigin = 0 + const body = '' + const code = 304 + const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { + requestsToOrigin++ + res.statusCode = code + res.setHeader('cache-control', 'public, max-age=60') + res.end(body) + }).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') - // Third request with different since should hit the origin too + equal(requestsToOrigin, 0) + + const request = { + origin: 'localhost', + method: 'GET', + path: '/', + headers: { + 'if-modified-since': new Date().toUTCString() + } + } + + // First request should hit the origin + { + const res = await client.request(request) + equal(requestsToOrigin, 1) + equal(res.statusCode, code) + strictEqual(await res.body.text(), body) + } + + // Second request with different since should hit the origin too request.headers['if-modified-since'] = new Date(0).toUTCString() { const res = await client.request(request) - equal(requestsToOrigin, 3) + equal(requestsToOrigin, 2) equal(res.statusCode, code) strictEqual(await res.body.text(), body) } From a8c858ae2083c8b9108dcb1d721a607922491e3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 16 Jul 2025 21:22:34 +0200 Subject: [PATCH 04/11] Fix the change and the test --- lib/handler/cache-handler.js | 2 +- test/interceptors/cache.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 7ad5450271e..891c348f231 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -256,7 +256,7 @@ function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirect } // Responses with neither status codes that are heuristically cacheable, nor explicit caching directives, // are not cacheable. - if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !cacheControlDirectives && !resHeaders['expires']) { + if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['cache-control'] && !resHeaders['expires']) { return false } diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index f09c5d69618..c10da5f799e 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1505,6 +1505,7 @@ describe('Cache Interceptor', () => { const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { requestsToOrigin++ res.statusCode = code + res.setHeader('date', '') res.end(body) }).listen(0) From e6bd1a1adc136f3b1a3dd59a13b9bde8acc7759c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 16 Jul 2025 21:46:30 +0200 Subject: [PATCH 05/11] Adhere stricly to RFC 9111 cacheable conditions --- lib/handler/cache-handler.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 891c348f231..c21a7206551 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -254,9 +254,15 @@ function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirect if (statusCode < 200 || NOT_UNDERSTOOD_STATUS_CODES.includes(statusCode)) { return false } - // Responses with neither status codes that are heuristically cacheable, nor explicit caching directives, - // are not cacheable. - if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['cache-control'] && !resHeaders['expires']) { + // Responses with neither status codes that are heuristically cacheable, nor "explicit enough" caching + // directives, are not cacheable. "Explicit enough": see https://www.rfc-editor.org/rfc/rfc9111.html#section-3 + if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['expires'] && + !cacheControlDirectives.public && + cacheControlDirectives['max-age'] === undefined && + // RFC 9111: a private response directive, if the cache is not shared + !(cacheControlDirectives.private && cacheType === 'private') && + !(cacheControlDirectives['s-maxage'] !== undefined && cacheType === 'shared') + ) { return false } From 5cf41d43425a8cf7825c86dbf9d68b9b4e068470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 16 Jul 2025 22:01:37 +0200 Subject: [PATCH 06/11] Do the full cacheability test only for cases requiring it --- lib/handler/cache-handler.js | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index c21a7206551..102e3f29381 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -134,7 +134,7 @@ class CacheHandler { } const cacheControlDirectives = cacheControlHeader ? parseCacheControlHeader(cacheControlHeader) : {} - if (!canCacheResponse(this.#cacheType, statusCode, resHeaders, cacheControlDirectives)) { + if (!canCacheResponse(this.#cacheType, statusCode, resHeaders, cacheControlDirectives, this.#cacheByDefault)) { return downstreamOnHeaders() } @@ -248,22 +248,28 @@ class CacheHandler { * @param {number} statusCode * @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives + * @param {number | undefined} cacheByDefault */ -function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives) { +function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives, cacheByDefault) { // Status code must be final and understood. if (statusCode < 200 || NOT_UNDERSTOOD_STATUS_CODES.includes(statusCode)) { return false } - // Responses with neither status codes that are heuristically cacheable, nor "explicit enough" caching - // directives, are not cacheable. "Explicit enough": see https://www.rfc-editor.org/rfc/rfc9111.html#section-3 - if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['expires'] && - !cacheControlDirectives.public && - cacheControlDirectives['max-age'] === undefined && - // RFC 9111: a private response directive, if the cache is not shared - !(cacheControlDirectives.private && cacheType === 'private') && - !(cacheControlDirectives['s-maxage'] !== undefined && cacheType === 'shared') - ) { - return false + + if (cacheByDefault) { + // When cacheByDefault is truthy, the cache interceptor does no more mandate an explicit expiry prior to + // reaching here. We should then check the full condition allowing to consider caching responses. + // Responses with neither status codes that are heuristically cacheable, nor "explicit enough" caching + // directives, are not cacheable. "Explicit enough": see https://www.rfc-editor.org/rfc/rfc9111.html#section-3 + if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['expires'] && + !cacheControlDirectives.public && + cacheControlDirectives['max-age'] === undefined && + // RFC 9111: a private response directive, if the cache is not shared + !(cacheControlDirectives.private && cacheType === 'private') && + !(cacheControlDirectives['s-maxage'] !== undefined && cacheType === 'shared') + ) { + return false + } } if (cacheControlDirectives['no-store']) { From 27c070537f430804ff7f41aafeb9aba1c47e7c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 16 Jul 2025 22:11:05 +0200 Subject: [PATCH 07/11] Adujst a comment --- lib/handler/cache-handler.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 102e3f29381..06188bcb6a4 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -257,8 +257,9 @@ function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirect } if (cacheByDefault) { - // When cacheByDefault is truthy, the cache interceptor does no more mandate an explicit expiry prior to - // reaching here. We should then check the full condition allowing to consider caching responses. + // When cacheByDefault is truthy, the cache interceptor does no more mandate an explicit expiry or + // heuristically cacheability prior to reaching here. We should then check the full condition allowing + // to consider caching responses. // Responses with neither status codes that are heuristically cacheable, nor "explicit enough" caching // directives, are not cacheable. "Explicit enough": see https://www.rfc-editor.org/rfc/rfc9111.html#section-3 if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['expires'] && From 3764f34c9cb0deead3f7b3c76a3a4f61904d867f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 16 Jul 2025 22:27:38 +0200 Subject: [PATCH 08/11] Remove a forgotten hack in test --- test/interceptors/cache.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index c10da5f799e..f09c5d69618 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1505,7 +1505,6 @@ describe('Cache Interceptor', () => { const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { requestsToOrigin++ res.statusCode = code - res.setHeader('date', '') res.end(body) }).listen(0) From 0fea79b76d218d628a00a90e02b0c4879cd84ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 16 Jul 2025 22:50:18 +0200 Subject: [PATCH 09/11] Redo the hack, required for actually testing --- test/interceptors/cache.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index f09c5d69618..4a9c0c84ca3 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1505,6 +1505,10 @@ describe('Cache Interceptor', () => { const server = createServer({ joinDuplicateHeaders: true }, (_, res) => { requestsToOrigin++ res.statusCode = code + // By default the response may have a date and last-modified header set to 'now', + // causing the cache to compute a 0 heuristic expiry, causing the test to not ascertain + // it is really not cached. + res.setHeader('date', '') res.end(body) }).listen(0) From 3a2c98169a0c70331316ca6077f8ad41292ff13e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Thu, 17 Jul 2025 08:54:12 +0200 Subject: [PATCH 10/11] Revert "Do the full cacheability test only for cases requiring it" This reverts commit 5cf41d43425a8cf7825c86dbf9d68b9b4e068470. --- lib/handler/cache-handler.js | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 06188bcb6a4..c21a7206551 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -134,7 +134,7 @@ class CacheHandler { } const cacheControlDirectives = cacheControlHeader ? parseCacheControlHeader(cacheControlHeader) : {} - if (!canCacheResponse(this.#cacheType, statusCode, resHeaders, cacheControlDirectives, this.#cacheByDefault)) { + if (!canCacheResponse(this.#cacheType, statusCode, resHeaders, cacheControlDirectives)) { return downstreamOnHeaders() } @@ -248,29 +248,22 @@ class CacheHandler { * @param {number} statusCode * @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives - * @param {number | undefined} cacheByDefault */ -function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives, cacheByDefault) { +function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives) { // Status code must be final and understood. if (statusCode < 200 || NOT_UNDERSTOOD_STATUS_CODES.includes(statusCode)) { return false } - - if (cacheByDefault) { - // When cacheByDefault is truthy, the cache interceptor does no more mandate an explicit expiry or - // heuristically cacheability prior to reaching here. We should then check the full condition allowing - // to consider caching responses. - // Responses with neither status codes that are heuristically cacheable, nor "explicit enough" caching - // directives, are not cacheable. "Explicit enough": see https://www.rfc-editor.org/rfc/rfc9111.html#section-3 - if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['expires'] && - !cacheControlDirectives.public && - cacheControlDirectives['max-age'] === undefined && - // RFC 9111: a private response directive, if the cache is not shared - !(cacheControlDirectives.private && cacheType === 'private') && - !(cacheControlDirectives['s-maxage'] !== undefined && cacheType === 'shared') - ) { - return false - } + // Responses with neither status codes that are heuristically cacheable, nor "explicit enough" caching + // directives, are not cacheable. "Explicit enough": see https://www.rfc-editor.org/rfc/rfc9111.html#section-3 + if (!HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode) && !resHeaders['expires'] && + !cacheControlDirectives.public && + cacheControlDirectives['max-age'] === undefined && + // RFC 9111: a private response directive, if the cache is not shared + !(cacheControlDirectives.private && cacheType === 'private') && + !(cacheControlDirectives['s-maxage'] !== undefined && cacheType === 'shared') + ) { + return false } if (cacheControlDirectives['no-store']) { From 662c81b4dc90a1ae380ce1010b44d7cc9873f7e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Thu, 17 Jul 2025 17:57:01 +0200 Subject: [PATCH 11/11] Remove obsolete config from test --- test/interceptors/cache.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 4a9c0c84ca3..a29ce9ca1e2 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -1456,7 +1456,7 @@ describe('Cache Interceptor', () => { res.end(body) }).listen(0) - const client = new Client(`http://localhost:${server.address().port}`, { maxRedirections: 0 }) + const client = new Client(`http://localhost:${server.address().port}`) .compose(interceptors.cache()) after(async () => { @@ -1512,7 +1512,7 @@ describe('Cache Interceptor', () => { res.end(body) }).listen(0) - const client = new Client(`http://localhost:${server.address().port}`, { maxRedirections: 0 }) + const client = new Client(`http://localhost:${server.address().port}`) .compose(interceptors.cache({ cacheByDefault: 60 })) after(async () => {