fix: store revalidation-only responses so etag revalidation can engage

lib/handler/cache-handler.js

@@ -26,6 +26,10 @@ const NOT_UNDERSTOOD_STATUS_CODES = [
 
 const MAX_RESPONSE_AGE = 2147483647000
 
+// Retention for revalidation-only entries (zero freshness lifetime but a
+//  validator present); each successful revalidation re-stores the entry.
+const REVALIDATION_ONLY_RETENTION = 86400000 // 24 hours
+
 /**
  * @typedef {import('../../types/dispatcher.d.ts').default.DispatchHandler} DispatchHandler
  *
@@ -150,16 +154,24 @@ class CacheHandler {
       ? parseHttpDate(resHeaders.date)
       : undefined
 
+    const hasValidator =
+      (typeof resHeaders.etag === 'string' && isEtagUsable(resHeaders.etag)) ||
+      typeof resHeaders['last-modified'] === 'string'
+
     const staleAt =
-      determineStaleAt(this.#cacheType, now, resAge, resHeaders, resDate, cacheControlDirectives) ??
+      determineStaleAt(this.#cacheType, now, resAge, resHeaders, resDate, cacheControlDirectives, hasValidator) ??
       this.#cacheByDefault
     if (staleAt === undefined || (resAge && resAge > staleAt)) {
       return downstreamOnHeaders()
     }
 
     const baseTime = resDate ? resDate.getTime() : now
     const absoluteStaleAt = staleAt + baseTime
-    if (now >= absoluteStaleAt) {
+    // Zero freshness lifetime but a validator: stale from the start, yet still
+    //  storable since each reuse is preceded by a revalidation request.
+    //  https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.2.4
+    const revalidationOnly = staleAt === 0 && hasValidator
+    if (now >= absoluteStaleAt && !revalidationOnly) {
       // Response is already stale
       return downstreamOnHeaders()
     }
@@ -422,23 +434,35 @@ function getAge (ageHeader) {
  * @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders
  * @param {Date | undefined} responseDate
  * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives
+ * @param {boolean} hasValidator whether the response has a validator (etag or
+ *  last-modified) that revalidation requests can be made with
  *
  * @returns {number | undefined} time that the value is stale at in seconds or undefined if it shouldn't be cached
  */
-function determineStaleAt (cacheType, now, age, resHeaders, responseDate, cacheControlDirectives) {
+function determineStaleAt (cacheType, now, age, resHeaders, responseDate, cacheControlDirectives, hasValidator) {
   if (cacheType === 'shared') {
     // Prioritize s-maxage since we're a shared cache
     //  s-maxage > max-age > Expire
     //  https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.2.10-3
     const sMaxAge = cacheControlDirectives['s-maxage']
     if (sMaxAge !== undefined) {
-      return sMaxAge > 0 ? sMaxAge * 1000 : undefined
+      if (sMaxAge > 0) {
+        return sMaxAge * 1000
+      }
+
+      // Immediately stale, but storable if we can revalidate it before reuse.
+      return sMaxAge === 0 && hasValidator ? 0 : undefined
     }
   }
 
   const maxAge = cacheControlDirectives['max-age']
   if (maxAge !== undefined) {
-    return maxAge > 0 ? maxAge * 1000 : undefined
+    if (maxAge > 0) {
+      return maxAge * 1000
+    }
+
+    // Immediately stale, but storable if we can revalidate it before reuse.
+    return maxAge === 0 && hasValidator ? 0 : undefined
   }
 
   if (typeof resHeaders.expires === 'string') {
@@ -482,6 +506,12 @@ function determineStaleAt (cacheType, now, age, resHeaders, responseDate, cacheC
     return 31536000000
   }
 
+  if (cacheControlDirectives['no-cache'] === true && hasValidator) {
+    // No freshness source, but a validator lets us revalidate before reuse.
+    //  https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.2.4
+    return 0
+  }
+
   return undefined
 }
 
@@ -518,6 +548,11 @@ function determineDeleteAt (baseTime, cachedAt, cacheControlDirectives, staleAt)
   // revalidated.
   if (staleWhileRevalidate === -Infinity && staleIfError === -Infinity && immutable === -Infinity) {
     const freshnessLifetime = staleAt - baseTime
+    if (freshnessLifetime <= 0) {
+      // Revalidation-only entry: no freshness lifetime to size the buffer on,
+      //  so retain it for a bounded window instead.
+      return cachedAt + REVALIDATION_ONLY_RETENTION
+    }
     const datePrecisionPadding = Math.min(Math.max(cachedAt - baseTime, 0), 1000)
     return staleAt + freshnessLifetime + datePrecisionPadding
   }

test/interceptors/cache.js

@@ -170,6 +170,167 @@ describe('Cache Interceptor', () => {
     strictEqual(requestCount, 2)
   })
 
+  test('stores response with no-cache directive and etag, revalidates it on reuse', async () => {
+    let requestsToOrigin = 0
+    let revalidationRequests = 0
+    const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
+      if (req.headers['if-none-match'] === '"asd123"') {
+        revalidationRequests++
+        res.statusCode = 304
+        res.end()
+      } else {
+        requestsToOrigin++
+        res.setHeader('cache-control', 'no-cache')
+        res.setHeader('etag', '"asd123"')
+        res.end('asd')
+      }
+    }).listen(0)
+
+    const client = new Client(`http://localhost:${server.address().port}`)
+      .compose(interceptors.cache())
+
+    after(async () => {
+      server.close()
+      await client.close()
+    })
+
+    await once(server, 'listening')
+
+    /**
+     * @type {import('../../types/dispatcher').default.RequestOptions}
+     */
+    const request = {
+      origin: 'localhost',
+      method: 'GET',
+      path: '/'
+    }
+
+    // Send initial request. This should reach the origin
+    {
+      const res = await client.request(request)
+      strictEqual(await res.body.text(), 'asd')
+      strictEqual(requestsToOrigin, 1)
+      strictEqual(revalidationRequests, 0)
+    }
+
+    // Send second request. The response was stored, so this should be a
+    //  revalidation request answered with a 304 and served from the cache
+    {
+      const res = await client.request(request)
+      strictEqual(await res.body.text(), 'asd')
+      strictEqual(requestsToOrigin, 1)
+      strictEqual(revalidationRequests, 1)
+    }
+  })
+
+  test('stores response with max-age=0 and etag, revalidates it on reuse', async () => {
+    let requestsToOrigin = 0
+    let revalidationRequests = 0
+    const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
+      if (req.headers['if-none-match'] === '"asd123"') {
+        revalidationRequests++
+        res.statusCode = 304
+        res.end()
+      } else {
+        requestsToOrigin++
+        res.setHeader('cache-control', 'max-age=0')
+        res.setHeader('etag', '"asd123"')
+        res.end('asd')
+      }
+    }).listen(0)
+
+    const client = new Client(`http://localhost:${server.address().port}`)
+      .compose(interceptors.cache())
+
+    after(async () => {
+      server.close()
+      await client.close()
+    })
+
+    await once(server, 'listening')
+
+    /**
+     * @type {import('../../types/dispatcher').default.RequestOptions}
+     */
+    const request = {
+      origin: 'localhost',
+      method: 'GET',
+      path: '/'
+    }
+
+    // Send initial request. This should reach the origin
+    {
+      const res = await client.request(request)
+      strictEqual(await res.body.text(), 'asd')
+      strictEqual(requestsToOrigin, 1)
+      strictEqual(revalidationRequests, 0)
+    }
+
+    // Send second request. The response was stored but is already stale, so
+    //  this should be a revalidation request answered with a 304 and served
+    //  from the cache
+    {
+      const res = await client.request(request)
+      strictEqual(await res.body.text(), 'asd')
+      strictEqual(requestsToOrigin, 1)
+      strictEqual(revalidationRequests, 1)
+    }
+  })
+
+  test('stores response with no-cache directive, etag and last-modified, revalidates it on reuse', async () => {
+    let requestsToOrigin = 0
+    let revalidationRequests = 0
+    const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
+      if (req.headers['if-none-match'] === '"asd123"') {
+        revalidationRequests++
+        res.statusCode = 304
+        res.end()
+      } else {
+        requestsToOrigin++
+        res.setHeader('cache-control', 'no-cache')
+        res.setHeader('etag', '"asd123"')
+        res.setHeader('last-modified', new Date(Date.now() - 60000).toUTCString())
+        res.end('asd')
+      }
+    }).listen(0)
+
+    const client = new Client(`http://localhost:${server.address().port}`)
+      .compose(interceptors.cache())
+
+    after(async () => {
+      server.close()
+      await client.close()
+    })
+
+    await once(server, 'listening')
+
+    /**
+     * @type {import('../../types/dispatcher').default.RequestOptions}
+     */
+    const request = {
+      origin: 'localhost',
+      method: 'GET',
+      path: '/'
+    }
+
+    // Send initial request. This should reach the origin
+    {
+      const res = await client.request(request)
+      strictEqual(await res.body.text(), 'asd')
+      strictEqual(requestsToOrigin, 1)
+      strictEqual(revalidationRequests, 0)
+    }
+
+    // Send second request. The response was stored, so this should be a
+    //  revalidation request answered with a 304 and served from the cache
+    {
+      const res = await client.request(request)
+      strictEqual(await res.body.text(), 'asd')
+      strictEqual(requestsToOrigin, 1)
+      strictEqual(revalidationRequests, 1)
+    }
+  })
+
   test('expires caching', async () => {
     const clock = FakeTimers.install({
       toFake: ['Date']