fix(proxy): hash upstream cache keys (#835)

Author: harlan-zw

packages/script/src/runtime/server/instagram-embed.ts

@@ -1,6 +1,7 @@
 import { createError, defineEventHandler, getQuery, setHeader } from 'h3'
 import { defineCachedFunction, useRuntimeConfig } from 'nitropack/runtime'
 import { $fetch } from 'ofetch'
+import { hash } from 'ohash'
 import { ELEMENT_NODE, parse, renderSync, TEXT_NODE, walkSync } from 'ultrahtml'
 import { createCachedJsonFetch } from './utils/cached-upstream'
 import { isEmbedShell, proxyAssetUrl, rewriteUrl, rewriteUrlsInText, RSRC_RE, scopeCss } from './utils/instagram-embed'
@@ -39,7 +40,7 @@ const cachedEmbedFetch = defineCachedFunction(
       const parts = [url]
       for (const [k, v] of Object.entries(headers).sort(([a], [b]) => a.localeCompare(b)))
         parts.push(`${k}=${v}`)
-      return parts.join('|')
+      return hash(parts)
     },
   },
 )

packages/script/src/runtime/server/utils/cached-upstream.ts

@@ -1,6 +1,7 @@
 import { Buffer } from 'node:buffer'
 import { defineCachedFunction } from 'nitropack/runtime'
 import { $fetch } from 'ofetch'
+import { hash } from 'ohash'
 
 /**
  * Server-side caches for upstream proxy fetches.
@@ -75,7 +76,7 @@ export function createCachedBinaryFetch(
       staleMaxAge: maxAge,
       getKey: (url: string, opts?: CachedBinaryFetchOptions) => {
         if (!opts)
-          return url
+          return hash(url)
         // Vary on headers + redirect mode — callers with different user agents
         // or redirect policies may get different upstream responses.
         const parts = [url]
@@ -86,7 +87,9 @@ export function createCachedBinaryFetch(
         }
         if (opts.redirect)
           parts.push(`redirect=${opts.redirect}`)
-        return parts.join('|')
+        if (opts.ignoreResponseError !== undefined)
+          parts.push(`ignoreResponseError=${opts.ignoreResponseError}`)
+        return hash(parts)
       },
     },
   )
@@ -102,7 +105,8 @@ export function createCachedBinaryFetch(
 /**
  * Cache upstream JSON/text fetches. `getKey` is caller-controlled so handlers
  * can normalize on whichever inner params identify the resource (tweet ID,
- * post URL, query hash).
+ * post URL, query hash). The normalized value is hashed before it reaches
+ * Nitro storage because raw URLs can contain reserved key characters.
  */
 export function createCachedJsonFetch<T>(
   name: string,
@@ -121,7 +125,7 @@ export function createCachedJsonFetch<T>(
       maxAge,
       swr: true,
       staleMaxAge: maxAge,
-      getKey,
+      getKey: (url, opts) => hash(getKey(url, opts)),
     },
   )
 }

test/unit/cached-upstream.test.ts

@@ -0,0 +1,77 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { cacheDefinitions, hashMock } = vi.hoisted(() => ({
+  cacheDefinitions: [] as Array<{ getKey?: (...args: any[]) => string }>,
+  hashMock: vi.fn((value: unknown) => `hashed:${JSON.stringify(value)}`),
+}))
+
+vi.mock('nitropack/runtime', () => ({
+  defineCachedFunction: vi.fn((handler, options) => {
+    cacheDefinitions.push(options)
+    return handler
+  }),
+}))
+
+vi.mock('ohash', () => ({
+  hash: hashMock,
+}))
+
+vi.mock('ofetch', () => ({
+  $fetch: Object.assign(vi.fn(), { raw: vi.fn() }),
+}))
+
+const { createCachedBinaryFetch, createCachedJsonFetch } = await import(
+  '../../packages/script/src/runtime/server/utils/cached-upstream',
+)
+
+beforeEach(() => {
+  cacheDefinitions.length = 0
+  hashMock.mockClear()
+})
+
+describe('upstream cache keys', () => {
+  it('hashes normalized JSON cache keys before passing them to Nitro storage', () => {
+    const normalizeKey = vi.fn((url: string) => new URL(url).searchParams.get('actor') || url)
+    createCachedJsonFetch('profile', 60, normalizeKey)
+
+    const getKey = cacheDefinitions[0]?.getKey
+    expect(getKey).toBeTypeOf('function')
+    expect(getKey?.('https://public.api.bsky.app/profile?actor=nuxt.com')).toBe('hashed:"nuxt.com"')
+    expect(normalizeKey).toHaveBeenCalledOnce()
+    expect(hashMock).toHaveBeenCalledWith('nuxt.com')
+  })
+
+  it('hashes binary URLs instead of exposing reserved characters to storage', () => {
+    createCachedBinaryFetch('image', 60)
+
+    const getKey = cacheDefinitions[0]?.getKey
+    const url = 'https://cdn.example.com/image.jpg?width=640&format=webp'
+    expect(getKey?.(url)).toBe(`hashed:${JSON.stringify(url)}`)
+    expect(hashMock).toHaveBeenCalledWith(url)
+  })
+
+  it('uses stable binary option ordering and varies on response-error behavior', () => {
+    createCachedBinaryFetch('image', 60)
+
+    const getKey = cacheDefinitions[0]?.getKey
+    const key = getKey?.('https://cdn.example.com/image.jpg', {
+      headers: { Zebra: 'last', Accept: 'image/webp' },
+      redirect: 'manual',
+      ignoreResponseError: true,
+    })
+
+    expect(key).toBe('hashed:["https://cdn.example.com/image.jpg","Accept=image/webp","Zebra=last","redirect=manual","ignoreResponseError=true"]')
+  })
+
+  it('hashes Instagram embed URLs together with their response-varying headers', async () => {
+    await import('../../packages/script/src/runtime/server/instagram-embed')
+
+    const getKey = cacheDefinitions[0]?.getKey
+    const key = getKey?.('https://www.instagram.com/p/example/embed/captioned/', {
+      'User-Agent': 'Nuxt Scripts',
+      'Accept': 'text/html',
+    })
+
+    expect(key).toBe('hashed:["https://www.instagram.com/p/example/embed/captioned/","Accept=text/html","User-Agent=Nuxt Scripts"]')
+  })
+})