fix(cache): split repeated vary headers on commas

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
   }
 

lib/util/cache.js

@@ -320,21 +320,36 @@ 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<string, string | string[]>} headers Request headers
  * @returns {Record<string, string | string[]>}
  */
 function parseVaryHeader (varyHeader, headers) {
-  if (typeof varyHeader === 'string' && varyHeader.includes('*')) {
+  if (varyHeaderHasWildcard(varyHeader)) {
     return headers
   }
 
   const output = /** @type {Record<string, string | string[] | null>} */ ({})
 
   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,

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', () => {

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) => {