fix: prevent cache dedup key collision via unescaped delimiters (#5012) (#5013)

Author: eddieran

lib/util/cache.js

@@ -374,9 +374,11 @@ function assertCacheMethods (methods, name = 'CacheMethods') {
  * @returns {string}
  */
 function makeDeduplicationKey (cacheKey, excludeHeaders) {
-  // Create a deterministic string key from the cache key
-  // Include origin, method, path, and sorted headers
-  let key = `${cacheKey.origin}:${cacheKey.method}:${cacheKey.path}`
+  // Use JSON.stringify to produce a collision-resistant key.
+  // Previous format used `:` and `=` delimiters without escaping, which
+  // allowed different header sets to produce identical keys (e.g.
+  // {a:"x:b=y"} vs {a:"x", b:"y"}). See: https://github.com/nodejs/undici/issues/5012
+  const headers = {}
 
   if (cacheKey.headers) {
     const sortedHeaders = Object.keys(cacheKey.headers).sort()
@@ -385,12 +387,11 @@ function makeDeduplicationKey (cacheKey, excludeHeaders) {
       if (excludeHeaders?.has(header.toLowerCase())) {
         continue
       }
-      const value = cacheKey.headers[header]
-      key += `:${header}=${Array.isArray(value) ? value.join(',') : value}`
+      headers[header] = cacheKey.headers[header]
     }
   }
 
-  return key
+  return JSON.stringify([cacheKey.origin, cacheKey.method, cacheKey.path, headers])
 }
 
 module.exports = {

test/interceptors/deduplicate.js

@@ -3,10 +3,11 @@
 const { createServer } = require('node:http')
 const { describe, test, after } = require('node:test')
 const { once } = require('node:events')
-const { strictEqual } = require('node:assert')
+const { strictEqual, notStrictEqual } = require('node:assert')
 const { setTimeout: sleep } = require('node:timers/promises')
 const diagnosticsChannel = require('node:diagnostics_channel')
 const { Client, interceptors } = require('../../index')
+const { makeDeduplicationKey } = require('../../lib/util/cache')
 
 describe('Deduplicate Interceptor', () => {
   test('deduplicates concurrent requests for the same resource', async () => {
@@ -1291,4 +1292,62 @@ describe('Deduplicate Interceptor', () => {
       message: 'expected opts.excludeHeaderNames to be an array, got string'
     })
   })
+
+  test('makeDeduplicationKey does not collide when header values contain delimiters', () => {
+    // Regression test for https://github.com/nodejs/undici/issues/5012
+    // Previously, headers {a:"x:b=y"} and {a:"x", b:"y"} produced the same key
+    const key1 = makeDeduplicationKey({
+      origin: 'https://example.com',
+      method: 'GET',
+      path: '/',
+      headers: { a: 'x:b=y' }
+    })
+
+    const key2 = makeDeduplicationKey({
+      origin: 'https://example.com',
+      method: 'GET',
+      path: '/',
+      headers: { a: 'x', b: 'y' }
+    })
+
+    notStrictEqual(key1, key2)
+  })
+
+  test('makeDeduplicationKey produces same key for identical headers', () => {
+    const key1 = makeDeduplicationKey({
+      origin: 'https://example.com',
+      method: 'GET',
+      path: '/',
+      headers: { b: '2', a: '1' }
+    })
+
+    const key2 = makeDeduplicationKey({
+      origin: 'https://example.com',
+      method: 'GET',
+      path: '/',
+      headers: { a: '1', b: '2' }
+    })
+
+    strictEqual(key1, key2)
+  })
+
+  test('makeDeduplicationKey respects excludeHeaders', () => {
+    const excludeHeaders = new Set(['x-request-id'])
+
+    const key1 = makeDeduplicationKey({
+      origin: 'https://example.com',
+      method: 'GET',
+      path: '/',
+      headers: { accept: 'text/html', 'x-request-id': '111' }
+    }, excludeHeaders)
+
+    const key2 = makeDeduplicationKey({
+      origin: 'https://example.com',
+      method: 'GET',
+      path: '/',
+      headers: { accept: 'text/html', 'x-request-id': '222' }
+    }, excludeHeaders)
+
+    strictEqual(key1, key2)
+  })
 })