diff --git a/lib/util/cache.js b/lib/util/cache.js index 5968f4efe5f..2da2c044dbd 100644 --- a/lib/util/cache.js +++ b/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 = { diff --git a/test/interceptors/deduplicate.js b/test/interceptors/deduplicate.js index 3a6546379e5..f0d9ef97852 100644 --- a/test/interceptors/deduplicate.js +++ b/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) + }) })