Skip to content

Check cache busting search params on all RSC requests #80669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: canary
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,31 @@ export const setCacheBustingSearchParam = (
return
}

setCacheBustingSearchParamWithHash(url, uniqueCacheKey)
}

/**
* Sets a cache-busting search parameter on a URL using a provided hash value.
*
* This function performs the same logic as `setCacheBustingSearchParam` but accepts
* a pre-computed hash instead of computing it from headers.
*
* Example:
* URL before: https://example.com/path?query=1
* hash: "abc123"
* URL after: https://example.com/path?query=1&_rsc=abc123
*
* Note: This function mutates the input URL directly and does not return anything.
*/
export const setCacheBustingSearchParamWithHash = (
url: URL,
hash: string | null
): void => {
if (!hash) {
// No hash provided, we don't need to set a cache-busting search param.
return
}

/**
* Note that we intentionally do not use `url.searchParams.set` here:
*
Expand All @@ -64,6 +89,6 @@ export const setCacheBustingSearchParam = (
.split('&')
.filter((pair) => pair && !pair.startsWith(`${NEXT_RSC_UNION_QUERY}=`))

pairs.push(`${NEXT_RSC_UNION_QUERY}=${uniqueCacheKey}`)
pairs.push(`${NEXT_RSC_UNION_QUERY}=${hash}`)
url.search = pairs.length ? `?${pairs.join('&')}` : ''
}
18 changes: 18 additions & 0 deletions packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import {
RSC_CONTENT_TYPE_HEADER,
NEXT_ROUTER_STATE_TREE_HEADER,
ACTION_HEADER,
NEXT_ROUTER_PREFETCH_HEADER,
NEXT_ROUTER_SEGMENT_PREFETCH_HEADER,
NEXT_URL,
} from '../../client/components/app-router-headers'
import {
getAccessFallbackHTTPStatus,
Expand Down Expand Up @@ -52,6 +55,7 @@ import type { TemporaryReferenceSet } from 'react-server-dom-webpack/server.edge
import { workUnitAsyncStorage } from '../app-render/work-unit-async-storage.external'
import { executeRevalidates } from '../revalidation-utils'
import { getRequestMeta } from '../request-meta'
import { setCacheBustingSearchParam } from '../../client/components/router-reducer/set-cache-busting-search-param'

function formDataFromSearchQueryString(query: string) {
const searchParams = new URLSearchParams(query)
Expand Down Expand Up @@ -337,6 +341,20 @@ async function createRedirectRenderResult(
forwardedHeaders.delete(ACTION_HEADER)

try {
setCacheBustingSearchParam(fetchUrl, {
[NEXT_ROUTER_PREFETCH_HEADER]: forwardedHeaders.get(
NEXT_ROUTER_PREFETCH_HEADER
)
? ('1' as const)
: undefined,
[NEXT_ROUTER_SEGMENT_PREFETCH_HEADER]:
forwardedHeaders.get(NEXT_ROUTER_SEGMENT_PREFETCH_HEADER) ??
undefined,
[NEXT_ROUTER_STATE_TREE_HEADER]:
forwardedHeaders.get(NEXT_ROUTER_STATE_TREE_HEADER) ?? undefined,
[NEXT_URL]: forwardedHeaders.get(NEXT_URL) ?? undefined,
})

const response = await fetch(fetchUrl, {
method: 'GET',
headers: forwardedHeaders,
Expand Down
38 changes: 18 additions & 20 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ import { getCacheHandlers } from './use-cache/handlers'
import { fixMojibake } from './lib/fix-mojibake'
import { computeCacheBustingSearchParam } from '../shared/lib/router/utils/cache-busting-search-param'
import { RedirectStatusCode } from '../client/components/redirect-status-code'
import { setCacheBustingSearchParamWithHash } from '../client/components/router-reducer/set-cache-busting-search-param'

export type FindComponentsResult = {
components: LoadComponentsReturnType
Expand Down Expand Up @@ -2055,27 +2056,19 @@ export default abstract class Server<
const isPossibleServerAction = getIsPossibleServerAction(req)
const hasGetInitialProps = !!components.Component?.getInitialProps
let isSSG = !!components.getStaticProps
// NOTE: Don't delete headers[RSC] yet, it still needs to be used in renderToHTML later
const isRSCRequest = getRequestMeta(req, 'isRSCRequest') ?? false

// Not all CDNs respect the Vary header when caching. We must assume that
// only the URL is used to vary the responses. The Next client computes a
// hash of the header values and sends it as a search param. Before
// responding to a request, we must verify that the hash matches the
// expected value. Neglecting to do this properly can lead to cache
// poisoning attacks on certain CDNs.
// TODO: This is verification only runs during per-segment prefetch
// requests, since those are the only ones that both vary on a custom
// header and are cacheable. But for safety, we should run this
// verification for all requests, once we confirm the behavior is correct.
// Will need to update our test suite, since there are a handlful of unit
// tests that send fetch requests with custom headers but without a
// corresponding cache-busting search param.
// TODO: Consider not using custom request headers at all, and instead fully
// encode everything into the search param.
if (
!this.minimalMode &&
this.nextConfig.experimental.validateRSCRequestHeaders &&
this.isAppSegmentPrefetchEnabled &&
getRequestMeta(req, 'segmentPrefetchRSCRequest')
isRSCRequest
) {
const headers = req.headers
const expectedHash = computeCacheBustingSearchParam(
Expand All @@ -2084,12 +2077,21 @@ export default abstract class Server<
headers[NEXT_ROUTER_STATE_TREE_HEADER.toLowerCase()],
headers[NEXT_URL.toLowerCase()]
)
const actualHash = getRequestMeta(req, 'cacheBustingSearchParam') ?? null
if (expectedHash !== actualHash) {
const actualHash =
getRequestMeta(req, 'cacheBustingSearchParam') ??
new URL(req.url || '', 'http://localhost').searchParams.get(
NEXT_RSC_UNION_QUERY
)

if (!actualHash || expectedHash !== actualHash) {
// The hash sent by the client does not match the expected value.
// Respond with an error.
res.statusCode = 400
res.setHeader('content-type', 'text/plain')
// Redirect to the URL with the correct cache-busting search param.
const url = new URL(req.url || '', 'http://localhost')
setCacheBustingSearchParamWithHash(url, expectedHash)
res.statusCode = 307
// We only return a relative URL here because this is from a RSC request,
// so it must be from the same origin
res.setHeader('location', `${url.pathname}${url.search}`)
res.body('').send()
return null
}
Expand Down Expand Up @@ -2173,10 +2175,6 @@ export default abstract class Server<
const isPrefetchRSCRequest =
getRequestMeta(req, 'isPrefetchRSCRequest') ?? false

// NOTE: Don't delete headers[RSC] yet, it still needs to be used in renderToHTML later

const isRSCRequest = getRequestMeta(req, 'isRSCRequest') ?? false

// when we are handling a middleware prefetch and it doesn't
// resolve to a static data route we bail early to avoid
// unexpected SSR invocations
Expand Down
34 changes: 25 additions & 9 deletions test/e2e/app-dir/app-inline-css/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { nextTestSetup } from 'e2e-utils'
import { NEXT_RSC_UNION_QUERY } from 'next/dist/client/components/app-router-headers'
import { computeCacheBustingSearchParam } from 'next/dist/shared/lib/router/utils/cache-busting-search-param'

describe('app dir - css - experimental inline css', () => {
const { next, isNextDev } = nextTestSetup({
files: __dirname,
Expand All @@ -16,13 +19,23 @@ describe('app dir - css - experimental inline css', () => {
})

it('should not return rsc payload with inlined style as a dynamic client nav', async () => {
const headers = {
rsc: '1',
}
const cacheBustingSearchParam = computeCacheBustingSearchParam(
null,
null,
'[]',
null
)
const rscPayload = await (
await next.fetch('/a', {
method: 'GET',
headers: {
rsc: '1',
},
})
await next.fetch(
`/a?${NEXT_RSC_UNION_QUERY}=${cacheBustingSearchParam}`,
{
method: 'GET',
headers,
}
)
).text()

const style = 'font-size'
Expand All @@ -32,9 +45,12 @@ describe('app dir - css - experimental inline css', () => {

expect(
await (
await next.fetch('/a', {
method: 'GET',
})
await next.fetch(
`/a?${NEXT_RSC_UNION_QUERY}=${cacheBustingSearchParam}`,
{
method: 'GET',
}
)
).text()
).toContain(style) // sanity check that HTML has the style
})
Expand Down
29 changes: 21 additions & 8 deletions test/e2e/app-dir/app-prefetch/prefetching.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { nextTestSetup } from 'e2e-utils'
import { check, waitFor, retry } from 'next-test-utils'
import { NEXT_RSC_UNION_QUERY } from 'next/dist/client/components/app-router-headers'
import { computeCacheBustingSearchParam } from 'next/dist/shared/lib/router/utils/cache-busting-search-param'

const browserConfigWithFixedTime = {
beforePageLoad: (page) => {
Expand Down Expand Up @@ -294,14 +295,26 @@ describe('app dir - prefetching', () => {
true,
])
)
const response = await next.fetch(`/prefetch-auto/justputit?_rsc=dcqtr`, {
headers: {
RSC: '1',
'Next-Router-Prefetch': '1',
'Next-Router-State-Tree': stateTree,
'Next-Url': '/prefetch-auto/vercel',
},
})

const headers = {
RSC: '1',
'Next-Router-Prefetch': '1',
'Next-Router-State-Tree': stateTree,
'Next-Url': '/prefetch-auto/vercel',
}

const url = new URL('/prefetch-auto/justputit', 'http://localhost')
const cacheBustingParam = computeCacheBustingSearchParam(
headers['Next-Router-Prefetch'],
undefined,
headers['Next-Router-State-Tree'],
headers['Next-Url']
)
if (cacheBustingParam) {
url.searchParams.set('_rsc', cacheBustingParam)
}

const response = await next.fetch(url.toString(), { headers })

const prefetchResponse = await response.text()
expect(prefetchResponse).not.toContain('Page Data!')
Expand Down
52 changes: 40 additions & 12 deletions test/e2e/app-dir/app-validation/validation.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { nextTestSetup } from 'e2e-utils'
import { computeCacheBustingSearchParam } from 'next/dist/shared/lib/router/utils/cache-busting-search-param'

describe('app dir - validation', () => {
const { next, skipped } = nextTestSetup({
Expand All @@ -11,20 +12,47 @@ describe('app dir - validation', () => {
}

it('should error when passing invalid router state tree', async () => {
const res = await next.fetch('/', {
headers: {
RSC: '1',
'Next-Router-State-Tree': JSON.stringify(['', '']),
},
})
const stateTree1 = JSON.stringify(['', ''])
const stateTree2 = JSON.stringify(['', {}])

const headers1 = {
RSC: '1',
'Next-Router-State-Tree': stateTree1,
}

const headers2 = {
RSC: '1',
'Next-Router-State-Tree': stateTree2,
}

const url1 = new URL('/', 'http://localhost')
const url2 = new URL('/', 'http://localhost')

// Add cache busting search param for both requests
const cacheBustingParam1 = computeCacheBustingSearchParam(
undefined,
undefined,
stateTree1,
undefined
)
const cacheBustingParam2 = computeCacheBustingSearchParam(
undefined,
undefined,
stateTree2,
undefined
)

if (cacheBustingParam1) {
url1.searchParams.set('_rsc', cacheBustingParam1)
}
if (cacheBustingParam2) {
url2.searchParams.set('_rsc', cacheBustingParam2)
}

const res = await next.fetch(url1.toString(), { headers: headers1 })
expect(res.status).toBe(500)

const res2 = await next.fetch('/', {
headers: {
RSC: '1',
'Next-Router-State-Tree': JSON.stringify(['', {}]),
},
})
const res2 = await next.fetch(url2.toString(), { headers: headers2 })
expect(res2.status).toBe(200)
})
})
Loading
Loading