Skip to content

Commit

Permalink
[ppr] Request normalization fixes (vercel#65717)
Browse files Browse the repository at this point in the history
This resolves an issue where a prefetch react server components (RSC)
request incorrectly causes cache poisoning issues during revalidation
for applications configured with partial prerendering (PPR).

It removes the test which used the header directly, and instead defers
to the `handleRSCRequest` method which includes specific environment
implementations.

This also fixes a bug where the prefetch RSC request for the root page
was not normalized.
  • Loading branch information
wyattjoh authored and panteliselef committed May 20, 2024
1 parent 20b4f8d commit 975d953
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 19 deletions.
43 changes: 28 additions & 15 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,14 @@ export default abstract class Server<
// revalidation requests and we want the cache to instead depend on the
// request path for flight information.
stripFlightHeaders(req.headers)

return false
} else if (req.headers[RSC_HEADER.toLowerCase()] === '1') {
addRequestMeta(req, 'isRSCRequest', true)

if (req.headers[NEXT_ROUTER_PREFETCH_HEADER.toLowerCase()] === '1') {
addRequestMeta(req, 'isPrefetchRSCRequest', true)
}
} else {
// Otherwise just return without doing anything.
return false
Expand Down Expand Up @@ -808,27 +815,30 @@ export default abstract class Server<
): Promise<void> {
await this.prepare()
const method = req.method.toUpperCase()
const rsc = isRSCRequestCheck(req) ? 'RSC ' : ''

const tracer = getTracer()
return tracer.withPropagatedContext(req.headers, () => {
return tracer.trace(
BaseServerSpan.handleRequest,
{
spanName: `${rsc}${method} ${req.url}`,
spanName: `${method} ${req.url}`,
kind: SpanKind.SERVER,
attributes: {
'http.method': method,
'http.target': req.url,
'next.rsc': Boolean(rsc),
},
},
async (span) =>
this.handleRequestImpl(req, res, parsedUrl).finally(() => {
if (!span) return

const isRSCRequest = isRSCRequestCheck(req) ?? false

span.setAttributes({
'http.status_code': res.statusCode,
'next.rsc': isRSCRequest,
})

const rootSpanAttributes = tracer.getRootSpanAttributes()
// We were unable to get attributes, probably OTEL is not enabled
if (!rootSpanAttributes) return
Expand All @@ -847,13 +857,22 @@ export default abstract class Server<

const route = rootSpanAttributes.get('next.route')
if (route) {
const newName = `${rsc}${method} ${route}`
const name = isRSCRequest
? `RSC ${method} ${route}`
: `${method} ${route}`

span.setAttributes({
'next.route': route,
'http.route': route,
'next.span_name': newName,
'next.span_name': name,
})
span.updateName(newName)
span.updateName(name)
} else {
span.updateName(
isRSCRequest
? `RSC ${method} ${req.url}`
: `${method} ${req.url}`
)
}
})
)
Expand Down Expand Up @@ -928,11 +947,8 @@ export default abstract class Server<
// it captures the initial URL.
this.attachRequestMeta(req, parsedUrl)

let finished: boolean = false
if (this.minimalMode && this.enabledDirectories.app) {
finished = await this.handleRSCRequest(req, res, parsedUrl)
if (finished) return
}
let finished = await this.handleRSCRequest(req, res, parsedUrl)
if (finished) return

const domainLocale = this.i18nProvider?.detectDomainLocale(
getHostname(parsedUrl, req.headers)
Expand Down Expand Up @@ -3588,8 +3604,5 @@ export default abstract class Server<
}

export function isRSCRequestCheck(req: BaseNextRequest): boolean {
return (
req.headers[RSC_HEADER.toLowerCase()] === '1' ||
Boolean(getRequestMeta(req, 'isRSCRequest'))
)
return getRequestMeta(req, 'isRSCRequest') === true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { PrefetchRSCPathnameNormalizer } from './prefetch-rsc'

describe('PrefetchRSCPathnameNormalizer', () => {
const normalizer = new PrefetchRSCPathnameNormalizer()

it('should match the prefetch rsc pathname', () => {
expect(normalizer.match('/blog/post.prefetch.rsc')).toBe(true)
})

it('should not match the prefetch rsc pathname with a different suffix', () => {
expect(normalizer.match('/blog/post.prefetch.rsc2')).toBe(false)
})

it('should normalize the prefetch rsc pathname', () => {
expect(normalizer.normalize('/blog/post.prefetch.rsc')).toBe('/blog/post')
})

it('should normalize the prefetch rsc index pathname', () => {
expect(normalizer.normalize('/__index.prefetch.rsc')).toBe('/')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,20 @@ export class PrefetchRSCPathnameNormalizer
constructor() {
super(RSC_PREFETCH_SUFFIX)
}

public match(pathname: string): boolean {
if (pathname === '/__index' + RSC_PREFETCH_SUFFIX) {
return true
}

return super.match(pathname)
}

public normalize(pathname: string, matched?: boolean): string {
if (pathname === '/__index' + RSC_PREFETCH_SUFFIX) {
return '/'
}

return super.normalize(pathname, matched)
}
}
4 changes: 0 additions & 4 deletions packages/next/src/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1810,10 +1810,6 @@ export default class NextNodeServer extends BaseServer<
? `https://${req.headers.host || 'localhost'}${req.url}`
: req.url

const isRSC = isRSCRequestCheck(req)
if (isRSC) {
addRequestMeta(req, 'isRSCRequest', true)
}
addRequestMeta(req, 'initURL', initUrl)
addRequestMeta(req, 'initQuery', { ...parsedUrl.query })
addRequestMeta(req, 'initProtocol', protocol)
Expand Down

0 comments on commit 975d953

Please sign in to comment.