test: add cache testing suite

Closes nodejs#3852 Closes nodejs#3869 Signed-off-by: flakey5 <[email protected]>
flakey5 · Nov 25, 2024 · 1042386 · 1042386
1 parent 71b6b0b
commit 1042386
Show file tree

Hide file tree

Showing 152 changed files with 69,243 additions and 23 deletions.
diff --git a/docs/docs/api/CacheStore.md b/docs/docs/api/CacheStore.md
@@ -15,7 +15,7 @@ The `MemoryCacheStore` stores the responses in-memory.
 
 - `maxCount` - The maximum amount of responses to store. Default `Infinity`.
 - `maxEntrySize` - The maximum size in bytes that a response's body can be. If a response's body is greater than or equal to this, the response will not be cached.
-
+todo DOCS
 ### `SqliteCacheStore`
 
 The `SqliteCacheStore` stores the responses in a SQLite database.

diff --git a/eslint.config.js b/eslint.config.js
@@ -7,6 +7,7 @@ module.exports = [
     ignores: [
       'lib/llhttp',
       'test/fixtures/wpt',
+      'test/fixtures/cache-tests',
       'undici-fetch.js'
     ],
     noJsx: true,

diff --git a/lib/cache/sqlite-cache-store.js b/lib/cache/sqlite-cache-store.js
@@ -6,6 +6,9 @@ const { assertCacheKey, assertCacheValue } = require('../util/cache.js')
 
 const VERSION = 2
 
+// 2gb
+const MAX_ENTRY_SIZE = 2 * 1000 * 1000 * 1000
+
 /**
  * @typedef {import('../../types/cache-interceptor.d.ts').default.CacheStore} CacheStore
  * @implements {CacheStore}
@@ -18,7 +21,7 @@ const VERSION = 2
  * } & import('../../types/cache-interceptor.d.ts').default.CacheValue} SqliteStoreValue
  */
 class SqliteCacheStore {
-  #maxEntrySize = Infinity
+  #maxEntrySize = MAX_ENTRY_SIZE
   #maxCount = Infinity
 
   /**
@@ -78,6 +81,11 @@ class SqliteCacheStore {
         ) {
           throw new TypeError('SqliteCacheStore options.maxEntrySize must be a non-negative integer')
         }
+
+        if (opts.maxEntrySize > MAX_ENTRY_SIZE) {
+          throw new TypeError('SqliteCacheStore options.maxEntrySize must be less than 2gb')
+        }
+
         this.#maxEntrySize = opts.maxEntrySize
       }
 

diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js
@@ -18,6 +18,11 @@ class CacheHandler {
    */
   #cacheKey
 
+  /**
+   * @type {import('../../types/cache-interceptor.d.ts').default.CacheOptions['type']}
+   */
+  #cacheType
+
   /**
    * @type {import('../../types/cache-interceptor.d.ts').default.CacheStore}
    */
@@ -39,9 +44,10 @@ class CacheHandler {
    * @param {import('../../types/dispatcher.d.ts').default.DispatchHandler} handler
    */
   constructor (opts, cacheKey, handler) {
-    const { store } = opts
+    const { store, type } = opts
 
     this.#store = store
+    this.#cacheType = type
     this.#cacheKey = cacheKey
     this.#handler = handler
   }
@@ -96,11 +102,17 @@ class CacheHandler {
     }
 
     const now = Date.now()
-    const staleAt = determineStaleAt(now, headers, cacheControlDirectives)
+    const staleAt = determineStaleAt(this.#cacheType, now, headers, cacheControlDirectives)
     if (staleAt) {
-      const varyDirectives = this.#cacheKey.headers && headers.vary
-        ? parseVaryHeader(headers.vary, this.#cacheKey.headers)
-        : undefined
+      let varyDirectives
+      if (this.#cacheKey.headers && headers.vary) {
+        varyDirectives = parseVaryHeader(headers.vary, this.#cacheKey.headers)
+        if (!varyDirectives) {
+          // Parse error
+          return downstreamOnHeaders()
+        }
+      }
+
       const deleteAt = determineDeleteAt(now, cacheControlDirectives, staleAt)
 
       const strippedHeaders = stripNecessaryHeaders(headers, cacheControlDirectives)
@@ -178,6 +190,20 @@ function canCacheResponse (statusCode, headers, cacheControlDirectives) {
     return false
   }
 
+  if (typeof headers.age === 'string') {
+    const age = parseInt(headers.age)
+    if (isNaN(age) || age >= 2147483647) {
+      return false
+    }
+
+    if (
+      cacheControlDirectives?.['max-age'] &&
+      age >= cacheControlDirectives?.['max-age']
+    ) {
+      return false
+    }
+  }
+
   if (
     cacheControlDirectives.private === true ||
     cacheControlDirectives['no-cache'] === true ||
@@ -187,7 +213,7 @@ function canCacheResponse (statusCode, headers, cacheControlDirectives) {
   }
 
   // https://www.rfc-editor.org/rfc/rfc9111.html#section-4.1-5
-  if (headers.vary === '*') {
+  if (headers.vary?.includes('*')) {
     return false
   }
 
@@ -216,19 +242,22 @@ function canCacheResponse (statusCode, headers, cacheControlDirectives) {
 }
 
 /**
+ * @param {import('../../types/cache-interceptor.d.ts').default.CacheOptions['type']}
  * @param {number} now
  * @param {Record<string, string | string[]>} headers
  * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives
  *
  * @returns {number | undefined} time that the value is stale at or undefined if it shouldn't be cached
  */
-function determineStaleAt (now, headers, cacheControlDirectives) {
-  // 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) {
-    return now + (sMaxAge * 1000)
+function determineStaleAt (cacheType, now, headers, cacheControlDirectives) {
+  if (cacheType === 'public') {
+    // 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) {
+      return now + (sMaxAge * 1000)
+    }
   }
 
   if (cacheControlDirectives.immutable) {
@@ -241,9 +270,9 @@ function determineStaleAt (now, headers, cacheControlDirectives) {
     return now + (maxAge * 1000)
   }
 
-  if (headers.expire && typeof headers.expire === 'string') {
+  if (headers.expires && typeof headers.expires === 'string') {
     // https://www.rfc-editor.org/rfc/rfc9111.html#section-5.3
-    const expiresDate = new Date(headers.expire)
+    const expiresDate = new Date(headers.expires)
     if (expiresDate instanceof Date && Number.isFinite(expiresDate.valueOf())) {
       return now + (Date.now() - expiresDate.getTime())
     }
@@ -281,6 +310,14 @@ function determineDeleteAt (now, cacheControlDirectives, staleAt) {
 function stripNecessaryHeaders (headers, cacheControlDirectives) {
   const headersToRemove = ['connection']
 
+  if (headers['connection']) {
+    if (Array.isArray(headers['connection'])) {
+      headersToRemove.push(...headers['connection'])
+    } else {
+      headersToRemove.push(headers['connection'])
+    }
+  }
+
   if (Array.isArray(cacheControlDirectives['no-cache'])) {
     headersToRemove.push(...cacheControlDirectives['no-cache'])
   }

diff --git a/lib/util/cache.js b/lib/util/cache.js
@@ -114,10 +114,8 @@ function parseCacheControlHeader (header) {
     let key
     let value
     if (keyValueDelimiter !== -1) {
-      key = directive.substring(0, keyValueDelimiter).trim()
-      value = directive
-        .substring(keyValueDelimiter + 1)
-        .trim()
+      key = directive.substring(0, keyValueDelimiter).trimStart()
+      value = directive.substring(keyValueDelimiter + 1)
     } else {
       key = directive.trim()
     }
@@ -129,10 +127,18 @@ function parseCacheControlHeader (header) {
       case 's-maxage':
       case 'stale-while-revalidate':
       case 'stale-if-error': {
-        if (value === undefined) {
+        if (value === undefined || value[0] === ' ') {
           continue
         }
 
+        if (
+          value.length >= 2 &&
+          value[0] === '"' &&
+          value[value.length - 1] === '"'
+        ) {
+          value = value.substring(1, value.length - 1)
+        }
+
         const parsedValue = parseInt(value, 10)
         // eslint-disable-next-line no-self-compare
         if (parsedValue !== parsedValue) {
@@ -229,7 +235,7 @@ function parseCacheControlHeader (header) {
  * @returns {Record<string, string | string[]>}
  */
 function parseVaryHeader (varyHeader, headers) {
-  if (typeof varyHeader === 'string' && varyHeader === '*') {
+  if (typeof varyHeader === 'string' && varyHeader.includes('*')) {
     return headers
   }
 
@@ -243,6 +249,8 @@ function parseVaryHeader (varyHeader, headers) {
 
     if (headers[trimmedHeader]) {
       output[trimmedHeader] = headers[trimmedHeader]
+    } else {
+      return undefined
     }
   }
 

diff --git a/test/cache-interceptor/cache-tests-worker.mjs b/test/cache-interceptor/cache-tests-worker.mjs
@@ -0,0 +1,6 @@
+'use strict'
+
+import { parentPort } from 'node:worker_threads'
+
+await import('../fixtures/cache-tests/test-engine/server/server.mjs')
+parentPort.postMessage('listening')