Repository: nodejs/undici

Problem statement:
Cache can add duplicate if-none-match, if-modified-since & varied headers (case-sensitivity)
## Bug Description

The code at https://github.com/nodejs/undici/blob/e43d01552adf78e57211f188ad6ce0f0a0529ab7/lib/interceptor/cache.js#L235-L249 will overwrite lower-cased user-defined headers, but duplicate non-lower-cased headers.

## Reproducible By

```js
import { Agent, fetch, interceptors } from 'undici';
import express from 'express';

const app = express();
app.get('/', (req, res) => {
  res.set('Cache-Control', 'max-age=1');
  res.set('ETag', '"asdf1234"');
  res.send({ requestHeaders:req.headers });
});

const dispatcher = new Agent().compose(interceptors.cache({
  cacheByDefault: Number.MAX_SAFE_INTEGER,
  type: 'private',
}));

let prevEtag;

const s = app.listen(4444, async () => {
  for(let i=1; i<5; ++i) {
    const headers = {};
    if(i == 2) headers['If-None-Match'] = prevEtag;
    if(i == 3) headers['if-none-match'] = prevEtag;

    const res = await fetch('http://localhost:4444', { dispatcher, headers });
  
    prevEtag = res.headers.get('etag');
  
    console.log(i, 'res.body:', (await res.json()).requestHeaders['if-none-match']);

    await new Promise(resolve => setTimeout(resolve, 2000));
  }

  s.close();
});
```

### Output:

```
1 res.body: undefined
2 res.body: "asdf1234", "asdf1234"
3 res.body: "asdf1234"
4 res.body: "asdf1234"
```

## Expected Behavior

<!-- A clear and concise description of what you expected to happen. -->
Either:

1. cache code defers to user-defined header values, or
2. cache code consistently overwrites user-defined header values

## Logs & Screenshots

<!-- If applicable, add screenshots to help explain your problem, or 
alternatively add your console logs here. -->

## Environment

<!-- This is just your OS and environment information [e.g. Ubuntu 18.04 LTS,
Node v14.14.0] -->
```sh
$ echo "node: $(node --version)"; echo "undici: $(jq -r .version ./node_modules/undici/package.json)"; echo "express: $(jq -r .version ./node_modules/express/package.json)"
node: v22.14.0
undici: 7.5.0
express: 4.21.2
```

### Additional context

<!-- Add any other context about the problem here. -->
There are some other issues relating to cache & case-sensitivity of headers:

* https://github.com/nodejs/undici/issues/3904
* https://github.com/nodejs/undici/pull/3990
* https://github.com/nodejs/undici/pull/4031



Hints:
Thanks for the report!

>cache code consistently overwrites user-defined header values

I believe this should be the way to move forward, as these are handled by the interceptor itself and overwrite them can lead to undefined behaviour.

Would you like to send a PR to address that?

> Would you like to send a PR to address that?

I'll take a look :+1:

Patch:
diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js
--- a/lib/interceptor/cache.js
+++ b/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()
     }
 
diff --git a/lib/util/cache.js b/lib/util/cache.js
--- a/lib/util/cache.js
+++ b/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 patch:
diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js
--- a/test/interceptors/cache.js
+++ b/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
       }