Skip to content

Commit 4d57658

Browse files
committed
Replay redirect if RSC parameter is missing
If the server responds with a redirect (e.g. 307), and the redirected location does not contain the cache busting search param set in the original request, the response is likely invalid — when following the redirect, the browser will have forwarded the request headers, but without a corresponding cache busting search param. (See vercel#79563 for more context.) Ideally, we would be able to intercept the redirect response and perform it manually, instead of letting the browser automatically follow it, but this is not allowed by the fetch API. So instead, we must "replay" the redirect by fetching the new location again, but this time we'll append the cache busting search param to prevent a mismatch. We can optimize Next.js's built-in middleware APIs by returning a custom status code, to prevent the browser from automatically following it. This will land in future PRs. Third-party proxies can choose to implement this same protocol for cases where they need to redirect Next.js requests but are not using Next.js APIs. However, we'll still need this fallback behavior for third-party proxies that don't implement our optimized redirect protocol. This does not affect Server Action-based redirects; those are encoded differently, as part of the Flight body. As of now, this is gated behind the experimental `validateRSCRequestHeaders` flag. We will test it before turning it on by default.
1 parent 895908a commit 4d57658

File tree

8 files changed

+173
-29
lines changed

8 files changed

+173
-29
lines changed

packages/next/src/build/define-env.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,9 @@ export function getDefineEnv({
187187
'process.env.__NEXT_CLIENT_SEGMENT_CACHE': Boolean(
188188
config.experimental.clientSegmentCache
189189
),
190+
'process.env.__NEXT_CLIENT_VALIDATE_RSC_REQUEST_HEADERS': Boolean(
191+
config.experimental.validateRSCRequestHeaders
192+
),
190193
'process.env.__NEXT_DYNAMIC_ON_HOVER': Boolean(
191194
config.experimental.dynamicOnHover
192195
),

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

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export type RequestHeaders = {
6666
'Next-Test-Fetch-Priority'?: RequestInit['priority']
6767
}
6868

69-
export function urlToUrlWithoutFlightMarker(url: string): URL {
69+
export function canonicalizeResponseURL(url: string): URL {
7070
const urlWithoutFlightParameters = new URL(url, location.origin)
7171
urlWithoutFlightParameters.searchParams.delete(NEXT_RSC_UNION_QUERY)
7272
if (process.env.NODE_ENV === 'production') {
@@ -85,7 +85,7 @@ export function urlToUrlWithoutFlightMarker(url: string): URL {
8585

8686
function doMpaNavigation(url: string): FetchServerResponseResult {
8787
return {
88-
flightData: urlToUrlWithoutFlightMarker(url).toString(),
88+
flightData: canonicalizeResponseURL(url).toString(),
8989
canonicalUrl: undefined,
9090
couldBeIntercepted: false,
9191
prerendered: false,
@@ -181,8 +181,7 @@ export async function fetchServerResponse(
181181
abortController.signal
182182
)
183183

184-
const responseUrl = urlToUrlWithoutFlightMarker(res.url)
185-
const canonicalUrl = res.redirected ? responseUrl : undefined
184+
const responseUrl = canonicalizeResponseURL(res.url)
186185

187186
const contentType = res.headers.get('content-type') || ''
188187
const interception = !!res.headers.get('vary')?.includes(NEXT_URL)
@@ -237,7 +236,7 @@ export async function fetchServerResponse(
237236

238237
return {
239238
flightData: normalizeFlightData(response.f),
240-
canonicalUrl: canonicalUrl,
239+
canonicalUrl: responseUrl,
241240
couldBeIntercepted: interception,
242241
prerendered: response.S,
243242
postponed,
@@ -265,7 +264,7 @@ export async function fetchServerResponse(
265264
}
266265
}
267266

268-
export function createFetch(
267+
export async function createFetch(
269268
url: URL,
270269
headers: RequestHeaders,
271270
fetchPriority: 'auto' | 'high' | 'low' | null,
@@ -286,13 +285,81 @@ export function createFetch(
286285
headers['x-deployment-id'] = process.env.NEXT_DEPLOYMENT_ID
287286
}
288287

289-
return fetch(fetchUrl, {
290-
// Backwards compat for older browsers. `same-origin` is the default in modern browsers.
288+
const fetchOptions: RequestInit = {
291289
credentials: 'same-origin',
292290
headers,
293291
priority: fetchPriority || undefined,
294292
signal,
295-
})
293+
}
294+
const response = await fetch(fetchUrl, fetchOptions)
295+
return replayInternalRSCRedirectsIfNeeded(
296+
response,
297+
fetchUrl,
298+
headers,
299+
fetchOptions
300+
)
301+
}
302+
303+
async function replayInternalRSCRedirectsIfNeeded(
304+
response: Response,
305+
fetchUrl: URL,
306+
headers: RequestHeaders,
307+
fetchOptions: RequestInit
308+
): Promise<Response> {
309+
if (!process.env.__NEXT_CLIENT_VALIDATE_RSC_REQUEST_HEADERS) {
310+
return response
311+
}
312+
// If the server responds with a redirect (e.g. 307), and the redirected
313+
// location does not contain the cache busting search param set in the
314+
// original request, the response is likely invalid — when following the
315+
// redirect, the browser forwards the request headers, but since the cache
316+
// busting search param is missing, the server will reject the request due to
317+
// a mismatch.
318+
//
319+
// Ideally, we would be able to intercept the redirect response and perform it
320+
// manually, instead of letting the browser automatically follow it, but this
321+
// is not allowed by the fetch API.
322+
//
323+
// So instead, we must "replay" the redirect by fetching the new location
324+
// again, but this time we'll append the cache busting search param to prevent
325+
// a mismatch.
326+
//
327+
// TODO: We can optimize Next.js's built-in middleware APIs by returning a
328+
// custom status code, to prevent the browser from automatically following it.
329+
//
330+
// This does not affect Server Action-based redirects; those are encoded
331+
// differently, as part of the Flight body. It only affects redirects that
332+
// occur in a middleware or a third-party proxy.
333+
const MAX_REDIRECTS = 20
334+
for (let n = 0; n < MAX_REDIRECTS; n++) {
335+
if (!response.redirected) {
336+
// The server did not perform a redirect.
337+
return response
338+
}
339+
const redirectedURL = new URL(response.url, fetchUrl)
340+
if (redirectedURL.origin !== fetchUrl.origin) {
341+
// The server redirected to an external URL. The rest of the logic below
342+
// is not relevant, because it only applies to internal redirects.
343+
return response
344+
}
345+
if (
346+
redirectedURL.searchParams.get(NEXT_RSC_UNION_QUERY) ===
347+
fetchUrl.searchParams.get(NEXT_RSC_UNION_QUERY)
348+
) {
349+
// The server performed an internal redirect, but the search param
350+
// was preserved.
351+
return response
352+
}
353+
// The RSC request was redirected. Assume the response is invalid, because
354+
// the cache-busting search param is missing from the redirected URL.
355+
//
356+
// Append the cache busting search param to the redirected URL and
357+
// fetch again.
358+
setCacheBustingSearchParam(redirectedURL, headers)
359+
fetchUrl = redirectedURL
360+
response = await fetch(fetchUrl, fetchOptions)
361+
}
362+
return response
296363
}
297364

298365
export function createFromNextReadableStream(

packages/next/src/client/components/segment-cache-impl/cache.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
RSC_HEADER,
2424
} from '../app-router-headers'
2525
import {
26+
canonicalizeResponseURL,
2627
createFetch,
2728
createFromNextReadableStream,
2829
type RequestHeaders,
@@ -1032,16 +1033,13 @@ export async function fetchRouteOnCacheMiss(
10321033
// Or, we should just use a (readonly) URL object instead. The type of the
10331034
// prop that we pass to seed the initial state does not need to be the same
10341035
// type as the state itself.
1036+
const responseUrl = removeSegmentPathFromURLInOutputExportMode(
1037+
href,
1038+
requestUrl.href,
1039+
response.url
1040+
)
10351041
const canonicalUrl = createHrefFromUrl(
1036-
new URL(
1037-
response.redirected
1038-
? removeSegmentPathFromURLInOutputExportMode(
1039-
href,
1040-
requestUrl.href,
1041-
response.url
1042-
)
1043-
: href
1044-
)
1042+
new URL(canonicalizeResponseURL(responseUrl).href)
10451043
)
10461044

10471045
// Check whether the response varies based on the Next-Url header.

packages/next/src/server/base-server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2101,7 +2101,7 @@ export default abstract class Server<
21012101
// The hash sent by the client does not match the expected value.
21022102
// Respond with an error.
21032103
res.statusCode = 400
2104-
res.body('Bad Request').send()
2104+
res.body('{}').send()
21052105
return null
21062106
}
21072107
}

test/e2e/app-dir/segment-cache/cdn-cache-busting/app/page.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ export default function Page() {
66
<li>
77
<LinkAccordion href="/target-page">Target page</LinkAccordion>
88
</li>
9+
<li>
10+
<LinkAccordion href="/redirect-to-target-page">
11+
Redirects to target page
12+
</LinkAccordion>
13+
</li>
914
</ul>
1015
)
1116
}

test/e2e/app-dir/segment-cache/cdn-cache-busting/cdn-cache-busting.test.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ describe('segment cache (CDN cache busting)', () => {
7979
'search param',
8080
async () => {
8181
const browser = await webdriver(port, '/')
82-
const { status, text } = await browser.eval(async () => {
82+
const { status } = await browser.eval(async () => {
8383
const res = await fetch('/target-page', {
8484
headers: {
8585
RSC: '1',
@@ -91,7 +91,44 @@ describe('segment cache (CDN cache busting)', () => {
9191
})
9292

9393
expect(status).toBe(400)
94-
expect(text).toContain('Bad Request')
94+
}
95+
)
96+
97+
it(
98+
'perform fully prefetched navigation when a third-party proxy ' +
99+
'performs a redirect',
100+
async () => {
101+
let act
102+
const browser = await webdriver(port, '/', {
103+
beforePageLoad(p: Playwright.Page) {
104+
act = createRouterAct(p)
105+
},
106+
})
107+
108+
await act(
109+
async () => {
110+
const linkToggle = await browser.elementByCss(
111+
'[data-link-accordion="/redirect-to-target-page"]'
112+
)
113+
await linkToggle.click()
114+
},
115+
{
116+
includes: 'Target page',
117+
}
118+
)
119+
120+
// Navigate to the prefetched target page.
121+
await act(async () => {
122+
const link = await browser.elementByCss(
123+
'a[href="/redirect-to-target-page"]'
124+
)
125+
await link.click()
126+
127+
// The page was prefetched, so we're able to render the target
128+
// page immediately.
129+
const div = await browser.elementById('target-page')
130+
expect(await div.text()).toBe('Target page')
131+
}, 'no-requests')
95132
}
96133
)
97134
})

test/e2e/app-dir/segment-cache/cdn-cache-busting/server.mjs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ import { createGunzip } from 'zlib'
88

99
const dir = dirname(fileURLToPath(import.meta.url))
1010

11+
// Redirects that happen in the proxy layer, rather than in Next.js itself. This
12+
// is used to test that the client is still able to fully prefetch the
13+
// target page.
14+
const proxyRedirects = {
15+
'/redirect-to-target-page': '/target-page',
16+
}
17+
1118
async function spawnNext(port) {
1219
const child = spawn('pnpm', ['next', 'start', '-p', port, dir], {
1320
env: process.env,
@@ -48,6 +55,17 @@ async function createFakeCDN(destPort) {
4855

4956
const proxy = httpProxy.createProxyServer()
5057
const cdnServer = createServer(async (req, res) => {
58+
const pathname = new URL(req.url, `http://localhost`).pathname
59+
const redirectUrl = proxyRedirects[pathname]
60+
if (redirectUrl) {
61+
console.log('Redirecting to:', redirectUrl)
62+
res.writeHead(307, {
63+
Location: redirectUrl,
64+
})
65+
res.end()
66+
return
67+
}
68+
5169
if (isCacheableRequest(req)) {
5270
// Serve from our fake CDN if there's a matching entry.
5371
const entry = await fakeCDNCache.get(req.url)

test/e2e/app-dir/segment-cache/router-act.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ type Batch = {
99

1010
type PendingRSCRequest = {
1111
route: Playwright.Route
12-
result: Promise<{ body: string; headers: Record<string, string> }>
12+
result: Promise<{
13+
text: string
14+
body: any
15+
headers: Record<string, string>
16+
status: number
17+
}>
1318
didProcess: boolean
1419
}
1520

@@ -145,10 +150,14 @@ export function createRouterAct(
145150
// but it should not affect the timing of when requests reach the
146151
// server; we pass the request to the server the immediately.
147152
result: new Promise(async (resolve) => {
148-
const originalResponse = await page.request.fetch(request)
153+
const originalResponse = await page.request.fetch(request, {
154+
maxRedirects: 0,
155+
})
149156
resolve({
150-
body: await originalResponse.text(),
157+
text: await originalResponse.text(),
158+
body: await originalResponse.body(),
151159
headers: originalResponse.headers(),
160+
status: originalResponse.status(),
152161
})
153162
}),
154163
didProcess: false,
@@ -343,7 +352,11 @@ Choose a more specific substring to assert on.
343352
// fulfill it yet.
344353
remaining.add(item)
345354
} else {
346-
await route.fulfill(fulfilled)
355+
await route.fulfill({
356+
body: fulfilled.body,
357+
headers: fulfilled.headers,
358+
status: fulfilled.status,
359+
})
347360
const browserResponse = await request.response()
348361
if (browserResponse !== null) {
349362
await browserResponse.finished()
@@ -353,10 +366,13 @@ Choose a more specific substring to assert on.
353366

354367
// After flushing the queue, wait for the microtask queue to be
355368
// exhausted, then check if any additional requests are initiated. A
356-
// microtask should be enough because if the router queue is network
357-
// throttled, the next request is issued within a microtask of the
358-
// previous one finishing.
359-
await page.evaluate(() => Promise.resolve())
369+
// single macrotask should be enough because if the router queue is
370+
// network throttled, the next request is issued either directly within
371+
// the task of the previous request's completion event, or in the
372+
// microtask queue of that event.
373+
await page.evaluate(
374+
() => new Promise<void>((res) => requestIdleCallback(() => res()))
375+
)
360376

361377
await waitForPendingRequestChecks()
362378
}

0 commit comments

Comments
 (0)