Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1b702ae

Browse filesBrowse files
acdliteacdlite
committed
Verify cache-busting param during segment prefetch
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. In this initial PR, the verification I've added only runs during per- segment prefetch requests, since those are the only ones that both vary on a custom header and are also 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. It would be even better if we stopped using custom request headers all, and instead fully encoded everything into the search param. At least for cacheable requests.
1 parent 5f3868b commit 1b702ae
Copy full SHA for 1b702ae

File tree

Expand file treeCollapse file tree

22 files changed

+441
-219
lines changed
Filter options
Expand file treeCollapse file tree

22 files changed

+441
-219
lines changed

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

Copy file name to clipboardExpand all lines: packages/next/src/client/components/router-reducer/set-cache-busting-search-param.ts
+12-8Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use client'
2-
import { hexHash } from '../../../shared/lib/hash'
2+
3+
import { computeCacheBustingSearchParam } from '../../../shared/lib/router/utils/cache-busting-search-param'
34
import {
45
NEXT_ROUTER_PREFETCH_HEADER,
56
NEXT_ROUTER_SEGMENT_PREFETCH_HEADER,
@@ -29,14 +30,17 @@ export const setCacheBustingSearchParam = (
2930
url: URL,
3031
headers: RequestHeaders
3132
): void => {
32-
const uniqueCacheKey = hexHash(
33-
[
34-
headers[NEXT_ROUTER_PREFETCH_HEADER] || '0',
35-
headers[NEXT_ROUTER_SEGMENT_PREFETCH_HEADER] || '0',
36-
headers[NEXT_ROUTER_STATE_TREE_HEADER],
37-
headers[NEXT_URL],
38-
].join(',')
33+
const uniqueCacheKey = computeCacheBustingSearchParam(
34+
headers[NEXT_ROUTER_PREFETCH_HEADER],
35+
headers[NEXT_ROUTER_SEGMENT_PREFETCH_HEADER],
36+
headers[NEXT_ROUTER_STATE_TREE_HEADER],
37+
headers[NEXT_URL]
3938
)
39+
if (uniqueCacheKey === null) {
40+
// None of our custom request headers are present. We don't need to set a
41+
// cache-busting search param.
42+
return
43+
}
4044

4145
/**
4246
* Note that we intentionally do not use `url.searchParams.set` here:

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

Copy file name to clipboardExpand all lines: packages/next/src/server/base-server.ts
+45Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ import {
105105
NEXT_DID_POSTPONE_HEADER,
106106
NEXT_URL,
107107
NEXT_IS_PRERENDER_HEADER,
108+
NEXT_ROUTER_STATE_TREE_HEADER,
108109
} from '../client/components/app-router-headers'
109110
import type {
110111
MatchOptions,
@@ -177,6 +178,7 @@ import { InvariantError } from '../shared/lib/invariant-error'
177178
import { decodeQueryPathParameter } from './lib/decode-query-path-parameter'
178179
import { getCacheHandlers } from './use-cache/handlers'
179180
import { fixMojibake } from './lib/fix-mojibake'
181+
import { computeCacheBustingSearchParam } from '../shared/lib/router/utils/cache-busting-search-param'
180182

181183
export type FindComponentsResult = {
182184
components: LoadComponentsReturnType
@@ -2063,6 +2065,44 @@ export default abstract class Server<
20632065
const hasGetInitialProps = !!components.Component?.getInitialProps
20642066
let isSSG = !!components.getStaticProps
20652067

2068+
// Not all CDNs respect the Vary header when caching. We must assume that
2069+
// only the URL is used to vary the responses. The Next client computes a
2070+
// hash of the header values and sends it as a search param. Before
2071+
// responding to a request, we must verify that the hash matches the
2072+
// expected value. Neglecting to do this properly can lead to cache
2073+
// poisoning attacks on certain CDNs.
2074+
// TODO: This is verification only runs during per-segment prefetch
2075+
// requests, since those are the only ones that both vary on a custom
2076+
// header and are cacheable. But for safety, we should run this
2077+
// verification for all requests, once we confirm the behavior is correct.
2078+
// Will need to update our test suite, since there are a handlful of unit
2079+
// tests that send fetch requests with custom headers but without a
2080+
// corresponding cache-busting search param.
2081+
// TODO: Consider not using custom request headers at all, and instead fully
2082+
// encode everything into the search param.
2083+
if (
2084+
this.isAppSegmentPrefetchEnabled &&
2085+
getRequestMeta(req, 'segmentPrefetchRSCRequest')
2086+
) {
2087+
const headers = req.headers
2088+
const expectedHash = computeCacheBustingSearchParam(
2089+
headers[NEXT_ROUTER_PREFETCH_HEADER.toLowerCase()],
2090+
headers[NEXT_ROUTER_SEGMENT_PREFETCH_HEADER.toLowerCase()],
2091+
headers[NEXT_ROUTER_STATE_TREE_HEADER.toLowerCase()],
2092+
headers[NEXT_URL.toLowerCase()]
2093+
)
2094+
if (expectedHash !== null) {
2095+
const actualHash = getRequestMeta(req, 'cacheBustingSearchParam')
2096+
if (expectedHash !== actualHash) {
2097+
// The hash sent by the client does not match the expected value.
2098+
// Respond with an error.
2099+
res.statusCode = 400
2100+
res.body('Bad Request').send()
2101+
return null
2102+
}
2103+
}
2104+
}
2105+
20662106
// Compute the iSSG cache key. We use the rewroteUrl since
20672107
// pages with fallback: false are allowed to be rewritten to
20682108
// and we need to look up the path by the rewritten path
@@ -3820,6 +3860,11 @@ export default abstract class Server<
38203860
let page = pathname
38213861
const bubbleNoFallback =
38223862
getRequestMeta(ctx.req, 'bubbleNoFallback') ?? false
3863+
addRequestMeta(
3864+
ctx.req,
3865+
'cacheBustingSearchParam',
3866+
query[NEXT_RSC_UNION_QUERY]
3867+
)
38233868
delete query[NEXT_RSC_UNION_QUERY]
38243869

38253870
const options: MatchOptions = {

‎packages/next/src/server/request-meta.ts

Copy file name to clipboardExpand all lines: packages/next/src/server/request-meta.ts
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,17 @@ export interface RequestMeta {
8989
*/
9090
isRSCRequest?: true
9191

92+
/**
93+
* A search param set by the Next.js client when performing RSC requests.
94+
* Because some CDNs do not vary their cache entries on our custom headers,
95+
* this search param represents a hash of the header values. For any cached
96+
* RSC request, we should verify that the hash matches before responding.
97+
* Otherwise this can lead to cache poisoning.
98+
* TODO: Consider not using custom request headers at all, and instead encode
99+
* everything into the search param.
100+
*/
101+
cacheBustingSearchParam?: string
102+
92103
/**
93104
* True when the request is for the `/_next/data` route using the pages
94105
* router.

‎packages/next/src/shared/lib/router/utils/cache-busting-search-param.ts

Copy file name to clipboard
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { hexHash } from '../../hash'
2+
3+
export function computeCacheBustingSearchParam(
4+
prefetchHeader: string | string[] | undefined,
5+
segmentPrefetchHeader: string | string[] | undefined,
6+
stateTreeHeader: string | string[] | undefined,
7+
nextUrlHeader: string | string[] | undefined
8+
): string | null {
9+
if (
10+
prefetchHeader === undefined &&
11+
segmentPrefetchHeader === undefined &&
12+
stateTreeHeader === undefined &&
13+
nextUrlHeader === undefined
14+
) {
15+
return null
16+
}
17+
return hexHash(
18+
[
19+
prefetchHeader || '0',
20+
segmentPrefetchHeader || '0',
21+
stateTreeHeader || '0',
22+
nextUrlHeader || '0',
23+
].join(',')
24+
)
25+
}

‎test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx
-9Lines changed: 0 additions & 9 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx
-19Lines changed: 0 additions & 19 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx
-9Lines changed: 0 additions & 9 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx
-3Lines changed: 0 additions & 3 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/app/page.jsx

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/app/page.jsx
-7Lines changed: 0 additions & 7 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/components/page.jsx

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/components/page.jsx
-40Lines changed: 0 additions & 40 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/next.config.js

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/next.config.js
-6Lines changed: 0 additions & 6 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/per-segment-prefetching.test.ts

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/per-segment-prefetching.test.ts
-65Lines changed: 0 additions & 65 deletions
This file was deleted.

‎test/e2e/app-dir/ppr-navigations/simple/simple.test.ts

Copy file name to clipboardExpand all lines: test/e2e/app-dir/ppr-navigations/simple/simple.test.ts
-31Lines changed: 0 additions & 31 deletions
This file was deleted.

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

Copy file name to clipboard
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export default function RootLayout({
2+
children,
3+
}: {
4+
children: React.ReactNode
5+
}) {
6+
return (
7+
<html lang="en">
8+
<body>{children}</body>
9+
</html>
10+
)
11+
}

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

Copy file name to clipboard
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { LinkAccordion } from '../components/link-accordion'
2+
3+
export default function Page() {
4+
return (
5+
<ul>
6+
<li>
7+
<LinkAccordion href="/target-page">Target page</LinkAccordion>
8+
</li>
9+
</ul>
10+
)
11+
}

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

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function TargetPage() {
2+
return <div id="target-page">Target page</div>
3+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.