Skip to content

Commit 26fe391

Browse files
fix caching of not understod statuses
finish fixing #4334
1 parent 0797def commit 26fe391

2 files changed

Lines changed: 121 additions & 2 deletions

File tree

lib/handler/cache-handler.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ const HEURISTICALLY_CACHEABLE_STATUS_CODES = [
1515
200, 203, 204, 206, 300, 301, 308, 404, 405, 410, 414, 501
1616
]
1717

18+
// Status codes which semantic is not handled by the cache
19+
// https://datatracker.ietf.org/doc/html/rfc9111#section-3
20+
// This list should not grow beyond 206 and 304 unless the RFC is updated
21+
// by a newer one including more. Please introduce another list if
22+
// implementing caching of responses with the 'must-understand' directive.
23+
const NOT_UNDERSTOOD_STATUS_CODES = [
24+
206, 304
25+
]
26+
1827
const MAX_RESPONSE_AGE = 2147483647000
1928

2029
/**
@@ -241,8 +250,8 @@ class CacheHandler {
241250
* @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives
242251
*/
243252
function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives) {
244-
// Status code must be final.
245-
if (statusCode < 200) {
253+
// Status code must be final and understood.
254+
if (statusCode < 200 || NOT_UNDERSTOOD_STATUS_CODES.includes(statusCode)) {
246255
return false
247256
}
248257
// Responses with neither status codes that are heuristically cacheable, nor explicit caching directives,

test/interceptors/cache.js

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,4 +1543,114 @@ describe('Cache Interceptor', () => {
15431543
}
15441544
})
15451545
}
1546+
1547+
test('discriminates caching of range requests, or does not cache them', async () => {
1548+
let requestsToOrigin = 0
1549+
const body = 'Fake range request response'
1550+
const code = 206
1551+
const server = createServer({ joinDuplicateHeaders: true }, (_, res) => {
1552+
requestsToOrigin++
1553+
res.statusCode = code
1554+
res.setHeader('cache-control', 'public, max-age=60')
1555+
res.end(body)
1556+
}).listen(0)
1557+
1558+
const client = new Client(`http://localhost:${server.address().port}`)
1559+
.compose(interceptors.cache())
1560+
1561+
after(async () => {
1562+
server.close()
1563+
await client.close()
1564+
})
1565+
1566+
await once(server, 'listening')
1567+
1568+
equal(requestsToOrigin, 0)
1569+
1570+
const request = {
1571+
origin: 'localhost',
1572+
method: 'GET',
1573+
path: '/',
1574+
headers: {
1575+
range: 'bytes=10-'
1576+
}
1577+
}
1578+
1579+
// First request should hit the origin
1580+
{
1581+
const res = await client.request(request)
1582+
equal(requestsToOrigin, 1)
1583+
equal(res.statusCode, code)
1584+
strictEqual(await res.body.text(), body)
1585+
}
1586+
1587+
// Second request with different range should hit the origin too
1588+
request.headers.range = 'bytes=5-'
1589+
{
1590+
const res = await client.request(request)
1591+
equal(requestsToOrigin, 2)
1592+
equal(res.statusCode, code)
1593+
strictEqual(await res.body.text(), body)
1594+
}
1595+
})
1596+
1597+
test('discriminates caching of conditionnal requests, or does not cache them', async () => {
1598+
let requestsToOrigin = 0
1599+
const body = ''
1600+
const code = 304
1601+
const server = createServer({ joinDuplicateHeaders: true }, (_, res) => {
1602+
requestsToOrigin++
1603+
res.statusCode = code
1604+
res.setHeader('cache-control', 'public, max-age=60')
1605+
res.end(body)
1606+
}).listen(0)
1607+
1608+
const client = new Client(`http://localhost:${server.address().port}`)
1609+
.compose(interceptors.cache())
1610+
1611+
after(async () => {
1612+
server.close()
1613+
await client.close()
1614+
})
1615+
1616+
await once(server, 'listening')
1617+
1618+
equal(requestsToOrigin, 0)
1619+
1620+
const request = {
1621+
origin: 'localhost',
1622+
method: 'GET',
1623+
path: '/',
1624+
headers: {
1625+
'if-modified-since': new Date().toUTCString(),
1626+
'if-none-match': 'some-etag'
1627+
}
1628+
}
1629+
1630+
// First request should hit the origin
1631+
{
1632+
const res = await client.request(request)
1633+
equal(requestsToOrigin, 1)
1634+
equal(res.statusCode, code)
1635+
strictEqual(await res.body.text(), body)
1636+
}
1637+
1638+
// Second request with different etag should hit the origin too
1639+
request.headers['if-none-match'] = 'another-etag'
1640+
{
1641+
const res = await client.request(request)
1642+
equal(requestsToOrigin, 2)
1643+
equal(res.statusCode, code)
1644+
strictEqual(await res.body.text(), body)
1645+
}
1646+
1647+
// Third request with different since should hit the origin too
1648+
request.headers['if-modified-since'] = new Date(0).toUTCString()
1649+
{
1650+
const res = await client.request(request)
1651+
equal(requestsToOrigin, 3)
1652+
equal(res.statusCode, code)
1653+
strictEqual(await res.body.text(), body)
1654+
}
1655+
})
15461656
})

0 commit comments

Comments
 (0)