diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 5a15990af0b..1bba719aff5 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -4,6 +4,7 @@ const util = require('../core/util') const { parseCacheControlHeader, parseVaryHeader, + varyHeaderHasWildcard, isEtagUsable } = require('../util/cache') const { parseHttpDate } = require('../util/date.js') @@ -434,7 +435,7 @@ function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirect } // https://www.rfc-editor.org/rfc/rfc9111.html#section-4.1-5 - if (resHeaders.vary?.includes('*')) { + if (resHeaders.vary !== undefined && varyHeaderHasWildcard(resHeaders.vary)) { return false } diff --git a/lib/util/cache.js b/lib/util/cache.js index c88db6292b3..3af4e19f373 100644 --- a/lib/util/cache.js +++ b/lib/util/cache.js @@ -320,13 +320,28 @@ function parseCacheControlHeader (header) { return output } +/** + * A repeated Vary header is equivalent to a single one holding the + * comma-separated concatenation of the values, so each part of the list still + * has to be looked at on its own. + * @see https://www.rfc-editor.org/rfc/rfc9110.html#section-5.3 + * + * @param {string | string[]} varyHeader Vary header from the server + * @returns {boolean} + */ +function varyHeaderHasWildcard (varyHeader) { + return typeof varyHeader === 'string' + ? varyHeader.includes('*') + : varyHeader.some(value => value.includes('*')) +} + /** * @param {string | string[]} varyHeader Vary header from the server * @param {Record} headers Request headers * @returns {Record} */ function parseVaryHeader (varyHeader, headers) { - if (typeof varyHeader === 'string' && varyHeader.includes('*')) { + if (varyHeaderHasWildcard(varyHeader)) { return headers } @@ -334,7 +349,7 @@ function parseVaryHeader (varyHeader, headers) { const varyingHeaders = typeof varyHeader === 'string' ? varyHeader.split(',') - : varyHeader + : varyHeader.flatMap(value => value.split(',')) for (const header of varyingHeaders) { const trimmedHeader = header.trim().toLowerCase() @@ -449,6 +464,7 @@ module.exports = { assertCacheValue, parseCacheControlHeader, parseVaryHeader, + varyHeaderHasWildcard, isEtagUsable, assertCacheMethods, assertCacheStore, diff --git a/test/cache-interceptor/utils.js b/test/cache-interceptor/utils.js index 103ee569449..7a4c399abcd 100644 --- a/test/cache-interceptor/utils.js +++ b/test/cache-interceptor/utils.js @@ -274,11 +274,29 @@ describe('parseVaryHeader', () => { }) }) + test('splits array entries on commas', () => { + const result = parseVaryHeader(['Accept-Encoding, X-User-Id', 'Accept-Language'], { + 'accept-encoding': 'gzip', + 'x-user-id': 'alice' + }) + deepStrictEqual(result, { + 'accept-encoding': 'gzip', + 'x-user-id': 'alice', + 'accept-language': null + }) + }) + test('preserves existing * behavior', () => { const headers = { accept: 'text/html' } const result = parseVaryHeader('*', headers) deepStrictEqual(result, headers) }) + + test('handles * in an array entry', () => { + const headers = { accept: 'text/html' } + const result = parseVaryHeader(['Accept-Encoding, *', 'Accept-Language'], headers) + deepStrictEqual(result, headers) + }) }) describe('isEtagUsable', () => { diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 26705ced8f3..4cc5d17d755 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -131,6 +131,78 @@ describe('Cache Interceptor', () => { } }) + test('vary directives spread over repeated headers are all honored', async () => { + let requestsToOrigin = 0 + const server = createServer((req, res) => { + requestsToOrigin++ + res.setHeader('cache-control', 's-maxage=10') + res.setHeader('vary', ['accept-encoding, a', 'accept-language']) + res.end(req.headers.a) + }).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') + + { + const res = await client.request({ origin: 'localhost', method: 'GET', path: '/', headers: { a: 'asd123' } }) + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd123') + } + + { + const res = await client.request({ origin: 'localhost', method: 'GET', path: '/', headers: { a: 'dsa321' } }) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'dsa321') + } + + { + const res = await client.request({ origin: 'localhost', method: 'GET', path: '/', headers: { a: 'asd123' } }) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd123') + } + }) + + test('does not cache a response whose repeated vary header contains *', async () => { + let requestsToOrigin = 0 + const server = createServer((req, res) => { + requestsToOrigin++ + res.setHeader('cache-control', 's-maxage=10') + res.setHeader('vary', ['accept-encoding, *', 'accept-language']) + 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') + + const request = { origin: 'localhost', method: 'GET', path: '/' } + + { + const res = await client.request(request) + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') + } + + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd') + } + }) + test('revalidates reponses with no-cache directive, regardless of cacheByDefault', async () => { let requestCount = 0 const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {