Refactor internal routing headers to use request meta (#66987)

ijjk · ztanner · commit b5db704240d5 · 2024-08-26T12:33:14.000-07:00
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.
diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts
@@ -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'
@@ -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
       }
@@ -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) {
@@ -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)
         }
@@ -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)
@@ -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
@@ -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) ||
@@ -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
 
@@ -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' &&
diff --git a/packages/next/src/server/internal-utils.ts b/packages/next/src/server/internal-utils.ts
@@ -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',
@@ -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]
-  }
-}
diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts
@@ -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.
@@ -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'
@@ -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
@@ -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(
@@ -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
         }
@@ -433,7 +436,7 @@ export async function initialize(opts: {
             '/405',
             handleIndex,
             {
-              'x-invoke-status': '405',
+              invokeStatus: 405,
             }
           )
         }
@@ -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,
               }
             )
           }
@@ -514,7 +517,7 @@ export async function initialize(opts: {
           parsedUrl.pathname || '/',
           handleIndex,
           {
-            'x-invoke-output': matchedOutput.itemPath,
+            invokeOutput: matchedOutput.itemPath,
           }
         )
       }
@@ -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,
       })
     }
 
@@ -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)
diff --git a/packages/next/src/server/lib/router-utils/resolve-routes.ts b/packages/next/src/server/lib/router-utils/resolve-routes.ts
@@ -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
@@ -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
diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts
@@ -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
@@ -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
     }
@@ -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)
 
diff --git a/packages/next/src/server/request-meta.ts b/packages/next/src/server/request-meta.ts
@@ -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
 }
 
 /**
diff --git a/packages/next/src/shared/lib/constants.ts b/packages/next/src/shared/lib/constants.ts
diff --git a/test/e2e/i18n-data-route/i18n-data-route.test.ts b/test/e2e/i18n-data-route/i18n-data-route.test.ts
