cache: ensure vary & revalidation headers are case-insensitive (#4112)

Closes #4103

Co-authored-by: alxndrsn <alxndrsn>

Author: alxndrsn

lib/interceptor/cache.js

@@ -6,7 +6,7 @@ const util = require('../core/util')
 const CacheHandler = require('../handler/cache-handler')
 const MemoryCacheStore = require('../cache/memory-cache-store')
 const CacheRevalidationHandler = require('../handler/cache-revalidation-handler')
-const { assertCacheStore, assertCacheMethods, makeCacheKey, parseCacheControlHeader } = require('../util/cache.js')
+const { assertCacheStore, assertCacheMethods, makeCacheKey, normaliseHeaders, parseCacheControlHeader } = require('../util/cache.js')
 const { AbortError } = require('../core/errors.js')
 
 /**
@@ -233,7 +233,7 @@ function handleResult (
     }
 
     let headers = {
-      ...opts.headers,
+      ...normaliseHeaders(opts),
       'if-modified-since': new Date(result.cachedAt).toUTCString()
     }
 

lib/util/cache.js

@@ -12,7 +12,21 @@ function makeCacheKey (opts) {
     throw new Error('opts.origin is undefined')
   }
 
-  /** @type {Record<string, string[] | string>} */
+  const headers = normaliseHeaders(opts)
+
+  return {
+    origin: opts.origin.toString(),
+    method: opts.method,
+    path: opts.path,
+    headers
+  }
+}
+
+/**
+ * @param {Record<string, string[] | string>}
+ * @return {Record<string, string[] | string>}
+ */
+function normaliseHeaders (opts) {
   let headers
   if (opts.headers == null) {
     headers = {}
@@ -38,12 +52,7 @@ function makeCacheKey (opts) {
     throw new Error('opts.headers is not an object')
   }
 
-  return {
-    origin: opts.origin.toString(),
-    method: opts.method,
-    path: opts.path,
-    headers
-  }
+  return headers
 }
 
 /**
@@ -350,6 +359,7 @@ function assertCacheMethods (methods, name = 'CacheMethods') {
 
 module.exports = {
   makeCacheKey,
+  normaliseHeaders,
   assertCacheKey,
   assertCacheValue,
   parseCacheControlHeader,

test/interceptors/cache.js

@@ -135,22 +135,30 @@ describe('Cache Interceptor', () => {
 
     let requestsToOrigin = 0
     let revalidationRequests = 0
+    let serverError
     const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
       res.setHeader('date', 0)
       res.setHeader('cache-control', 's-maxage=1, stale-while-revalidate=10')
 
-      if (req.headers['if-modified-since']) {
-        revalidationRequests++
+      try {
+        if (req.headers['if-modified-since']) {
+          equal(req.headers['if-modified-since'].length, 29)
+
+          revalidationRequests++
 
-        if (revalidationRequests === 2) {
-          res.end('updated')
+          if (revalidationRequests === 3) {
+            res.end('updated')
+          } else {
+            res.statusCode = 304
+            res.end()
+          }
         } else {
-          res.statusCode = 304
-          res.end()
+          requestsToOrigin++
+          res.end('asd')
         }
-      } else {
-        requestsToOrigin++
-        res.end('asd')
+      } catch (err) {
+        serverError = err
+        res.end()
       }
     }).listen(0)
 
@@ -188,6 +196,10 @@ describe('Cache Interceptor', () => {
     // Send initial request. This should reach the origin
     {
       const res = await client.request(request)
+      if (serverError) {
+        throw serverError
+      }
+
       equal(requestsToOrigin, 1)
       equal(revalidationRequests, 0)
       strictEqual(await res.body.text(), 'asd')
@@ -198,16 +210,42 @@ describe('Cache Interceptor', () => {
     // Response is now stale, the origin should get a revalidation request
     {
       const res = await client.request(request)
+      if (serverError) {
+        throw serverError
+      }
+
       equal(requestsToOrigin, 1)
       equal(revalidationRequests, 1)
       strictEqual(await res.body.text(), 'asd')
     }
 
+    // Response is still stale, extra header should be overwritten, and the
+    // origin should get a revalidation request
+    {
+      const res = await client.request({
+        ...request,
+        headers: {
+          'if-modified-SINCE': 'Thu, 01 Jan 1970 00:00:00 GMT'
+        }
+      })
+      if (serverError) {
+        throw serverError
+      }
+
+      equal(requestsToOrigin, 1)
+      equal(revalidationRequests, 2)
+      strictEqual(await res.body.text(), 'asd')
+    }
+
     // Response is still stale, but revalidation should fail now.
     {
       const res = await client.request(request)
+      if (serverError) {
+        throw serverError
+      }
+
       equal(requestsToOrigin, 1)
-      equal(revalidationRequests, 2)
+      equal(revalidationRequests, 3)
       strictEqual(await res.body.text(), 'updated')
     }
   })
@@ -230,7 +268,7 @@ describe('Cache Interceptor', () => {
 
           equal(req.headers['if-none-match'], '"asd123"')
 
-          if (revalidationRequests === 2) {
+          if (revalidationRequests === 3) {
             res.end('updated')
           } else {
             res.statusCode = 304
@@ -296,6 +334,24 @@ describe('Cache Interceptor', () => {
       strictEqual(await res.body.text(), 'asd')
     }
 
+    // Response is still stale, extra headers should be overwritten, and the
+    // origin should get a revalidation request
+    {
+      const res = await client.request({
+        ...request,
+        headers: {
+          'if-NONE-match': '"nonsense-etag"'
+        }
+      })
+      if (serverError) {
+        throw serverError
+      }
+
+      equal(requestsToOrigin, 1)
+      equal(revalidationRequests, 2)
+      strictEqual(await res.body.text(), 'asd')
+    }
+
     // Response is still stale, but revalidation should fail now.
     {
       const res = await client.request(request)
@@ -304,7 +360,7 @@ describe('Cache Interceptor', () => {
       }
 
       equal(requestsToOrigin, 1)
-      equal(revalidationRequests, 2)
+      equal(revalidationRequests, 3)
       strictEqual(await res.body.text(), 'updated')
     }
   })
@@ -327,13 +383,13 @@ describe('Cache Interceptor', () => {
         if (ifNoneMatch) {
           revalidationRequests++
           notEqual(req.headers.a, undefined)
-          notEqual(req.headers.b, undefined)
+          notEqual(req.headers['b-mixed-case'], undefined)
 
           res.statusCode = 304
           res.end()
         } else {
           requestsToOrigin++
-          res.setHeader('vary', 'a, b')
+          res.setHeader('vary', 'a, B-MIXED-CASe')
           res.setHeader('etag', '"asd"')
           res.end('asd')
         }
@@ -360,15 +416,17 @@ describe('Cache Interceptor', () => {
     const request = {
       origin: 'localhost',
       path: '/',
-      method: 'GET',
-      headers: {
-        a: 'asd',
-        b: 'asd'
-      }
+      method: 'GET'
     }
 
     {
-      const response = await client.request(request)
+      const response = await client.request({
+        ...request,
+        headers: {
+          a: 'asd',
+          'b-Mixed-case': 'asd'
+        }
+      })
       if (serverError) {
         throw serverError
       }
@@ -380,7 +438,13 @@ describe('Cache Interceptor', () => {
     clock.tick(1500)
 
     {
-      const response = await client.request(request)
+      const response = await client.request({
+        ...request,
+        headers: {
+          a: 'asd',
+          'B-mixed-CASE': 'asd'
+        }
+      })
       if (serverError) {
         throw serverError
       }