fix: update stored cache entry on synchronous 304 revalidation

lib/handler/cache-handler.js

@@ -121,6 +121,12 @@ class CacheHandler {
       return downstreamOnHeaders()
     }
 
+    // Not modified, freshen the stored response and re-use its body
+    // https://www.rfc-editor.org/rfc/rfc9111.html#name-freshening-stored-responses
+    if (statusCode === 304) {
+      return this.#handle304(controller, resHeaders, downstreamOnHeaders)
+    }
+
     const cacheControlHeader = resHeaders['cache-control']
     const heuristicallyCacheable = resHeaders['last-modified'] && HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode)
     if (
@@ -191,126 +197,202 @@ class CacheHandler {
       deleteAt
     }
 
-    // Not modified, re-use the cached value
-    // https://www.rfc-editor.org/rfc/rfc9111.html#name-handling-304-not-modified
-    if (statusCode === 304) {
-      const handle304 = (cachedValue) => {
-        if (!cachedValue) {
-          // Do not create a new cache entry, as a 304 won't have a body - so cannot be cached.
-          return downstreamOnHeaders()
+    if (typeof resHeaders.etag === 'string' && isEtagUsable(resHeaders.etag)) {
+      value.etag = resHeaders.etag
+    }
+
+    this.#writeStream = this.#store.createWriteStream(this.#cacheKey, value)
+
+    if (!this.#writeStream) {
+      return downstreamOnHeaders()
+    }
+
+    this.#writeStream
+      .on('drain', () => controller.resume())
+      .on('error', function () {
+        // TODO (fix): Make error somehow observable?
+        handler.#writeStream = undefined
+
+        // Delete the value in case the cache store is holding onto state from
+        //  the call to createWriteStream
+        handler.#store.delete(handler.#cacheKey)
+      })
+      .on('close', function () {
+        if (handler.#writeStream === this) {
+          handler.#writeStream = undefined
         }
 
-        // Re-use the cached value: statuscode, statusmessage, headers and body
-        value.statusCode = cachedValue.statusCode
-        value.statusMessage = cachedValue.statusMessage
-        value.etag = cachedValue.etag
-        value.headers = { ...cachedValue.headers, ...strippedHeaders }
+        // TODO (fix): Should we resume even if was paused downstream?
+        controller.resume()
+      })
 
-        downstreamOnHeaders()
+    downstreamOnHeaders()
+  }
 
-        this.#writeStream = this.#store.createWriteStream(this.#cacheKey, value)
+  /**
+   * Handles a 304 Not Modified validation response by updating the stored
+   *  response with the validation response's header fields and recomputing
+   *  its freshness from the merged headers, per RFC 9111 §4.3.4.
+   *
+   * https://www.rfc-editor.org/rfc/rfc9111.html#name-freshening-stored-responses
+   *
+   * @param {import('../../types/dispatcher.d.ts').default.DispatchController} controller
+   * @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders
+   * @param {() => void} downstreamOnHeaders
+   */
+  #handle304 (controller, resHeaders, downstreamOnHeaders) {
+    const handler = this
 
-        if (!this.#writeStream || !cachedValue?.body) {
-          return
-        }
+    const handle304 = (cachedValue) => {
+      if (!cachedValue) {
+        // Do not create a new cache entry, as a 304 won't have a body - so cannot be cached.
+        return downstreamOnHeaders()
+      }
 
-        if (typeof cachedValue.body.values === 'function') {
-          const bodyIterator = cachedValue.body.values()
-
-          const streamCachedBody = () => {
-            for (const chunk of bodyIterator) {
-              const full = this.#writeStream.write(chunk) === false
-              this.#handler.onResponseData?.(controller, chunk)
-              // when stream is full stop writing until we get a 'drain' event
-              if (full) {
-                break
-              }
-            }
-          }
+      // https://www.rfc-editor.org/rfc/rfc9111.html#section-4.3.4-4
+      const headers = mergeValidationHeaders(cachedValue.headers, resHeaders)
+      const cacheControlDirectives = headers['cache-control']
+        ? parseCacheControlHeader(headers['cache-control'])
+        : {}
 
-          this.#writeStream
-            .on('error', function () {
-              handler.#writeStream = undefined
-              handler.#store.delete(handler.#cacheKey)
-            })
-            .on('drain', () => {
-              streamCachedBody()
-            })
-            .on('close', function () {
-              if (handler.#writeStream === this) {
-                handler.#writeStream = undefined
-              }
-            })
-
-          streamCachedBody()
-        } else if (typeof cachedValue.body.on === 'function') {
-          // Readable stream body (e.g. from async/remote cache stores)
-          cachedValue.body
-            .on('data', (chunk) => {
-              this.#writeStream.write(chunk)
-              this.#handler.onResponseData?.(controller, chunk)
-            })
-            .on('end', () => {
-              this.#writeStream.end()
-            })
-            .on('error', () => {
-              this.#writeStream = undefined
-              this.#store.delete(this.#cacheKey)
-            })
-
-          this.#writeStream
-            .on('error', function () {
-              handler.#writeStream = undefined
-              handler.#store.delete(handler.#cacheKey)
-            })
-            .on('close', function () {
-              if (handler.#writeStream === this) {
-                handler.#writeStream = undefined
-              }
-            })
+      if (!canCacheResponse(this.#cacheType, cachedValue.statusCode, headers, cacheControlDirectives, this.#cacheKey.headers)) {
+        // The freshened response is no longer storable, leave the stored entry as-is
+        return downstreamOnHeaders()
+      }
+
+      const now = Date.now()
+      const resAge = resHeaders.age ? getAge(resHeaders.age) : undefined
+      if (resAge && resAge >= MAX_RESPONSE_AGE) {
+        return downstreamOnHeaders()
+      }
+
+      const resDate = typeof resHeaders.date === 'string'
+        ? parseHttpDate(resHeaders.date)
+        : undefined
+
+      // Recompute freshness from the merged headers, not just those on the 304
+      const staleAt =
+        determineStaleAt(this.#cacheType, now, resAge, headers, resDate, cacheControlDirectives) ??
+        this.#cacheByDefault
+      if (staleAt === undefined || (resAge && resAge > staleAt)) {
+        return downstreamOnHeaders()
+      }
+
+      const baseTime = resDate ? resDate.getTime() : now
+      const absoluteStaleAt = staleAt + baseTime
+      if (now >= absoluteStaleAt) {
+        // Still stale after freshening, leave the stored entry as it is
+        return downstreamOnHeaders()
+      }
+
+      let varyDirectives
+      if (this.#cacheKey.headers && headers.vary) {
+        varyDirectives = parseVaryHeader(headers.vary, this.#cacheKey.headers)
+        if (!varyDirectives) {
+          // Parse error
+          return downstreamOnHeaders()
         }
       }
 
+      const cachedAt = resAge ? now - resAge : now
+
       /**
        * @type {import('../../types/cache-interceptor.d.ts').default.CacheValue}
        */
-      const result = this.#store.get(this.#cacheKey)
-      if (result && typeof result.then === 'function') {
-        result.then(handle304)
-      } else {
-        handle304(result)
+      const value = {
+        statusCode: cachedValue.statusCode,
+        statusMessage: cachedValue.statusMessage,
+        headers,
+        vary: varyDirectives,
+        cacheControlDirectives,
+        cachedAt,
+        staleAt: absoluteStaleAt,
+        deleteAt: determineDeleteAt(baseTime, cachedAt, cacheControlDirectives, absoluteStaleAt)
       }
-    } else {
+
       if (typeof resHeaders.etag === 'string' && isEtagUsable(resHeaders.etag)) {
         value.etag = resHeaders.etag
+      } else {
+        value.etag = cachedValue.etag
       }
 
+      downstreamOnHeaders()
+
       this.#writeStream = this.#store.createWriteStream(this.#cacheKey, value)
 
-      if (!this.#writeStream) {
-        return downstreamOnHeaders()
+      if (!this.#writeStream || !cachedValue.body) {
+        return
       }
 
-      this.#writeStream
-        .on('drain', () => controller.resume())
-        .on('error', function () {
-          // TODO (fix): Make error somehow observable?
-          handler.#writeStream = undefined
-
-          // Delete the value in case the cache store is holding onto state from
-          //  the call to createWriteStream
-          handler.#store.delete(handler.#cacheKey)
-        })
-        .on('close', function () {
-          if (handler.#writeStream === this) {
+      if (typeof cachedValue.body.on === 'function') {
+        // Readable stream body (e.g. from async/remote cache stores)
+        cachedValue.body
+          .on('data', (chunk) => {
+            this.#writeStream.write(chunk)
+            this.#handler.onResponseData?.(controller, chunk)
+          })
+          .on('end', () => {
+            this.#writeStream.end()
+          })
+          .on('error', () => {
+            this.#writeStream = undefined
+            this.#store.delete(this.#cacheKey)
+          })
+
+        this.#writeStream
+          .on('error', function () {
             handler.#writeStream = undefined
+            handler.#store.delete(handler.#cacheKey)
+          })
+          .on('close', function () {
+            if (handler.#writeStream === this) {
+              handler.#writeStream = undefined
+            }
+          })
+      } else {
+        // Iterable of chunks (e.g. MemoryCacheStore), or a single Buffer
+        //  (e.g. SqliteCacheStore) whose values() would iterate single bytes
+        const bodyIterator = typeof cachedValue.body.values === 'function' && !ArrayBuffer.isView(cachedValue.body)
+          ? cachedValue.body.values()
+          : [cachedValue.body].values()
+
+        const streamCachedBody = () => {
+          for (const chunk of bodyIterator) {
+            const full = this.#writeStream.write(chunk) === false
+            this.#handler.onResponseData?.(controller, chunk)
+            // when stream is full stop writing until we get a 'drain' event
+            if (full) {
+              break
+            }
           }
+        }
 
-          // TODO (fix): Should we resume even if was paused downstream?
-          controller.resume()
-        })
+        this.#writeStream
+          .on('error', function () {
+            handler.#writeStream = undefined
+            handler.#store.delete(handler.#cacheKey)
+          })
+          .on('drain', () => {
+            streamCachedBody()
+          })
+          .on('close', function () {
+            if (handler.#writeStream === this) {
+              handler.#writeStream = undefined
+            }
+          })
 
-      downstreamOnHeaders()
+        streamCachedBody()
+      }
+    }
+
+    /**
+     * @type {import('../../types/cache-interceptor.d.ts').default.CacheValue}
+     */
+    const result = this.#store.get(this.#cacheKey)
+    if (result && typeof result.then === 'function') {
+      result.then(handle304)
+    } else {
+      handle304(result)
     }
   }
 
@@ -525,6 +607,27 @@ function determineDeleteAt (baseTime, cachedAt, cacheControlDirectives, staleAt)
   return Math.max(staleAt, staleWhileRevalidate, staleIfError, immutable)
 }
 
+/**
+ * Updates stored response headers with the ones of a validation response,
+ *  per RFC 9111 §4.3.4. Content-Length is not carried over since it
+ *  describes the validation response, not the stored body.
+ *
+ * https://www.rfc-editor.org/rfc/rfc9111.html#section-4.3.4-4
+ *
+ * @param {Record<string, string | string[]>} cachedHeaders headers of the stored response
+ * @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders headers of the validation response
+ * @returns {Record<string, string | string[]>}
+ */
+function mergeValidationHeaders (cachedHeaders, resHeaders) {
+  const cacheControlDirectives = resHeaders['cache-control']
+    ? parseCacheControlHeader(resHeaders['cache-control'])
+    : {}
+  const validationHeaders = { ...stripNecessaryHeaders(resHeaders, cacheControlDirectives) }
+  delete validationHeaders['content-length']
+
+  return { ...cachedHeaders, ...validationHeaders }
+}
+
 /**
  * Strips headers required to be removed in cached responses
  * @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders
@@ -584,3 +687,4 @@ function isValidDate (date) {
 }
 
 module.exports = CacheHandler
+module.exports.mergeValidationHeaders = mergeValidationHeaders