diff --git a/lib/cache/memory-cache-store.js b/lib/cache/memory-cache-store.js index dd5ac00b5a3..6fa7356b366 100644 --- a/lib/cache/memory-cache-store.js +++ b/lib/cache/memory-cache-store.js @@ -79,7 +79,13 @@ class MemoryCacheStore { const entry = this.#entries.get(topLevelKey)?.find((entry) => ( entry.deleteAt > now && entry.method === key.method && - (entry.vary == null || Object.keys(entry.vary).every(headerName => entry.vary[headerName] === key.headers?.[headerName])) + (entry.vary == null || Object.keys(entry.vary).every(headerName => { + if (entry.vary[headerName] === null) { + return key.headers[headerName] === undefined + } + + return entry.vary[headerName] === key.headers[headerName] + })) )) return entry == null diff --git a/lib/cache/sqlite-cache-store.js b/lib/cache/sqlite-cache-store.js index 682aac31326..e027cff84ea 100644 --- a/lib/cache/sqlite-cache-store.js +++ b/lib/cache/sqlite-cache-store.js @@ -411,10 +411,6 @@ module.exports = class SqliteCacheStore { let matches = true if (value.vary) { - if (!headers) { - return undefined - } - const vary = JSON.parse(value.vary) for (const header in vary) { @@ -440,18 +436,21 @@ module.exports = class SqliteCacheStore { * @returns {boolean} */ function headerValueEquals (lhs, rhs) { + if (lhs == null && rhs == null) { + return true + } + + if ((lhs == null && rhs != null) || + (lhs != null && rhs == null)) { + return false + } + if (Array.isArray(lhs) && Array.isArray(rhs)) { if (lhs.length !== rhs.length) { return false } - for (let i = 0; i < lhs.length; i++) { - if (rhs.includes(lhs[i])) { - return false - } - } - - return true + return lhs.every((x, i) => x === rhs[i]) } return lhs === rhs diff --git a/lib/util/cache.js b/lib/util/cache.js index 35c53512b2a..41d1c1b7656 100644 --- a/lib/util/cache.js +++ b/lib/util/cache.js @@ -26,10 +26,14 @@ function makeCacheKey (opts) { if (typeof key !== 'string' || typeof val !== 'string') { throw new Error('opts.headers is not a valid header map') } - headers[key] = val + headers[key.toLowerCase()] = val } } else if (typeof opts.headers === 'object') { - headers = opts.headers + headers = {} + + for (const key of Object.keys(opts.headers)) { + headers[key.toLowerCase()] = opts.headers[key] + } } else { throw new Error('opts.headers is not an object') } @@ -260,19 +264,16 @@ function parseVaryHeader (varyHeader, headers) { return headers } - const output = /** @type {Record} */ ({}) + const output = /** @type {Record} */ ({}) const varyingHeaders = typeof varyHeader === 'string' ? varyHeader.split(',') : varyHeader + for (const header of varyingHeaders) { const trimmedHeader = header.trim().toLowerCase() - if (headers[trimmedHeader]) { - output[trimmedHeader] = headers[trimmedHeader] - } else { - return undefined - } + output[trimmedHeader] = headers[trimmedHeader] ?? null } return output diff --git a/test/cache-interceptor/utils.js b/test/cache-interceptor/utils.js index 2fc8bc17dea..2ae75d15489 100644 --- a/test/cache-interceptor/utils.js +++ b/test/cache-interceptor/utils.js @@ -214,6 +214,40 @@ describe('parseVaryHeader', () => { 'another-one': '123' }) }) + + test('handles missing headers with null', () => { + const result = parseVaryHeader('Accept-Encoding, Authorization', {}) + deepStrictEqual(result, { + 'accept-encoding': null, + authorization: null + }) + }) + + test('handles mix of present and missing headers', () => { + const result = parseVaryHeader('Accept-Encoding, Authorization', { + authorization: 'example-value' + }) + deepStrictEqual(result, { + 'accept-encoding': null, + authorization: 'example-value' + }) + }) + + test('handles array input', () => { + const result = parseVaryHeader(['Accept-Encoding', 'Authorization'], { + 'accept-encoding': 'gzip' + }) + deepStrictEqual(result, { + 'accept-encoding': 'gzip', + authorization: null + }) + }) + + test('preserves existing * behavior', () => { + const headers = { accept: 'text/html' } + const result = parseVaryHeader('*', headers) + deepStrictEqual(result, headers) + }) }) describe('isEtagUsable', () => { diff --git a/test/issue-3959.js b/test/issue-3959.js new file mode 100644 index 00000000000..2d83a6030e0 --- /dev/null +++ b/test/issue-3959.js @@ -0,0 +1,65 @@ +const { describe, test, after } = require('node:test') +const assert = require('node:assert') +const { createServer } = require('node:http') +const MemoryCacheStore = require('../lib/cache/memory-cache-store.js') +const { request, Agent, setGlobalDispatcher } = require('..') +const { interceptors } = require('..') + +describe('Cache with Vary headers', () => { + async function runCacheTest (store) { + let requestCount = 0 + const server = createServer((req, res) => { + requestCount++ + res.setHeader('Vary', 'Accept-Encoding') + res.setHeader('Cache-Control', 'max-age=60') + res.end(`Request count: ${requestCount}`) + }) + + await new Promise(resolve => server.listen(0, resolve)) + const port = server.address().port + const url = `http://localhost:${port}` + + const agent = new Agent() + setGlobalDispatcher( + agent.compose( + interceptors.cache({ + store, + cacheByDefault: 1000, + methods: ['GET'] + }) + ) + ) + + const res1 = await request(url) + const body1 = await res1.body.text() + assert.strictEqual(body1, 'Request count: 1') + assert.strictEqual(requestCount, 1) + + const res2 = await request(url) + const body2 = await res2.body.text() + assert.strictEqual(body2, 'Request count: 1') + assert.strictEqual(requestCount, 1) + + const res3 = await request(url, { + headers: { + 'Accept-Encoding': 'gzip' + } + }) + const body3 = await res3.body.text() + assert.strictEqual(body3, 'Request count: 2') + assert.strictEqual(requestCount, 2) + + await new Promise(resolve => server.close(resolve)) + } + + test('should cache response with MemoryCacheStore when Vary header exists but request header is missing', async () => { + await runCacheTest(new MemoryCacheStore()) + }) + + test('should cache response with SqliteCacheStore when Vary header exists but request header is missing', { skip: process.versions.node < '22' }, async () => { + const SqliteCacheStore = require('../lib/cache/sqlite-cache-store.js') + const sqliteStore = new SqliteCacheStore() + await runCacheTest(sqliteStore) + after(() => sqliteStore.close()) + }) +}) diff --git a/test/types/cache-interceptor.test-d.ts b/test/types/cache-interceptor.test-d.ts index 6fb66955de5..ce9543646cb 100644 --- a/test/types/cache-interceptor.test-d.ts +++ b/test/types/cache-interceptor.test-d.ts @@ -78,6 +78,45 @@ expectNotAssignable({ deleteAt: '' }) +expectAssignable({ + statusCode: 200, + statusMessage: 'OK', + headers: {}, + vary: { + 'accept-encoding': null, + authorization: 'example-value' + }, + cachedAt: 0, + staleAt: 0, + deleteAt: 0 +}) + +expectAssignable({ + statusCode: 200, + statusMessage: 'OK', + headers: {}, + vary: { + 'accept-encoding': null, + authorization: null + }, + cachedAt: 0, + staleAt: 0, + deleteAt: 0 +}) + +expectNotAssignable({ + statusCode: 200, + statusMessage: 'OK', + headers: {}, + vary: { + 'accept-encoding': undefined, + authorization: 'example-value' + }, + cachedAt: 0, + staleAt: 0, + deleteAt: 0 +}) + expectAssignable({}) expectAssignable({ maxSize: 0 diff --git a/types/cache-interceptor.d.ts b/types/cache-interceptor.d.ts index 1713ca7e747..e53be60a611 100644 --- a/types/cache-interceptor.d.ts +++ b/types/cache-interceptor.d.ts @@ -70,7 +70,7 @@ declare namespace CacheHandler { statusCode: number statusMessage: string headers: Record - vary?: Record + vary?: Record etag?: string cacheControlDirectives?: CacheControlDirectives cachedAt: number @@ -88,7 +88,7 @@ declare namespace CacheHandler { statusCode: number statusMessage: string headers: Record - vary?: Record + vary?: Record etag?: string body?: Readable | Iterable | AsyncIterable | Buffer | Iterable | AsyncIterable | string cacheControlDirectives: CacheControlDirectives,