Skip to content

Commit d10ce4d

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 222f7f5 commit d10ce4d

File tree

8 files changed

+205
-21
lines changed

8 files changed

+205
-21
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: 108 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,18 +267,29 @@ export async function fetchServerResponse(
267267
}
268268
}
269269

270-
export function createFetch(
270+
// This is a subset of the standard Response type. We use a custom type for
271+
// this so we can limit which details about the response leak into the rest of
272+
// the codebase. For example, there's some custom logic for manually following
273+
// redirects, so "redirected" in this type could be a composite of multiple
274+
// browser fetch calls; however, this fact should not leak to the caller.
275+
export type RSCResponse = {
276+
ok: boolean
277+
redirected: boolean
278+
headers: Headers
279+
body: ReadableStream<Uint8Array> | null
280+
status: number
281+
url: string
282+
}
283+
284+
export async function createFetch(
271285
url: URL,
272286
headers: RequestHeaders,
273287
fetchPriority: 'auto' | 'high' | 'low' | null,
274288
signal?: AbortSignal
275-
) {
276-
const fetchUrl = new URL(url)
277-
289+
): Promise<RSCResponse> {
278290
// TODO: In output: "export" mode, the headers do nothing. Omit them (and the
279291
// cache busting search param) from the request so they're
280292
// maximally cacheable.
281-
setCacheBustingSearchParam(fetchUrl, headers)
282293

283294
if (process.env.__NEXT_TEST_MODE && fetchPriority !== null) {
284295
headers['Next-Test-Fetch-Priority'] = fetchPriority
@@ -288,13 +299,103 @@ export function createFetch(
288299
headers['x-deployment-id'] = process.env.NEXT_DEPLOYMENT_ID
289300
}
290301

291-
return fetch(fetchUrl, {
302+
const fetchOptions: RequestInit = {
292303
// Backwards compat for older browsers. `same-origin` is the default in modern browsers.
293304
credentials: 'same-origin',
294305
headers,
295306
priority: fetchPriority || undefined,
296307
signal,
297-
})
308+
}
309+
// `fetchUrl` is slightly different from `url` because we add a cache-busting
310+
// search param to it. This should not leak outside of this function, so we
311+
// track them separately.
312+
let fetchUrl = new URL(url)
313+
setCacheBustingSearchParam(fetchUrl, headers)
314+
let browserResponse = await fetch(fetchUrl, fetchOptions)
315+
316+
// If the server responds with a redirect (e.g. 307), and the redirected
317+
// location does not contain the cache busting search param set in the
318+
// original request, the response is likely invalid — when following the
319+
// redirect, the browser forwards the request headers, but since the cache
320+
// busting search param is missing, the server will reject the request due to
321+
// a mismatch.
322+
//
323+
// Ideally, we would be able to intercept the redirect response and perform it
324+
// manually, instead of letting the browser automatically follow it, but this
325+
// is not allowed by the fetch API.
326+
//
327+
// So instead, we must "replay" the redirect by fetching the new location
328+
// again, but this time we'll append the cache busting search param to prevent
329+
// a mismatch.
330+
//
331+
// TODO: We can optimize Next.js's built-in middleware APIs by returning a
332+
// custom status code, to prevent the browser from automatically following it.
333+
//
334+
// This does not affect Server Action-based redirects; those are encoded
335+
// differently, as part of the Flight body. It only affects redirects that
336+
// occur in a middleware or a third-party proxy.
337+
338+
let redirected = browserResponse.redirected
339+
if (process.env.__NEXT_CLIENT_VALIDATE_RSC_REQUEST_HEADERS) {
340+
// This is to prevent a redirect loop. Same limit used by Chrome.
341+
const MAX_REDIRECTS = 20
342+
for (let n = 0; n < MAX_REDIRECTS; n++) {
343+
if (!browserResponse.redirected) {
344+
// The server did not perform a redirect.
345+
break
346+
}
347+
const responseUrl = new URL(browserResponse.url, fetchUrl)
348+
if (responseUrl.origin !== fetchUrl.origin) {
349+
// The server redirected to an external URL. The rest of the logic below
350+
// is not relevant, because it only applies to internal redirects.
351+
break
352+
}
353+
if (
354+
responseUrl.searchParams.get(NEXT_RSC_UNION_QUERY) ===
355+
fetchUrl.searchParams.get(NEXT_RSC_UNION_QUERY)
356+
) {
357+
// The redirected URL already includes the cache busting search param.
358+
// This was probably intentional. Regardless, there's no reason to
359+
// issue another request to this URL because it already has the param
360+
// value that we would have added below.
361+
break
362+
}
363+
// The RSC request was redirected. Assume the response is invalid.
364+
//
365+
// Append the cache busting search param to the redirected URL and
366+
// fetch again.
367+
fetchUrl = new URL(responseUrl)
368+
setCacheBustingSearchParam(fetchUrl, headers)
369+
browserResponse = await fetch(fetchUrl, fetchOptions)
370+
// We just performed a manual redirect, so this is now true.
371+
redirected = true
372+
}
373+
}
374+
375+
// Remove the cache busting search param from the response URL, to prevent it
376+
// from leaking outside of this function.
377+
const responseUrl = new URL(browserResponse.url, fetchUrl)
378+
responseUrl.searchParams.delete(NEXT_RSC_UNION_QUERY)
379+
380+
const rscResponse: RSCResponse = {
381+
url: responseUrl.href,
382+
383+
// This is true if any redirects occurred, either automatically by the
384+
// browser, or manually by us. So it's different from
385+
// `browserResponse.redirected`, which only tells us whether the browser
386+
// followed a redirect, and only for the last response in the chain.
387+
redirected,
388+
389+
// These can be copied from the last browser response we received. We
390+
// intentionally only expose the subset of fields that are actually used
391+
// elsewhere in the codebase.
392+
ok: browserResponse.ok,
393+
headers: browserResponse.headers,
394+
body: browserResponse.body,
395+
status: browserResponse.status,
396+
}
397+
398+
return rscResponse
298399
}
299400

300401
export function createFromNextReadableStream(

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
import {
2626
createFetch,
2727
createFromNextReadableStream,
28+
type RSCResponse,
2829
type RequestHeaders,
2930
} from '../router-reducer/fetch-server-response'
3031
import {
@@ -997,6 +998,8 @@ export async function fetchRouteOnCacheMiss(
997998
}
998999

9991000
// In output: "export" mode, we need to add the segment path to the URL.
1001+
// TODO: Consider moving this to `createFetch`, where we do similar logic for
1002+
// manipulating the request URL to encode extra information.
10001003
const url = new URL(href)
10011004
const requestUrl = isOutputExportMode
10021005
? addSegmentPathToUrlInOutputExportMode(url, segmentPath)
@@ -1372,7 +1375,7 @@ export async function fetchSegmentPrefetchesUsingDynamicRequest(
13721375
function writeDynamicTreeResponseIntoCache(
13731376
now: number,
13741377
task: PrefetchTask,
1375-
response: Response,
1378+
response: RSCResponse,
13761379
serverData: NavigationFlightResponse,
13771380
entry: PendingRouteCacheEntry,
13781381
couldBeIntercepted: boolean,
@@ -1462,7 +1465,7 @@ function rejectSegmentEntriesIfStillPending(
14621465
function writeDynamicRenderResponseIntoCache(
14631466
now: number,
14641467
task: PrefetchTask,
1465-
response: Response,
1468+
response: RSCResponse,
14661469
serverData: NavigationFlightResponse,
14671470
isResponsePartial: boolean,
14681471
route: FulfilledRouteCacheEntry,
@@ -1629,7 +1632,7 @@ function writeSeedDataIntoCache(
16291632
async function fetchPrefetchResponse(
16301633
url: URL,
16311634
headers: RequestHeaders
1632-
): Promise<Response | null> {
1635+
): Promise<RSCResponse | null> {
16331636
const fetchPriority = 'low'
16341637
const response = await createFetch(url, headers, fetchPriority)
16351638
if (!response.ok) {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2106,7 +2106,8 @@ export default abstract class Server<
21062106
// The hash sent by the client does not match the expected value.
21072107
// Respond with an error.
21082108
res.statusCode = 400
2109-
res.body('Bad Request').send()
2109+
res.setHeader('content-type', 'text/plain')
2110+
res.body('').send()
21102111
return null
21112112
}
21122113
}

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)