Skip to content

Commit cb105d7

Browse files
mcollinamcollina
committed
fix(cache): trim qualified field names
Signed-off-by: Matteo Collina <[email protected]>
1 parent 5655ea4 commit cb105d7

4 files changed

Lines changed: 92 additions & 3 deletions

File tree

‎lib/util/cache.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,10 @@ function parseCacheControlHeader (header) {
228228
headers[headers.length - 1] = lastHeader
229229
}
230230

231+
for (let j = 0; j < headers.length; j++) {
232+
headers[j] = headers[j].trim()
233+
}
234+
231235
if (key in output) {
232236
output[key] = output[key].concat(headers)
233237
} else {
@@ -236,10 +240,12 @@ function parseCacheControlHeader (header) {
236240
}
237241
} else {
238242
// Something like `no-cache="some-header"`
243+
const fieldName = value.trim()
244+
239245
if (key in output) {
240-
output[key] = output[key].concat(value)
246+
output[key] = output[key].concat(fieldName)
241247
} else {
242-
output[key] = [value]
248+
output[key] = [fieldName]
243249
}
244250
}
245251

‎test/cache-interceptor/utils.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,37 @@ describe('parseCacheControlHeader', () => {
9595
})
9696
})
9797

98+
test('trims qualified no-cache and private field names', () => {
99+
let directives = parseCacheControlHeader('private=" authorization"')
100+
deepStrictEqual(directives, {
101+
private: [
102+
'authorization'
103+
]
104+
})
105+
106+
directives = parseCacheControlHeader('no-cache="\tauthorization"')
107+
deepStrictEqual(directives, {
108+
'no-cache': [
109+
'authorization'
110+
]
111+
})
112+
113+
directives = parseCacheControlHeader('no-cache=authorization\t')
114+
deepStrictEqual(directives, {
115+
'no-cache': [
116+
'authorization'
117+
]
118+
})
119+
120+
directives = parseCacheControlHeader('private=" authorization, x-user\t"')
121+
deepStrictEqual(directives, {
122+
private: [
123+
'authorization',
124+
'x-user'
125+
]
126+
})
127+
})
128+
98129
test('private with headers', () => {
99130
let directives = parseCacheControlHeader('max-age=10, private=some-header, only-if-cached')
100131
deepStrictEqual(directives, {

‎test/interceptors/cache.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2287,6 +2287,56 @@ describe('Cache Interceptor', () => {
22872287
}
22882288
})
22892289

2290+
test('does not cache response when request has Authorization and qualified no-cache/private names Authorization with OWS', async () => {
2291+
for (const cacheControl of [
2292+
'public, max-age=60, private=" authorization"',
2293+
'public, max-age=60, no-cache="\tauthorization"',
2294+
'public, max-age=60, no-cache=authorization\t'
2295+
]) {
2296+
let requestsToOrigin = 0
2297+
const server = createServer({ joinDuplicateHeaders: true }, (_, res) => {
2298+
requestsToOrigin++
2299+
res.setHeader('cache-control', cacheControl)
2300+
res.end(`authenticated ${requestsToOrigin}`)
2301+
}).listen(0)
2302+
2303+
await once(server, 'listening')
2304+
2305+
const client = new Client(`http://localhost:${server.address().port}`)
2306+
.compose(interceptors.cache())
2307+
2308+
try {
2309+
const request = {
2310+
origin: 'localhost',
2311+
method: 'GET',
2312+
path: '/',
2313+
headers: {
2314+
authorization: 'Bearer token123'
2315+
}
2316+
}
2317+
2318+
{
2319+
const res = await client.request(request)
2320+
equal(requestsToOrigin, 1)
2321+
strictEqual(await res.body.text(), 'authenticated 1')
2322+
}
2323+
2324+
{
2325+
const res = await client.request({
2326+
origin: 'localhost',
2327+
method: 'GET',
2328+
path: '/'
2329+
})
2330+
equal(requestsToOrigin, 2)
2331+
strictEqual(await res.body.text(), 'authenticated 2')
2332+
}
2333+
} finally {
2334+
await client.close()
2335+
await new Promise(resolve => server.close(resolve))
2336+
}
2337+
}
2338+
})
2339+
22902340
test('does not cache response when request has Authorization and response only has max-age', async () => {
22912341
let requestsToOrigin = 0
22922342
const server = createServer({ joinDuplicateHeaders: true }, (_, res) => {

‎test/issue-803.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ test('https://github.com/nodejs/undici/issues/803', { timeout: 60000 }, async (t
1111
const SIZE = 5900373096
1212

1313
const server = createServer({ joinDuplicateHeaders: true }, async (req, res) => {
14-
const chunkSize = res.writableHighWaterMark << 5
14+
// Use large writes so this >32-bit Content-Length regression test does
15+
// not monopolize loopback I/O when the unit suite runs concurrently.
16+
const chunkSize = 16 * 1024 * 1024
1517
const parts = (SIZE / chunkSize) | 0
1618
const lastPartSize = SIZE % chunkSize
1719
const chunk = Buffer.allocUnsafe(chunkSize)

0 commit comments

Comments
 (0)