fix(app-router): validate interception context identity - #3078
Conversation
|
/bigbonk review for issues Please limit this review to 7 minutes. |
commit: |
|
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
![ask-bonk[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| setInterceptionResponseUncacheable(true); | ||
| return badRequestResponse(); | ||
| } | ||
| const bypassInterceptionContextCache = |
There was a problem hiding this comment.
[P1] Revalidate interception proof after late rewrites
This proof is fixed before the afterFiles/fallback loops below. If the original target has a dynamic App match and a concrete intercept, it passes here; an afterFiles rewrite can then change cleanPathname to a different page, but bypassInterceptionContextCache remains false even though the generated findIntercept no longer matches that final target. The selector/context-bearing response can therefore be admitted to ISR/CDN without final target-specific proof (and context-only dynamic sources can fan out variants again). Recompute matchInterceptRoute after all late rewrites, rejecting an ID or forcing the context-only path to bypass caches before dispatch.
There was a problem hiding this comment.
Fixed in 19e3561. Late rewrite matches now invalidate the earlier proof immediately, and the final App target is revalidated before dispatch. Context-only mismatches bypass CDN/ISR caches; selector-bearing mismatches return 400. Added regressions for both paths.
|
Posted 1 actionable inline finding. |
|
/bigbonk review for issues Please review the cumulative diff, including the late-rewrite proof fix, and limit this review to 7 minutes. |
| setInterceptionResponseUncacheable(true); | ||
| return badRequestResponse(); | ||
| } | ||
| bypassInterceptionContextCache = !hasVerifiedFinalInterceptionSource; |
There was a problem hiding this comment.
[P1] Authorize interception sources discovered after late rewrites
When the original dynamic target has no intercept for this context, interceptionSourceMatch is null, so the source-middleware pass above is skipped. An afterFiles/fallback rewrite can then land on a target for which finalInterceptionSourceMatch is concrete; this branch restores cacheability, and dispatchMatchedPage subsequently resolves that final intercept and renders its source route. Middleware protecting the source pathname never ran. Re-run source authorization for a newly selected final match (or keep it unverified/non-intercepting) before accepting this proof.
There was a problem hiding this comment.
Fixed in 09f6df8. Final proof after a late rewrite is accepted only when it is concrete and matches the route and params of the source already authorized before the rewrite. Newly discovered sources remain unverified/no-store. Added regressions for newly discovered and previously authorized sources.
|
Posted 1 actionable inline finding. |
|
/bigbonk review for issues Please review the complete updated diff and limit this review to 7 minutes. |
|
@james-elicx Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
|
@james-elicx Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
|
/bigbonk review for issues Please review the complete diff at head 09e30a2, including the new real-HTTP regression, and limit this review to 7 minutes. |
1 participant
Summary
Next.js parity
Validation
vp checkon all changed filesorigin/mainin both dev and production (200 with guarded content exposed) and passes on this branch (400 with no guarded content)vp run vinext#build