fix: handle missing vary header values

Co-authored-by: flakey5 <[email protected]>

Author: gurgunday

lib/cache/memory-cache-store.js

@@ -79,7 +79,7 @@ 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 => entry.vary[headerName] === key.headers[headerName]))
     ))
 
     return entry == null

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<string, string | string[]>} */ ({})
+  const output = /** @type {Record<string, string | string[] | undefined>} */ ({})
 
   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]
   }
 
   return output

test/issue-3959.js

@@ -0,0 +1,55 @@
+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', () => {
+  test('should cache response when Vary header exists but request header is missing', async () => {
+    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 store = new MemoryCacheStore()
+    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)
+
+    after(() => server.close())
+  })
+})

test/types/cache-interceptor.test-d.ts

@@ -78,6 +78,45 @@ expectNotAssignable<CacheInterceptor.CacheValue>({
   deleteAt: ''
 })
 
+expectAssignable<CacheInterceptor.CacheValue>({
+  statusCode: 200,
+  statusMessage: 'OK',
+  headers: {},
+  vary: {
+    'accept-encoding': undefined,
+    authorization: 'example-value'
+  },
+  cachedAt: 0,
+  staleAt: 0,
+  deleteAt: 0
+})
+
+expectAssignable<CacheInterceptor.CacheValue>({
+  statusCode: 200,
+  statusMessage: 'OK',
+  headers: {},
+  vary: {
+    'accept-encoding': undefined,
+    authorization: undefined
+  },
+  cachedAt: 0,
+  staleAt: 0,
+  deleteAt: 0
+})
+
+expectNotAssignable<CacheInterceptor.CacheValue>({
+  statusCode: 200,
+  statusMessage: 'OK',
+  headers: {},
+  vary: {
+    'accept-encoding': false,
+    authorization: 'example-value'
+  },
+  cachedAt: 0,
+  staleAt: 0,
+  deleteAt: 0
+})
+
 expectAssignable<CacheInterceptor.MemoryCacheStoreOpts>({})
 expectAssignable<CacheInterceptor.MemoryCacheStoreOpts>({
   maxSize: 0

test/utils/cache.js

@@ -0,0 +1,41 @@
+'use strict'
+
+const { describe, test } = require('node:test')
+const assert = require('node:assert')
+const { parseVaryHeader } = require('../../lib/util/cache.js')
+
+describe('parseVaryHeader', () => {
+  test('handles missing headers with undefined', () => {
+    const result = parseVaryHeader('Accept-Encoding, Authorization', {})
+    assert.deepStrictEqual(result, {
+      'accept-encoding': undefined,
+      authorization: undefined
+    })
+  })
+
+  test('handles mix of present and missing headers', () => {
+    const result = parseVaryHeader('Accept-Encoding, Authorization', {
+      authorization: 'example-value'
+    })
+    assert.deepStrictEqual(result, {
+      'accept-encoding': undefined,
+      authorization: 'example-value'
+    })
+  })
+
+  test('handles array input', () => {
+    const result = parseVaryHeader(['Accept-Encoding', 'Authorization'], {
+      'accept-encoding': 'gzip'
+    })
+    assert.deepStrictEqual(result, {
+      'accept-encoding': 'gzip',
+      authorization: undefined
+    })
+  })
+
+  test('preserves existing * behavior', () => {
+    const headers = { accept: 'text/html' }
+    const result = parseVaryHeader('*', headers)
+    assert.deepStrictEqual(result, headers)
+  })
+})

types/cache-interceptor.d.ts

@@ -70,7 +70,7 @@ declare namespace CacheHandler {
     statusCode: number
     statusMessage: string
     headers: Record<string, string | string[]>
-    vary?: Record<string, string | string[]>
+    vary?: Record<string, string | string[] | undefined>
     etag?: string
     cacheControlDirectives?: CacheControlDirectives
     cachedAt: number
@@ -88,7 +88,7 @@ declare namespace CacheHandler {
     statusCode: number
     statusMessage: string
     headers: Record<string, string | string[]>
-    vary?: Record<string, string | string[]>
+    vary?: Record<string, string | string[] | undefined>
     etag?: string
     body?: Readable | Iterable<Buffer> | AsyncIterable<Buffer> | Buffer | Iterable<string> | AsyncIterable<string> | string
     cacheControlDirectives: CacheControlDirectives,