Skip to content

Commit

Permalink
Mask Flight Parameters from Middleware (#39939)
Browse files Browse the repository at this point in the history
This masks flight parameters from middleware so it doesn't interfere with RSC or routing.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)


Co-authored-by: Tim Neutkens <6324199+timneutkens@users.noreply.github.com>
  • Loading branch information
wyattjoh and timneutkens committed Sep 15, 2022
1 parent a8e54e7 commit 33a6dca
Show file tree
Hide file tree
Showing 13 changed files with 263 additions and 76 deletions.
42 changes: 23 additions & 19 deletions packages/next/client/components/app-router.client.tsx
Expand Up @@ -202,28 +202,32 @@ export default function AppRouter({
}

prefetched.add(href)

const url = new URL(href, location.origin)
// TODO-APP: handle case where history.state is not the new router history entry
const r = fetchServerResponse(
url,
// initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case.
window.history.state?.tree || initialTree,
true
)

try {
r.readRoot()
} catch (e) {
await e
const flightData = r.readRoot()
// @ts-ignore startTransition exists
React.startTransition(() => {
dispatch({
type: ACTION_PREFETCH,
url,
flightData,
// TODO-APP: handle case where history.state is not the new router history entry
const r = fetchServerResponse(
url,
// initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case.
window.history.state?.tree || initialTree,
true
)
try {
r.readRoot()
} catch (e) {
await e
const flightData = r.readRoot()
// @ts-ignore startTransition exists
React.startTransition(() => {
dispatch({
type: ACTION_PREFETCH,
url,
flightData,
})
})
})
}
} catch (err) {
console.error('PREFETCH ERROR', err)
}
},
replace: (href, options = {}) => {
Expand Down
3 changes: 2 additions & 1 deletion packages/next/server/app-render.tsx
Expand Up @@ -18,14 +18,15 @@ import {
} from './node-web-streams-helper'
import { isDynamicRoute } from '../shared/lib/router/utils'
import { ESCAPE_REGEX, htmlEscapeJsonString } from './htmlescape'
import { shouldUseReactRoot, stripInternalQueries } from './utils'
import { shouldUseReactRoot } from './utils'
import { NextApiRequestCookies } from './api-utils'
import { matchSegment } from '../client/components/match-segments'
import {
FlightCSSManifest,
FlightManifest,
} from '../build/webpack/plugins/flight-manifest-plugin'
import { FlushEffectsContext } from '../client/components/hooks-client'
import { stripInternalQueries } from './internal-utils'
import type { ComponentsType } from '../build/webpack/loaders/next-app-loader'

// this needs to be required lazily so that `next-server` can set
Expand Down
38 changes: 38 additions & 0 deletions packages/next/server/internal-utils.ts
@@ -0,0 +1,38 @@
import type { NextParsedUrlQuery } from './request-meta'

const INTERNAL_QUERY_NAMES = [
'__nextFallback',
'__nextLocale',
'__nextDefaultLocale',
'__nextIsNotFound',
// RSC
'__flight__',
// Routing
'__flight_router_state_tree__',
'__flight_prefetch__',
] as const

const EXTENDED_INTERNAL_QUERY_NAMES = ['__nextDataReq'] as const

export function stripInternalQueries(query: NextParsedUrlQuery) {
for (const name of INTERNAL_QUERY_NAMES) {
delete query[name]
}
}

export function stripInternalSearchParams(
searchParams: URLSearchParams,
extended?: boolean
) {
for (const name of INTERNAL_QUERY_NAMES) {
searchParams.delete(name)
}

if (extended) {
for (const name of EXTENDED_INTERNAL_QUERY_NAMES) {
searchParams.delete(name)
}
}

return searchParams
}
3 changes: 2 additions & 1 deletion packages/next/server/render.tsx
Expand Up @@ -79,7 +79,8 @@ import {
} from './node-web-streams-helper'
import { ImageConfigContext } from '../shared/lib/image-config-context'
import stripAnsi from 'next/dist/compiled/strip-ansi'
import { shouldUseReactRoot, stripInternalQueries } from './utils'
import { shouldUseReactRoot } from './utils'
import { stripInternalQueries } from './internal-utils'

let tryGetPreviewData: typeof import('./api-utils/node').tryGetPreviewData
let warn: typeof import('../build/output/log').warn
Expand Down
14 changes: 0 additions & 14 deletions packages/next/server/utils.ts
@@ -1,4 +1,3 @@
import type { NextParsedUrlQuery } from './request-meta'
import React from 'react'
import { BLOCKED_PAGES } from '../shared/lib/constants'

Expand All @@ -23,18 +22,5 @@ export function isTargetLikeServerless(target: string) {
return isServerless || isServerlessTrace
}

export function stripInternalQueries(query: NextParsedUrlQuery) {
delete query.__nextFallback
delete query.__nextLocale
delete query.__nextDefaultLocale
delete query.__nextIsNotFound

// RSC
delete query.__flight__
delete query.__props__
// routing
delete query.__flight_router_state_tree__
}

// When react version is >= 18 opt-in using reactRoot
export const shouldUseReactRoot = parseInt(React.version) >= 18
21 changes: 16 additions & 5 deletions packages/next/server/web/adapter.ts
Expand Up @@ -8,6 +8,7 @@ import { NextResponse } from './spec-extension/response'
import { relativizeURL } from '../../shared/lib/router/utils/relativize-url'
import { waitUntilSymbol } from './spec-extension/fetch-event'
import { NextURL } from './next-url'
import { stripInternalSearchParams } from '../internal-utils'

class NextRequestHint extends NextRequest {
sourcePage: string
Expand Down Expand Up @@ -39,6 +40,9 @@ export async function adapter(params: {
page: string
request: RequestData
}): Promise<FetchEventResult> {
// TODO-APP: use explicit marker for this
const isEdgeRendering = typeof self.__BUILD_MANIFEST !== 'undefined'

const requestUrl = new NextURL(params.request.url, {
headers: params.request.headers,
nextConfig: params.request.nextConfig,
Expand All @@ -54,13 +58,16 @@ export async function adapter(params: {
requestUrl.pathname = '/'
}

// clean-up any internal query params
for (const key of [...requestUrl.searchParams.keys()]) {
if (key.startsWith('__next')) {
requestUrl.searchParams.delete(key)
}
// Preserve flight data.
const flightSearchParameters = requestUrl.flightSearchParameters
// Parameters should only be stripped for middleware
if (!isEdgeRendering) {
requestUrl.flightSearchParameters = undefined
}

// Strip internal query parameters off the request.
stripInternalSearchParams(requestUrl.searchParams, true)

const request = new NextRequestHint({
page: params.page,
input: String(requestUrl),
Expand Down Expand Up @@ -105,6 +112,8 @@ export async function adapter(params: {

if (rewriteUrl.host === request.nextUrl.host) {
rewriteUrl.buildId = buildId || rewriteUrl.buildId
rewriteUrl.flightSearchParameters =
flightSearchParameters || rewriteUrl.flightSearchParameters
response.headers.set('x-middleware-rewrite', String(rewriteUrl))
}

Expand Down Expand Up @@ -142,6 +151,8 @@ export async function adapter(params: {

if (redirectURL.host === request.nextUrl.host) {
redirectURL.buildId = buildId || redirectURL.buildId
redirectURL.flightSearchParameters =
flightSearchParameters || redirectURL.flightSearchParameters
response.headers.set('Location', String(redirectURL))
}

Expand Down
80 changes: 79 additions & 1 deletion packages/next/server/web/next-url.ts
Expand Up @@ -15,6 +15,12 @@ interface Options {
}
}

const FLIGHT_PARAMETERS = [
'__flight__',
'__flight_router_state_tree__',
'__flight_prefetch__',
] as const

const REGEX_LOCALHOST_HOSTNAME =
/(?!^https?:\/\/)(127(?:\.(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}|::1|localhost)/

Expand All @@ -25,12 +31,35 @@ function parseURL(url: string | URL, base?: string | URL) {
)
}

function parseFlightParameters(
searchParams: URLSearchParams
): Record<string, string> | undefined {
let flightSearchParameters: Record<string, string> = {}
let flightSearchParametersUpdated = false
for (const name of FLIGHT_PARAMETERS) {
const value = searchParams.get(name)
if (value === null) {
continue
}

flightSearchParameters[name] = value
flightSearchParametersUpdated = true
}

if (!flightSearchParametersUpdated) {
return undefined
}

return flightSearchParameters
}

const Internal = Symbol('NextURLInternal')

export class NextURL {
[Internal]: {
basePath: string
buildId?: string
flightSearchParameters?: Record<string, string>
defaultLocale?: string
domainLocale?: DomainLocale
locale?: string
Expand Down Expand Up @@ -89,6 +118,9 @@ export class NextURL {
this[Internal].buildId = pathnameInfo.buildId
this[Internal].locale = pathnameInfo.locale ?? defaultLocale
this[Internal].trailingSlash = pathnameInfo.trailingSlash
this[Internal].flightSearchParameters = parseFlightParameters(
this[Internal].url.searchParams
)
}

private formatPathname() {
Expand All @@ -104,6 +136,25 @@ export class NextURL {
})
}

private formatSearch() {
const flightSearchParameters = this[Internal].flightSearchParameters
// If no flight parameters are set, return the search string as is.
// This is a fast path to ensure URLSearchParams only has to be recreated on Flight requests.
if (!flightSearchParameters) {
return this[Internal].url.search
}

// Create separate URLSearchParams to ensure the original search string is not modified.
const searchParams = new URLSearchParams(this[Internal].url.searchParams)
// If any exist this loop is always limited to the amount of FLIGHT_PARAMETERS.
for (const name in flightSearchParameters) {
searchParams.set(name, flightSearchParameters[name])
}

const params = searchParams.toString()
return params === '' ? '' : `?${params}`
}

public get buildId() {
return this[Internal].buildId
}
Expand All @@ -112,6 +163,32 @@ export class NextURL {
this[Internal].buildId = buildId
}

public get flightSearchParameters() {
return this[Internal].flightSearchParameters
}

public set flightSearchParameters(
flightSearchParams: Record<string, string> | undefined
) {
if (flightSearchParams) {
for (const name of FLIGHT_PARAMETERS) {
// Ensure only the provided values are set
if (flightSearchParams[name]) {
this[Internal].url.searchParams.set(name, flightSearchParams[name])
} else {
// Delete the ones that are not provided as flightData should be overridden.
this[Internal].url.searchParams.delete(name)
}
}
} else {
for (const name of FLIGHT_PARAMETERS) {
this[Internal].url.searchParams.delete(name)
}
}

this[Internal].flightSearchParameters = flightSearchParams
}

public get locale() {
return this[Internal].locale ?? ''
}
Expand Down Expand Up @@ -175,7 +252,8 @@ export class NextURL {

get href() {
const pathname = this.formatPathname()
return `${this.protocol}//${this.host}${pathname}${this[Internal].url.search}`
const search = this.formatSearch()
return `${this.protocol}//${this.host}${pathname}${search}`
}

set href(url: string) {
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app/app/internal/failure/page.server.js
@@ -0,0 +1,3 @@
export default function Page() {
return <div id="failure">Failure</div>
}
18 changes: 18 additions & 0 deletions test/e2e/app-dir/app/app/internal/page.server.js
@@ -0,0 +1,18 @@
import Link from 'next/link'

export default function Page() {
return (
<div>
<div>
<Link href="/internal/test/rewrite">
<a id="navigate-rewrite">Navigate Rewrite</a>
</Link>
</div>
<div>
<Link href="/internal/test/redirect">
<a id="navigate-redirect">Navigate Redirect</a>
</Link>
</div>
</div>
)
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app/app/internal/success/page.server.js
@@ -0,0 +1,3 @@
export default function Page() {
return <div id="success">Success</div>
}
19 changes: 18 additions & 1 deletion test/e2e/app-dir/app/middleware.js
@@ -1,8 +1,25 @@
// @ts-check
import { NextResponse } from 'next/server'

/**
* @param {import('next/server').NextRequest} request
* @returns {NextResponse | undefined}
*/
export function middleware(request) {
if (request.nextUrl.pathname === '/middleware-to-dashboard') {
// TODO: this does not copy __flight__ and __flight_router_state_tree__
return NextResponse.rewrite(new URL('/dashboard', request.url))
}

if (request.nextUrl.pathname.startsWith('/internal/test')) {
const method = request.nextUrl.pathname.endsWith('rewrite')
? 'rewrite'
: 'redirect'

const internal = ['__flight__', '__flight_router_state_tree__']
if (internal.some((name) => request.nextUrl.searchParams.has(name))) {
return NextResponse[method](new URL('/internal/failure', request.url))
}

return NextResponse[method](new URL('/internal/success', request.url))
}
}

0 comments on commit 33a6dca

Please sign in to comment.