diff --git a/lib/util/cache.js b/lib/util/cache.js index 5968f4efe5f..499847e209e 100644 --- a/lib/util/cache.js +++ b/lib/util/cache.js @@ -374,9 +374,8 @@ 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}` + /** @type {[string, string | string[]][]} */ + const headers = [] if (cacheKey.headers) { const sortedHeaders = Object.keys(cacheKey.headers).sort() @@ -385,12 +384,17 @@ function makeDeduplicationKey (cacheKey, excludeHeaders) { if (excludeHeaders?.has(header.toLowerCase())) { continue } - const value = cacheKey.headers[header] - key += `:${header}=${Array.isArray(value) ? value.join(',') : value}` + + headers.push([header, cacheKey.headers[header]]) } } - return key + return JSON.stringify([ + cacheKey.origin, + cacheKey.method, + cacheKey.path, + headers + ]) } module.exports = { diff --git a/test/cache-interceptor/cache-utils.js b/test/cache-interceptor/cache-utils.js index 7b66e66d3aa..061f7e34992 100644 --- a/test/cache-interceptor/cache-utils.js +++ b/test/cache-interceptor/cache-utils.js @@ -2,7 +2,7 @@ const { tspl } = require('@matteo.collina/tspl') const { test } = require('node:test') -const { normalizeHeaders } = require('../../lib/util/cache') +const { makeDeduplicationKey, normalizeHeaders } = require('../../lib/util/cache') test('normalizeHeaders handles plain object headers with polluted Object.prototype[Symbol.iterator]', (t) => { const { strictEqual } = tspl(t, { plan: 2 }) @@ -42,3 +42,28 @@ test('normalizeHeaders handles headers from Map', (t) => { strictEqual(headers['x-test'], 'ok') }) + +test('makeDeduplicationKey does not collide when headers contain delimiters', (t) => { + const { strictEqual } = tspl(t, { plan: 1 }) + + const keyWithDelimitedValue = makeDeduplicationKey({ + origin: 'https://example.com', + method: 'GET', + path: '/', + headers: { + a: 'x:b=y' + } + }) + + const keyWithExtraHeader = makeDeduplicationKey({ + origin: 'https://example.com', + method: 'GET', + path: '/', + headers: { + a: 'x', + b: 'y' + } + }) + + strictEqual(keyWithDelimitedValue === keyWithExtraHeader, false) +}) diff --git a/test/interceptors/deduplicate.js b/test/interceptors/deduplicate.js index 3a6546379e5..e28b3ddccf1 100644 --- a/test/interceptors/deduplicate.js +++ b/test/interceptors/deduplicate.js @@ -154,6 +154,55 @@ describe('Deduplicate Interceptor', () => { strictEqual(body2, 'response for br') }) + test('does not deduplicate requests when header delimiters would previously collide', async () => { + let requestsToOrigin = 0 + const server = createServer({ joinDuplicateHeaders: true }, async (req, res) => { + requestsToOrigin++ + await sleep(100) + res.end(`a=${req.headers.a};b=${req.headers.b ?? ''}`) + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.deduplicate()) + + after(async () => { + server.close() + await client.close() + }) + + await once(server, 'listening') + + const [res1, res2] = await Promise.all([ + client.request({ + origin: 'localhost', + method: 'GET', + path: '/', + headers: { + a: 'x:b=y' + } + }), + client.request({ + origin: 'localhost', + method: 'GET', + path: '/', + headers: { + a: 'x', + b: 'y' + } + }) + ]) + + strictEqual(requestsToOrigin, 2) + + const [body1, body2] = await Promise.all([ + res1.body.text(), + res2.body.text() + ]) + + strictEqual(body1, 'a=x:b=y;b=') + strictEqual(body2, 'a=x;b=y') + }) + test('does not deduplicate requests with different paths', async () => { let requestsToOrigin = 0 const server = createServer({ joinDuplicateHeaders: true }, async (req, res) => {