fix: retain request headers in cache lookups (#204)

Author: eduardoboucas

packages/cache/src/bootstrap/cache.test.ts

@@ -151,6 +151,54 @@ describe('Cache API', () => {
       expect(missResponse).toBeUndefined()
     })
 
+    test('retains request headers, discarding any forbidden ones', async () => {
+      const headers = new Headers()
+      headers.set('content-type', 'text/html')
+      headers.set('x-custom-header', 'foobar')
+
+      const response = new Response('<h1>Hello world</h1>', { headers })
+      const mockFetch = new MockFetch()
+        .get({
+          url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
+          headers: (headers) => {
+            expect(headers.Authorization).toBe(`Bearer ${token}`)
+            expect(headers['netlify-forwarded-host']).toBe(host)
+            expect(headers['x-custom-header']).toBe('foo')
+            expect(headers['netlify-programmable-store']).toBe('my-cache')
+            expect(headers['netlify-programmable-headers']).toBeUndefined()
+          },
+          response,
+        })
+        .inject()
+      const cache = new NetlifyCache({
+        getContext: ({ operation }) => {
+          expect(operation).toBe(Operation.Read)
+
+          return { host, token, url }
+        },
+        name: 'my-cache',
+        userAgent,
+      })
+
+      const cached = await cache.match(
+        new Request('https://netlify.com', {
+          headers: {
+            'X-Custom-Header': 'foo',
+            'Netlify-Programmable-Headers': 'forbidden',
+            'Netlify-Programmable-Store': 'not the right store',
+            'Netlify-Forwarded-Host': 'this would break things',
+          },
+        }),
+      )
+
+      mockFetch.restore()
+
+      expect(mockFetch.requests.length).toBe(1)
+
+      expect(cached?.status).toBe(200)
+      expect(await cached?.text()).toBe('<h1>Hello world</h1>')
+    })
+
     test('is a no-op when the `getContext` callback returns `null`', async () => {
       const mockFetch = new MockFetch().inject()
       const cache = new NetlifyCache({

packages/cache/src/bootstrap/cache.ts

@@ -11,12 +11,17 @@ const allowedProtocols = new Set(['http:', 'https:'])
 // sent to the API.
 const discardedHeaders = new Set(['cookie', 'content-encoding', 'content-length'])
 
+// Headers with these prefixes will be discarded from requests when doing cache
+// lookups; allowing clients to set them could lead to unexpected behavior.
+const forbiddenHeaderPrefixes = ['netlify-programmable-', 'x-nf-']
+
 type NetlifyCacheOptions = EnvironmentOptions & {
   name: string
 }
 
 const getInternalHeaders = Symbol('getInternalHeaders')
-const serializeResourceHeaders = Symbol('serializeResourceHeaders')
+const serializeRequestHeaders = Symbol('serializeRequestHeaders')
+const serializeResponseHeaders = Symbol('serializeResponseHeaders')
 
 export class NetlifyCache implements Cache {
   #getContext: RequestContextFactory
@@ -47,7 +52,26 @@ export class NetlifyCache implements Cache {
     return headers
   }
 
-  private [serializeResourceHeaders](headers: Headers) {
+  private [serializeRequestHeaders](headers: Headers) {
+    const headersMap: Record<string, string> = {}
+
+    headers.forEach((value, key) => {
+      const normalizedKey = key.toLowerCase()
+
+      // Discard any internal headers that the client should not be setting.
+      for (const prefix of forbiddenHeaderPrefixes) {
+        if (normalizedKey.startsWith(prefix)) {
+          return
+        }
+      }
+
+      headersMap[normalizedKey] = value
+    })
+
+    return headersMap
+  }
+
+  private [serializeResponseHeaders](headers: Headers) {
     const headersMap: Record<string, string[]> = {}
 
     headers.forEach((value, key) => {
@@ -110,7 +134,10 @@ export class NetlifyCache implements Cache {
       const resourceURL = extractAndValidateURL(request)
       const cacheURL = `${context.url}/${toCacheKey(resourceURL)}`
       const response = await fetch(cacheURL, {
-        headers: this[getInternalHeaders](context),
+        headers: {
+          ...(request instanceof Request ? this[serializeRequestHeaders](request.headers) : {}),
+          ...this[getInternalHeaders](context),
+        },
         method: 'GET',
       })
 
@@ -164,7 +191,7 @@ export class NetlifyCache implements Cache {
       body: response.body,
       headers: {
         ...this[getInternalHeaders](context),
-        [HEADERS.ResourceHeaders]: this[serializeResourceHeaders](response.headers),
+        [HEADERS.ResourceHeaders]: this[serializeResponseHeaders](response.headers),
         [HEADERS.ResourceStatus]: response.status.toString(),
       },
       // @ts-expect-error https://github.com/whatwg/fetch/pull/1457

packages/cache/src/fetchwithcache.test.ts

@@ -53,6 +53,43 @@ describe('`fetchWithCache`', () => {
     expect(mockFetch.requests.length).toBe(2)
   })
 
+  test('Respects request headers', async () => {
+    const headers = new Headers()
+    headers.set('content-type', 'text/html')
+    headers.set('vary', 'x-custom-header')
+
+    const cachedResponse = new Response('<h1>Hello world</h1>', { headers })
+    const mockFetch = new MockFetch()
+      .post({
+        response: new Response(null, { status: 201 }),
+        url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
+      })
+      .get({
+        headers: (headers) => {
+          expect(headers.vary).toBe('x-custom-header')
+        },
+        response: () => cachedResponse,
+        url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
+      })
+      .inject()
+    const resourceURL = 'https://netlify.com'
+
+    const cache = await caches.open('')
+    await cache.put(resourceURL, cachedResponse)
+
+    const response = await fetchWithCache(resourceURL, {
+      headers: {
+        vary: 'x-custom-header',
+      },
+    })
+
+    expect(await response.text()).toBe('<h1>Hello world</h1>')
+
+    mockFetch.restore()
+
+    expect(mockFetch.requests.length).toBe(2)
+  })
+
   test('Throws when used with a method other than GET', async () => {
     const mockFetch = new MockFetch().inject()
     const resourceURL = 'https://netlify.com'
@@ -103,7 +140,7 @@ describe('`fetchWithCache`', () => {
           url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
           response: () => new Response('<h1>Hello world</h1>', { headers }),
         })
-        .get({ url: 'https://netlify.com', response: new Response('<h1>Hello world</h1>', { headers }) })
+        .get({ url: 'https://netlify.com/', response: new Response('<h1>Hello world</h1>', { headers }) })
         .inject()
       const resourceURL = 'https://netlify.com'
 
@@ -152,7 +189,7 @@ describe('`fetchWithCache`', () => {
           response: new Response('<h1>Hello world</h1>', { headers }),
         })
         .get({
-          url: 'https://netlify.com',
+          url: 'https://netlify.com/',
           response: new Response('<h1>Hello world</h1>', { headers }),
         })
         .inject()
@@ -218,7 +255,7 @@ describe('`fetchWithCache`', () => {
           response: new Response('<h1>Hello world</h1>', { headers }),
         })
         .get({
-          url: 'https://netlify.com',
+          url: 'https://netlify.com/',
           response: new Response('<h1>Hello world</h1>', { headers }),
         })
         .inject()
@@ -253,7 +290,7 @@ describe('`fetchWithCache`', () => {
         response: new Response(html),
       })
       .get({
-        url: 'https://netlify.com',
+        url: 'https://netlify.com/',
         response: new Response(html),
       })
       .inject()

packages/cache/src/fetchwithcache.ts

@@ -72,7 +72,7 @@ type FetchWithCache = {
  * whether it comes from the cache or from the network.
  */
 export const fetchWithCache: FetchWithCache = async (
-  request: string | URL | Request,
+  requestOrURL: string | URL | Request,
   optionsOrCacheSettings?: RequestInit | CacheSettings,
   cacheOptionsParam?: CacheOptions,
 ) => {
@@ -87,15 +87,9 @@ export const fetchWithCache: FetchWithCache = async (
     requestInit = {}
   }
 
-  let method: string | undefined
+  const request = new Request(requestOrURL, requestInit)
 
-  if (request instanceof Request) {
-    method = request.method
-  } else {
-    method = requestInit?.method
-  }
-
-  if (method && method?.toLowerCase() !== 'get') {
+  if (request.method.toLowerCase() !== 'get') {
     throw new TypeError('`fetchWithCache` only supports GET requests.')
   }
 
@@ -121,7 +115,7 @@ export const fetchWithCache: FetchWithCache = async (
     return cached
   }
 
-  const fresh = await fetch(request, requestInit)
+  const fresh = await fetch(request)
   if (!fresh.body) {
     return fresh
   }