Fix: Preserve intentional percent encoding in search param for client nav (vercel#74473)

Author: gaojude

packages/next/src/client/components/router-reducer/fetch-server-response.ts

@@ -15,9 +15,10 @@ import type {
   FlightRouterState,
   NavigationFlightResponse,
 } from '../../../server/app-render/types'
+
+import type { NEXT_ROUTER_SEGMENT_PREFETCH_HEADER } from '../app-router-headers'
 import {
   NEXT_ROUTER_PREFETCH_HEADER,
-  NEXT_ROUTER_SEGMENT_PREFETCH_HEADER,
   NEXT_ROUTER_STATE_TREE_HEADER,
   NEXT_RSC_UNION_QUERY,
   NEXT_URL,
@@ -30,12 +31,12 @@ import {
 import { callServer } from '../../app-call-server'
 import { findSourceMapURL } from '../../app-find-source-map-url'
 import { PrefetchKind } from './router-reducer-types'
-import { hexHash } from '../../../shared/lib/hash'
 import {
   normalizeFlightData,
   type NormalizedFlightData,
 } from '../../flight-data-helpers'
 import { getAppBuildId } from '../../app-build-id'
+import { setCacheBustingSearchParam } from './set-cache-busting-search-param'
 
 export interface FetchServerResponseOptions {
   readonly flightRouterState: FlightRouterState
@@ -264,21 +265,7 @@ export function createFetch(
     }
   }
 
-  // This is used to cache bust CDNs that don't support custom headers. The
-  // result is stored in a search param.
-  // TODO: Given that we have to use a search param anyway, we might as well
-  // _only_ use a search param and not bother with the custom headers.
-  // Add unique cache query to avoid caching conflicts on CDN which don't respect the Vary header
-  const uniqueCacheQuery = hexHash(
-    [
-      headers[NEXT_ROUTER_PREFETCH_HEADER] || '0',
-      headers[NEXT_ROUTER_SEGMENT_PREFETCH_HEADER] || '0',
-      headers[NEXT_ROUTER_STATE_TREE_HEADER],
-      headers[NEXT_URL],
-    ].join(',')
-  )
-
-  fetchUrl.searchParams.set(NEXT_RSC_UNION_QUERY, uniqueCacheQuery)
+  setCacheBustingSearchParam(fetchUrl, headers)
 
   if (process.env.__NEXT_TEST_MODE && fetchPriority !== null) {
     headers['Next-Test-Fetch-Priority'] = fetchPriority

packages/next/src/client/components/router-reducer/set-cache-busting-search-param.test.ts

@@ -0,0 +1,78 @@
+import { setCacheBustingSearchParam } from './set-cache-busting-search-param'
+import {
+  NEXT_ROUTER_PREFETCH_HEADER,
+  NEXT_ROUTER_SEGMENT_PREFETCH_HEADER,
+  NEXT_ROUTER_STATE_TREE_HEADER,
+  NEXT_URL,
+} from '../app-router-headers'
+
+describe('setCacheBustingSearchParam', () => {
+  it('should append cache-busting search parameter to URL without existing search params', () => {
+    const url = new URL('https://example.com/path')
+    const headers = {
+      [NEXT_ROUTER_PREFETCH_HEADER]: '1',
+      [NEXT_ROUTER_SEGMENT_PREFETCH_HEADER]: 'segment-1',
+      [NEXT_ROUTER_STATE_TREE_HEADER]: 'state-tree-1',
+    } as const
+
+    setCacheBustingSearchParam(url, headers)
+    expect(url.toString()).toMatch(
+      /https:\/\/example\.com\/path\?_rsc=[a-z0-9]+$/
+    )
+  })
+
+  it('should append cache-busting search parameter to URL with existing search params', () => {
+    const url = new URL('https://example.com/path?query=1')
+    const headers = {
+      [NEXT_ROUTER_PREFETCH_HEADER]: '1',
+      [NEXT_ROUTER_SEGMENT_PREFETCH_HEADER]: 'segment-2',
+      [NEXT_ROUTER_STATE_TREE_HEADER]: 'state-tree-2',
+    } as const
+
+    setCacheBustingSearchParam(url, headers)
+    expect(url.toString()).toMatch(
+      /https:\/\/example\.com\/path\?query=1&_rsc=[a-z0-9]+$/
+    )
+  })
+
+  it('should generate unique cache key based on headers', () => {
+    const url = new URL('https://example.com/path')
+    const headers1 = {
+      [NEXT_ROUTER_PREFETCH_HEADER]: '1',
+      [NEXT_ROUTER_SEGMENT_PREFETCH_HEADER]: 'segment-3',
+      [NEXT_ROUTER_STATE_TREE_HEADER]: 'state-tree-3',
+    } as const
+
+    const headers2 = {
+      [NEXT_ROUTER_PREFETCH_HEADER]: '1',
+      [NEXT_ROUTER_SEGMENT_PREFETCH_HEADER]: 'segment-4',
+      [NEXT_ROUTER_STATE_TREE_HEADER]: 'state-tree-4',
+      [NEXT_URL]: 'https://example.com/next-url-2',
+    } as const
+
+    setCacheBustingSearchParam(url, headers1)
+    const url1String = url.toString()
+
+    setCacheBustingSearchParam(url, headers2)
+    const url2String = url.toString()
+
+    expect(url1String).not.toEqual(url2String)
+  })
+
+  it('should append cache-busting search parameter to URL with existing search params containing %20', () => {
+    const url = new URL('https://example.com/path?query=apple%20watch')
+    const headers = {
+      [NEXT_ROUTER_PREFETCH_HEADER]: '1',
+      [NEXT_ROUTER_SEGMENT_PREFETCH_HEADER]: 'segment-5',
+      [NEXT_ROUTER_STATE_TREE_HEADER]: 'state-tree-5',
+      [NEXT_URL]: 'https://example.com/next-url',
+    } as const
+
+    setCacheBustingSearchParam(url, headers)
+
+    // Ensure %20 is not automatically decoded
+    expect(url.toString()).toMatch(
+      /https:\/\/example\.com\/path\?query=apple%20watch&_rsc=[a-z0-9]+$/
+    )
+  })
+})

packages/next/src/client/components/router-reducer/set-cache-busting-search-param.ts

@@ -0,0 +1,59 @@
+'use client'
+import { hexHash } from '../../../shared/lib/hash'
+import {
+  NEXT_ROUTER_PREFETCH_HEADER,
+  NEXT_ROUTER_SEGMENT_PREFETCH_HEADER,
+  NEXT_ROUTER_STATE_TREE_HEADER,
+  NEXT_URL,
+  NEXT_RSC_UNION_QUERY,
+} from '../app-router-headers'
+import type { RequestHeaders } from './fetch-server-response'
+
+/**
+ * Mutates the provided URL by adding a cache-busting search parameter for CDNs that don't
+ * support custom headers. This helps avoid caching conflicts by making each request unique.
+ *
+ * Rather than relying on the Vary header which some CDNs ignore, we append a search param
+ * to create a unique URL that forces a fresh request.
+ *
+ * Example:
+ * URL before: https://example.com/path?query=1
+ * URL after: https://example.com/path?query=1&_rsc=abc123
+ *
+ * Note: This function mutates the input URL directly and does not return anything.
+ *
+ * TODO: Since we need to use a search param anyway, we could simplify by removing the custom
+ * headers approach entirely and just use search params.
+ */
+export const setCacheBustingSearchParam = (
+  url: URL,
+  headers: RequestHeaders
+): void => {
+  const uniqueCacheKey = hexHash(
+    [
+      headers[NEXT_ROUTER_PREFETCH_HEADER] || '0',
+      headers[NEXT_ROUTER_SEGMENT_PREFETCH_HEADER] || '0',
+      headers[NEXT_ROUTER_STATE_TREE_HEADER],
+      headers[NEXT_URL],
+    ].join(',')
+  )
+
+  /**
+   * Note that we intentionally do not use `url.searchParams.set` here:
+   *
+   * const url = new URL('https://example.com/search?q=custom%20spacing');
+   * url.searchParams.set('_rsc', 'abc123');
+   * console.log(url.toString()); // Outputs: https://example.com/search?q=custom+spacing&_rsc=abc123
+   *                                                                             ^ <--- this is causing confusion
+   * This is in fact intended based on https://url.spec.whatwg.org/#interface-urlsearchparams, but
+   * we want to preserve the %20 as %20 if that's what the user passed in, hence the custom
+   * logic below.
+   */
+  const existingSearch = url.search
+  const rawQuery = existingSearch.startsWith('?')
+    ? existingSearch.slice(1)
+    : existingSearch
+  const pairs = rawQuery.split('&').filter(Boolean)
+  pairs.push(`${NEXT_RSC_UNION_QUERY}=${uniqueCacheKey}`)
+  url.search = pairs.length ? `?${pairs.join('&')}` : ''
+}

test/e2e/app-dir/app-prefetch/app/uri-encoded-prefetch/page.js

@@ -0,0 +1,18 @@
+import Link from 'next/link'
+
+export default async function Page() {
+  return (
+    <>
+      <p id="uri-encoded-prefetch">URI Encoded Prefetch</p>
+      <p>
+        <Link
+          href="/?param=with%20space"
+          id="prefetch-via-link"
+          prefetch={true}
+        >
+          Prefetch Via Link
+        </Link>
+      </p>
+    </>
+  )
+}

test/e2e/app-dir/app-prefetch/prefetching.test.ts

@@ -342,6 +342,48 @@ describe('app dir - prefetching', () => {
     await browser.waitForElementByCss('#prefetch-auto-page-data')
   })
 
+  it('should not unintentionally modify the requested prefetch by escaping the uri encoded query params', async () => {
+    const rscRequests = []
+    const browser = await next.browser('/uri-encoded-prefetch', {
+      beforePageLoad(page: Page) {
+        page.on('request', async (req: Request) => {
+          const url = new URL(req.url())
+          const headers = await req.allHeaders()
+          if (headers['rsc']) {
+            rscRequests.push(url.pathname + url.search)
+          }
+        })
+      },
+    })
+
+    // sanity check: the link should be present
+    expect(await browser.elementById('prefetch-via-link')).toBeDefined()
+
+    await browser.waitForIdleNetwork()
+
+    // The space encoding of the prefetch request should be the same as the href, and should not be replaced with a +
+    await retry(async () => {
+      expect(
+        rscRequests.filter((req) => req.includes('/?param=with%20space'))
+      ).toHaveLength(1)
+    })
+
+    // Click the link
+    await browser.elementById('prefetch-via-link').click()
+
+    // Assert that we're on the homepage
+    expect(await browser.hasElementByCssSelector('#to-dashboard')).toBe(true)
+
+    await browser.waitForIdleNetwork()
+
+    // No new requests should be made since it is correctly prefetched
+    await retry(async () => {
+      expect(
+        rscRequests.filter((req) => req.includes('/?param=with%20space'))
+      ).toHaveLength(1)
+    })
+  })
+
   describe('prefetch cache seeding', () => {
     it('should not re-fetch the initial static page if the same page is prefetched with prefetch={true}', async () => {
       const rscRequests = []