Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lib/util/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 = {
Expand Down
61 changes: 60 additions & 1 deletion test/interceptors/deduplicate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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)
})
})
Loading