cache: 304 revalidation response never updates the stored entry on the synchronous path (RFC 9111 §4.3.4) — entry stays stale forever

[jeswr]

Bug Description

#4617 (fixing #4596) taught CacheHandler to merge 304 responses into the stored entry — but
that code path is only reachable where CacheHandler sees the response directly (the
stale-while-revalidate background refresh). On the synchronous revalidation path,
CacheRevalidationHandler short-circuits on success (statusCode === 304 →
callback(true) → return true, lib/handler/cache-revalidation-handler.js:72-88) and never
forwards anything to the wrapped CacheHandler, so:

1. Headers carried by the 304 (notably a new Cache-Control/Expires/Date) are discarded.
   RFC 9111 §4.3.4: the cache MUST update the stored response with the 304's header fields and
   recompute freshness. An origin extending freshness via 304 + Cache-Control: max-age=60
   (standard Varnish/CDN behavior) gets zero benefit: undici revalidates again on the very next
   request, every time, forever.
2. Even a bare 304 leaves the entry stale — repeated per-request conditional traffic instead of
   a fresh window.
3. The revalidated response is served with Warning: 110 - "response is stale" although it has
   just been successfully validated (acknowledged TODO at lib/interceptor/cache.js:378; the
   Warning header is also obsolete per RFC 9111 §5.5).

The five 304-etag-update-response-* cache-tests are currently in the conformance skip list
with the comment "We're not caching 304s currently" (test/cache-interceptor/cache-tests.mjs:76-81);
fixing this would un-skip them.

Reproducible By

const { Agent, interceptors, cacheStores, request } = require('undici')
const http = require('node:http')
const { setTimeout: sleep } = require('node:timers/promises')

let hits = 0
const server = http.createServer((req, res) => {
  hits++
  if (req.headers['if-none-match'] === '"v1"') {
    // Extend freshness for a minute — per §4.3.4 the client must apply this
    res.writeHead(304, { 'cache-control': 'max-age=60', etag: '"v1"' })
    return res.end()
  }
  res.writeHead(200, { 'cache-control': 'max-age=1', etag: '"v1"' })
  res.end('hello')
}).listen(0, async () => {
  const origin = `http://localhost:${server.address().port}`
  const d = new Agent().compose(interceptors.cache({ store: new cacheStores.MemoryCacheStore() }))
  const get = () => request(origin, { dispatcher: d }).then(r => r.body.text())
  await get()            // hits=1 (stored, fresh 1s)
  await sleep(1400)
  await get()            // hits=2 (revalidate -> 304 max-age=60)
  await get()            // expected: served fresh from updated entry (hits stays 2)
  console.log(hits)      // observed: 3 — and keeps growing on every subsequent request
  server.close()
})

Observed on undici 8.7.0 (and current main), Node 22.

Expected Behavior

On a 304 validation response the stored entry must be updated per RFC 9111 §4.3.4 (merge 304
headers minus Content-Length, recompute staleAt/deleteAt) — the merge logic from #4617 in
CacheHandler's 304 branch is the right home; the synchronous path needs to route the 304
through it (or the interceptor's revalidation callback needs to write the updated value back to
the store). The Warning: 110 on successfully validated responses should be dropped in the same
change (TODO at interceptor/cache.js:378).

Note: CacheHandler.onResponseStart's cacheability pre-check (cache-handler.js:124-135) returns
early for 304s that carry no cache-control/expires/last-modified, so bare 304s bypass the merge
branch even on the background path — worth folding into the same fix (a 304's freshness should be
recomputed from the merged headers, not gated on the 304's own headers).

---

Found during an agent-assisted HTTP-caching review for @jeswr; every claim reproduced on undici 8.6.0 (repo) and 8.7.0 (npm) on Node 22.23.1. Fix PR to follow.