Skip to content

Commit

Permalink
Refactor internal routing headers to use request meta (#66987)
Browse files Browse the repository at this point in the history 
This refactors our handling of passing routing information to the render
logic via headers which is legacy from when we had separate routing and
render workers. Now this will just attach this meta in our normal
request meta handling which is more consistent and type safe.
  • Loading branch information
@ijjk @ztanner
ijjk authored and ztanner committed Aug 26, 2024
1 parent deeeb5f commit b5db704
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 121 deletions.
70 changes: 16 additions & 54 deletions packages/next/src/server/base-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ import {
} from './web/spec-extension/adapters/next-request'
import { matchNextDataPathname } from './lib/match-next-data-pathname'
import getRouteFromAssetPath from '../shared/lib/router/utils/get-route-from-asset-path'
import { stripInternalHeaders } from './internal-utils'
import { RSCPathnameNormalizer } from './future/normalizers/request/rsc'
import { PostponedPathnameNormalizer } from './future/normalizers/request/postponed'
import { ActionPathnameNormalizer } from './future/normalizers/request/action'
Expand Down Expand Up @@ -621,7 +620,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
// Ignore if its a middleware request when we aren't on edge.
if (
process.env.NEXT_RUNTIME !== 'edge' &&
req.headers['x-middleware-invoke']
getRequestMeta(req, 'middlewareInvoke')
) {
return false
}
Expand Down Expand Up @@ -941,7 +940,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
const useMatchedPathHeader =
this.minimalMode && typeof req.headers['x-matched-path'] === 'string'

// TODO: merge handling with x-invoke-path
// TODO: merge handling with invokePath
if (useMatchedPathHeader) {
try {
if (this.enabledDirectories.app) {
Expand Down Expand Up @@ -1245,35 +1244,26 @@ export default abstract class Server<ServerOptions extends Options = Options> {
;(globalThis as any).__incrementalCache = incrementalCache
}

// when x-invoke-path is specified we can short short circuit resolving
// when invokePath is specified we can short short circuit resolving
// we only honor this header if we are inside of a render worker to
// prevent external users coercing the routing path
const invokePath = req.headers['x-invoke-path'] as string
const invokePath = getRequestMeta(req, 'invokePath')
const useInvokePath =
!useMatchedPathHeader &&
process.env.NEXT_RUNTIME !== 'edge' &&
invokePath

if (useInvokePath) {
if (req.headers['x-invoke-status']) {
const invokeQuery = req.headers['x-invoke-query']
const invokeStatus = getRequestMeta(req, 'invokeStatus')
if (invokeStatus) {
const invokeQuery = getRequestMeta(req, 'invokeQuery')

if (typeof invokeQuery === 'string') {
Object.assign(
parsedUrl.query,
JSON.parse(decodeURIComponent(invokeQuery))
)
if (invokeQuery) {
Object.assign(parsedUrl.query, invokeQuery)
}

res.statusCode = Number(req.headers['x-invoke-status'])
let err: Error | null = null

if (typeof req.headers['x-invoke-error'] === 'string') {
const invokeError = JSON.parse(
req.headers['x-invoke-error'] || '{}'
)
err = new Error(invokeError.message)
}
res.statusCode = invokeStatus
let err: Error | null = getRequestMeta(req, 'invokeError') || null

return this.renderError(err, req, res, '/_error', parsedUrl.query)
}
Expand Down Expand Up @@ -1310,13 +1300,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
delete parsedUrl.query[key]
}
}
const invokeQuery = req.headers['x-invoke-query']
const invokeQuery = getRequestMeta(req, 'invokeQuery')

if (typeof invokeQuery === 'string') {
Object.assign(
parsedUrl.query,
JSON.parse(decodeURIComponent(invokeQuery))
)
if (invokeQuery) {
Object.assign(parsedUrl.query, invokeQuery)
}

finished = await this.normalizeAndAttachMetadata(req, res, parsedUrl)
Expand All @@ -1328,7 +1315,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {

if (
process.env.NEXT_RUNTIME !== 'edge' &&
req.headers['x-middleware-invoke']
getRequestMeta(req, 'middlewareInvoke')
) {
finished = await this.normalizeAndAttachMetadata(req, res, parsedUrl)
if (finished) return
Expand Down Expand Up @@ -1693,28 +1680,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
)
}

protected stripInternalHeaders(req: BaseNextRequest): void {
// Skip stripping internal headers in test mode while the header stripping
// has been explicitly disabled. This allows tests to verify internal
// routing behavior.
if (
process.env.__NEXT_TEST_MODE &&
process.env.__NEXT_NO_STRIP_INTERNAL_HEADERS === '1'
) {
return
}

// Strip the internal headers from both the request and the original
// request.
stripInternalHeaders(req.headers)
if (
'originalRequest' in req &&
'headers' in (req as NodeNextRequest).originalRequest
) {
stripInternalHeaders((req as NodeNextRequest).originalRequest.headers)
}
}

protected pathCouldBeIntercepted(resolvedPathname: string): boolean {
return (
isInterceptionRouteAppPath(resolvedPathname) ||
Expand Down Expand Up @@ -1762,9 +1727,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
const is404Page = pathname === '/404'

// Strip the internal headers.
this.stripInternalHeaders(req)

const is500Page = pathname === '/500'
const isAppPath = components.isAppPath === true

Expand Down Expand Up @@ -3089,7 +3051,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
for await (const match of this.matchers.matchAll(pathname, options)) {
// when a specific invoke-output is meant to be matched
// ensure a prior dynamic route/page doesn't take priority
const invokeOutput = ctx.req.headers['x-invoke-output']
const invokeOutput = getRequestMeta(ctx.req, 'invokeOutput')
if (
!this.minimalMode &&
typeof invokeOutput === 'string' &&
Expand Down
13 changes: 0 additions & 13 deletions packages/next/src/server/internal-utils.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import type { IncomingHttpHeaders } from 'http'
import type { NextParsedUrlQuery } from './request-meta'

import { NEXT_RSC_UNION_QUERY } from '../client/components/app-router-headers'
import { INTERNAL_HEADERS } from '../shared/lib/constants'

const INTERNAL_QUERY_NAMES = [
'__nextFallback',
Expand Down Expand Up @@ -39,14 +37,3 @@ export function stripInternalSearchParams<T extends string | URL>(

return (isStringUrl ? instance.toString() : instance) as T
}

/**
* Strip internal headers from the request headers.
*
* @param headers the headers to strip of internal headers
*/
export function stripInternalHeaders(headers: IncomingHttpHeaders) {
for (const key of INTERNAL_HEADERS) {
delete headers[key]
}
}
47 changes: 25 additions & 22 deletions packages/next/src/server/lib/router-server.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// this must come first as it includes require hooks
import type { WorkerRequestHandler, WorkerUpgradeHandler } from './types'
import type { DevBundler } from './router-utils/setup-dev-bundler'
import type { NextUrlWithParsedQuery } from '../request-meta'
import type { NextUrlWithParsedQuery, RequestMeta } from '../request-meta'
import type { NextServer } from '../next'

// This is required before other imports to ensure the require hook is setup.
Expand All @@ -19,7 +19,7 @@ import { setupFsCheck } from './router-utils/filesystem'
import { proxyRequest } from './router-utils/proxy-request'
import { isAbortError, pipeToNodeResponse } from '../pipe-readable'
import { getResolveRoutes } from './router-utils/resolve-routes'
import { getRequestMeta } from '../request-meta'
import { addRequestMeta, getRequestMeta } from '../request-meta'
import { pathHasPrefix } from '../../shared/lib/router/utils/path-has-prefix'
import { removePathPrefix } from '../../shared/lib/router/utils/remove-path-prefix'
import setupCompression from 'next/dist/compiled/compression'
Expand Down Expand Up @@ -216,7 +216,7 @@ export async function initialize(opts: {
parsedUrl: NextUrlWithParsedQuery,
invokePath: string,
handleIndex: number,
additionalInvokeHeaders: Record<string, string> = {}
additionalRequestMeta?: RequestMeta
) {
// invokeRender expects /api routes to not be locale prefixed
// so normalize here before continuing
Expand Down Expand Up @@ -247,16 +247,19 @@ export async function initialize(opts: {
throw new Error('Failed to initialize render server')
}

const invokeHeaders: typeof req.headers = {
...req.headers,
'x-middleware-invoke': '',
'x-invoke-path': invokePath,
'x-invoke-query': encodeURIComponent(JSON.stringify(parsedUrl.query)),
...(additionalInvokeHeaders || {}),
addRequestMeta(req, 'invokePath', invokePath)
addRequestMeta(req, 'invokeQuery', parsedUrl.query)
addRequestMeta(req, 'middlewareInvoke', false)

for (const key in additionalRequestMeta || {}) {
addRequestMeta(
req,
key as keyof RequestMeta,
additionalRequestMeta![key as keyof RequestMeta]
)
}
Object.assign(req.headers, invokeHeaders)

debug('invokeRender', req.url, invokeHeaders)
debug('invokeRender', req.url, req.headers)

try {
const initResult = await renderServer?.instance?.initialize(
Expand Down Expand Up @@ -404,10 +407,10 @@ export async function initialize(opts: {
) {
res.statusCode = 500
await invokeRender(parsedUrl, '/_error', handleIndex, {
'x-invoke-status': '500',
'x-invoke-error': JSON.stringify({
message: `A conflicting public file and page file was found for path ${matchedOutput.itemPath} https://nextjs.org/docs/messages/conflicting-public-file-page`,
}),
invokeStatus: 500,
invokeError: new Error(
`A conflicting public file and page file was found for path ${matchedOutput.itemPath} https://nextjs.org/docs/messages/conflicting-public-file-page`
),
})
return
}
Expand All @@ -433,7 +436,7 @@ export async function initialize(opts: {
'/405',
handleIndex,
{
'x-invoke-status': '405',
invokeStatus: 405,
}
)
}
Expand Down Expand Up @@ -491,14 +494,14 @@ export async function initialize(opts: {

if (typeof err.statusCode === 'number') {
const invokePath = `/${err.statusCode}`
const invokeStatus = `${err.statusCode}`
const invokeStatus = err.statusCode
res.statusCode = err.statusCode
return await invokeRender(
url.parse(invokePath, true),
invokePath,
handleIndex,
{
'x-invoke-status': invokeStatus,
invokeStatus,
}
)
}
Expand All @@ -514,7 +517,7 @@ export async function initialize(opts: {
parsedUrl.pathname || '/',
handleIndex,
{
'x-invoke-output': matchedOutput.itemPath,
invokeOutput: matchedOutput.itemPath,
}
)
}
Expand Down Expand Up @@ -544,13 +547,13 @@ export async function initialize(opts: {
UNDERSCORE_NOT_FOUND_ROUTE,
handleIndex,
{
'x-invoke-status': '404',
invokeStatus: 404,
}
)
}

await invokeRender(parsedUrl, '/404', handleIndex, {
'x-invoke-status': '404',
invokeStatus: 404,
})
}

Expand All @@ -569,7 +572,7 @@ export async function initialize(opts: {
}
res.statusCode = Number(invokeStatus)
return await invokeRender(url.parse(invokePath, true), invokePath, 0, {
'x-invoke-status': invokeStatus,
invokeStatus: res.statusCode,
})
} catch (err2) {
console.error(err2)
Expand Down
14 changes: 2 additions & 12 deletions packages/next/src/server/lib/router-utils/resolve-routes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,15 +460,8 @@ export function getResolveRoutes(
throw new Error(`Failed to initialize render server "middleware"`)
}

const invokeHeaders: typeof req.headers = {
'x-invoke-path': '',
'x-invoke-query': '',
'x-invoke-output': '',
'x-middleware-invoke': '1',
}
Object.assign(req.headers, invokeHeaders)

debug('invoking middleware', req.url, invokeHeaders)
addRequestMeta(req, 'middlewareInvoke', true)
debug('invoking middleware', req.url, req.headers)

let middlewareRes: Response | undefined = undefined
let bodyStream: ReadableStream | undefined = undefined
Expand Down Expand Up @@ -573,9 +566,6 @@ export function getResolveRoutes(
'x-middleware-rewrite',
'x-middleware-redirect',
'x-middleware-refresh',
'x-middleware-invoke',
'x-invoke-path',
'x-invoke-query',
].includes(key)
) {
continue
Expand Down
9 changes: 3 additions & 6 deletions packages/next/src/server/next-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,7 @@ export default class NextNodeServer extends BaseServer {
'originalResponse' in _res ? _res.originalResponse : _res

const reqStart = Date.now()
const isMiddlewareRequest = req.headers['x-middleware-invoke']
const isMiddlewareRequest = getRequestMeta(req, 'middlewareInvoke')

const reqCallback = () => {
// we don't log for non-route requests
Expand Down Expand Up @@ -1643,14 +1643,14 @@ export default class NextNodeServer extends BaseServer {
res: BaseNextResponse,
parsed: NextUrlWithParsedQuery
) => {
const isMiddlewareInvoke = req.headers['x-middleware-invoke']
const isMiddlewareInvoke = getRequestMeta(req, 'middlewareInvoke')

if (!isMiddlewareInvoke) {
return false
}

const handleFinished = () => {
res.setHeader('x-middleware-invoke', '1')
addRequestMeta(req, 'middlewareInvoke', true)
res.body('').send()
return true
}
Expand Down Expand Up @@ -1678,9 +1678,6 @@ export default class NextNodeServer extends BaseServer {
>
let bubblingResult = false

// Strip the internal headers.
this.stripInternalHeaders(req)

try {
await this.ensureMiddleware(req.url)

Expand Down
30 changes: 30 additions & 0 deletions packages/next/src/server/request-meta.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,36 @@ export interface RequestMeta {
* The previous revalidate before rendering 404 page for notFound: true
*/
notFoundRevalidate?: number | false

/**
* The path we routed to and should be invoked
*/
invokePath?: string

/**
* The specific page output we should be matching
*/
invokeOutput?: string

/**
* The status we are invoking the request with from routing
*/
invokeStatus?: number

/**
* The routing error we are invoking with
*/
invokeError?: Error

/**
* The query parsed for the invocation
*/
invokeQuery?: Record<string, undefined | string | string[]>

/**
* Whether the request is a middleware invocation
*/
middlewareInvoke?: boolean
}

/**
Expand Down
Loading

0 comments on commit b5db704

Please sign in to comment.