Skip to content

Commit 85a2405

Browse files
committed
fix(cache): trim qualified field names
Signed-off-by: Matteo Collina <hello@matteocollina.com> (cherry picked from commit cb105d7)
1 parent d0574cc commit 85a2405

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
@@ -2208,6 +2208,56 @@ describe('Cache Interceptor', () => {
22082208
}
22092209
})
22102210

2211+
test('does not cache response when request has Authorization and qualified no-cache/private names Authorization with OWS', async () => {
2212+
for (const cacheControl of [
2213+
'public, max-age=60, private=" authorization"',
2214+
'public, max-age=60, no-cache="\tauthorization"',
2215+
'public, max-age=60, no-cache=authorization\t'
2216+
]) {
2217+
let requestsToOrigin = 0
2218+
const server = createServer({ joinDuplicateHeaders: true }, (_, res) => {
2219+
requestsToOrigin++
2220+
res.setHeader('cache-control', cacheControl)
2221+
res.end(`authenticated ${requestsToOrigin}`)
2222+
}).listen(0)
2223+
2224+
await once(server, 'listening')
2225+
2226+
const client = new Client(`http://localhost:${server.address().port}`)
2227+
.compose(interceptors.cache())
2228+
2229+
try {
2230+
const request = {
2231+
origin: 'localhost',
2232+
method: 'GET',
2233+
path: '/',
2234+
headers: {
2235+
authorization: 'Bearer token123'
2236+
}
2237+
}
2238+
2239+
{
2240+
const res = await client.request(request)
2241+
equal(requestsToOrigin, 1)
2242+
strictEqual(await res.body.text(), 'authenticated 1')
2243+
}
2244+
2245+
{
2246+
const res = await client.request({
2247+
origin: 'localhost',
2248+
method: 'GET',
2249+
path: '/'
2250+
})
2251+
equal(requestsToOrigin, 2)
2252+
strictEqual(await res.body.text(), 'authenticated 2')
2253+
}
2254+
} finally {
2255+
await client.close()
2256+
await new Promise(resolve => server.close(resolve))
2257+
}
2258+
}
2259+
})
2260+
22112261
test('does not cache response when request has Authorization and response only has max-age', async () => {
22122262
let requestsToOrigin = 0
22132263
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)