From 34bb5769da2a1831d5bc914db5418eb6a2e1a519 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 2 Jan 2025 14:12:44 +0100 Subject: [PATCH 01/21] ee -> events --- .../core-http-router-server-internal/src/router.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index d1640a4db6db1..dcf7b1a63e3e4 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -189,7 +189,10 @@ type RouterEvents = export class Router implements IRouter { - private static ee = new EventEmitter(); + /** + * Used for global request events at the router level, similar to what we get from Hapi's request lifecycle events. + */ + private static events = new EventEmitter(); public routes: Array> = []; public pluginId?: symbol; public get: InternalRegistrar<'get', Context>; @@ -253,11 +256,11 @@ export class Router void) { - Router.ee.on(event, cb); + Router.events.on(event, cb); } public static off(event: RouterEvents, cb: (req: CoreKibanaRequest, ...args: any[]) => void) { - Router.ee.off(event, cb); + Router.events.off(event, cb); } public getRoutes({ excludeVersionedRoutes }: { excludeVersionedRoutes?: boolean } = {}) { @@ -298,7 +301,7 @@ export class Router { const postValidate: RouterEvents = 'onPostValidate'; - Router.ee.emit(postValidate, request, postValidateConext); + Router.events.emit(postValidate, request, postValidateConext); }; private async handle({ From f50d4cc8e824b197e9be78131ac19ef9027a5e3b Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 2 Jan 2025 14:13:20 +0100 Subject: [PATCH 02/21] added link to router events interface --- .../core/http/core-http-router-server-internal/src/router.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index dcf7b1a63e3e4..28880b3b35f19 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -191,6 +191,8 @@ export class Router> = []; From 495b9e4f860c10cf6f139a1dd21fe4b76fa84220 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 2 Jan 2025 17:33:18 +0100 Subject: [PATCH 03/21] export http protocol detection logic --- .../core-http-router-server-internal/src/request.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/core/http/core-http-router-server-internal/src/request.ts b/packages/core/http/core-http-router-server-internal/src/request.ts index cbccf31fc0946..7873e37628a4d 100644 --- a/packages/core/http/core-http-router-server-internal/src/request.ts +++ b/packages/core/http/core-http-router-server-internal/src/request.ts @@ -191,7 +191,7 @@ export class CoreKibanaRequest< enumerable: false, }); - this.httpVersion = isRealReq ? request.raw.req.httpVersion : '1.0'; + this.httpVersion = isRealReq ? getHttpVersionFromRequest(request) : '1.0'; this.apiVersion = undefined; this.protocol = getProtocolFromHttpVersion(this.httpVersion); @@ -424,3 +424,11 @@ function sanitizeRequest(req: Request): { query: unknown; params: unknown; body: function getProtocolFromHttpVersion(httpVersion: string): HttpProtocol { return httpVersion.split('.')[0] === '2' ? 'http2' : 'http1'; } + +function getHttpVersionFromRequest(request: Request) { + return request.raw.req.httpVersion; +} + +export function getProtocolFromRequest(request: Request) { + return getProtocolFromHttpVersion(getHttpVersionFromRequest(request)); +} From f5670a17ca86470e5f9a059dd763bb5e46e38b34 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 11:24:29 +0100 Subject: [PATCH 04/21] wip 1 --- .../src/route.ts | 114 +++++++++++++++++- .../src/router.ts | 42 ++----- .../src/util.ts | 33 ++++- 3 files changed, 156 insertions(+), 33 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/route.ts b/packages/core/http/core-http-router-server-internal/src/route.ts index 6faae2c1816b9..4b283d7cc4f7c 100644 --- a/packages/core/http/core-http-router-server-internal/src/route.ts +++ b/packages/core/http/core-http-router-server-internal/src/route.ts @@ -7,8 +7,24 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import type { RouteMethod, SafeRouteMethod, RouteConfig } from '@kbn/core-http-server'; +import { + type RouteMethod, + type SafeRouteMethod, + type RouteConfig, + type KibanaRequest, + getRequestValidation, +} from '@kbn/core-http-server'; import type { RouteSecurityGetter, RouteSecurity } from '@kbn/core-http-server'; +import type { Request } from '@hapi/hapi'; +import { isConfigSchema } from '@kbn/config-schema'; +import { isZod } from '@kbn/zod'; +import type { Logger } from '@kbn/logging'; +import { RequestHandlerEnhanced, Router } from './router'; +import { CoreKibanaRequest } from './request'; +import { RouteValidator } from './validator'; +import { BASE_PUBLIC_VERSION } from './versioned_router'; +import { kibanaResponseFactory } from './response'; +import { getVersionHeader, injectVersionHeader, formatErrorMeta } from './util'; export function isSafeMethod(method: RouteMethod): method is SafeRouteMethod { return method === 'get' || method === 'options'; @@ -21,3 +37,99 @@ export type InternalRouteConfig = Omit< > & { security?: RouteSecurityGetter | RouteSecurity; }; + +/** @internal */ +interface InternalRouteHandlerArgs { + router: Router; + route: InternalRouteConfig; + request: Request; + log: Logger; + handler: RequestHandlerEnhanced; +} + +/** @internal */ +export async function handle({ + router, + route, + request, + handler, + log, +}: InternalRouteHandlerArgs) { + const routeSchemas = routeSchemasFromRouteConfig(route, request.method as Method); + + let kibanaRequest: KibanaRequest< + P, + Q, + B, + typeof request.method extends RouteMethod ? typeof request.method : any + >; + try { + kibanaRequest = CoreKibanaRequest.from(request, routeSchemas); + } catch (error) { + log.error('400 Bad Request', formatErrorMeta(400, { request, error })); + const response = kibanaResponseFactory.badRequest({ + body: error.message, + headers: isPublicAccessRoute(route) ? getVersionHeader(BASE_PUBLIC_VERSION) : undefined, + }); + + // Emit onPostValidation even if validation fails. + const req = CoreKibanaRequest.from(request); + router.emitPostValidate(req, { + deprecated: req.route.options.deprecated, + isInternalApiRequest: req.isInternalApiRequest, + isPublicAccess: isPublicAccessRoute(route), + }); + return response; + } + + router.emitPostValidate(kibanaRequest, { + deprecated: kibanaRequest.route.options.deprecated, + isInternalApiRequest: kibanaRequest.isInternalApiRequest, + isPublicAccess: isPublicAccessRoute(route), + }); + + const kibanaResponse = await handler(kibanaRequest, kibanaResponseFactory); + + if (isPublicAccessRoute(route)) { + injectVersionHeader(BASE_PUBLIC_VERSION, kibanaResponse); + } + + return kibanaResponse; +} + +function isPublicAccessRoute({ + options, +}: InternalRouteConfig): boolean { + return options?.access === 'public'; +} + +/** + * Create the validation schemas for a route + * + * @returns Route schemas if `validate` is specified on the route, otherwise + * undefined. + */ +function routeSchemasFromRouteConfig( + route: InternalRouteConfig, + routeMethod: RouteMethod +) { + // The type doesn't allow `validate` to be undefined, but it can still + // happen when it's used from JavaScript. + if (route.validate === undefined) { + throw new Error( + `The [${routeMethod}] at [${route.path}] does not have a 'validate' specified. Use 'false' as the value if you want to bypass validation.` + ); + } + + if (route.validate !== false) { + const validation = getRequestValidation(route.validate); + Object.entries(validation).forEach(([key, schema]) => { + if (!(isConfigSchema(schema) || isZod(schema) || typeof schema === 'function')) { + throw new Error( + `Expected a valid validation logic declared with '@kbn/config-schema' package, '@kbn/zod' package or a RouteValidationFunction at key: [${key}].` + ); + } + }); + return RouteValidator.from(validation); + } +} diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index 28880b3b35f19..a16781b2d67bd 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -41,7 +41,14 @@ import { kibanaResponseFactory } from './response'; import { HapiResponseAdapter } from './response_adapter'; import { wrapErrors } from './error_wrapper'; import { Method } from './versioned_router/types'; -import { getVersionHeader, injectVersionHeader, prepareRouteConfigValidation } from './util'; +import { + getVersionHeader, + injectVersionHeader, + prepareRouteConfigValidation, + getRouteFullPath, + isSafeMethod, + formatErrorMeta, +} from './util'; import { stripIllegalHttp2Headers } from './strip_illegal_http2_headers'; import { validRouteSecurity } from './security_route_config_validator'; import { InternalRouteConfig } from './route'; @@ -54,13 +61,6 @@ export type ContextEnhancer< Context extends RequestHandlerContextBase > = (handler: RequestHandler) => RequestHandlerEnhanced; -export function getRouteFullPath(routerPath: string, routePath: string) { - // If router's path ends with slash and route's path starts with slash, - // we should omit one of them to have a valid concatenated path. - const routePathStartIndex = routerPath.endsWith('/') && routePath.startsWith('/') ? 1 : 0; - return `${routerPath}${routePath.slice(routePathStartIndex)}`; -} - /** * Create the validation schemas for a route * @@ -274,26 +274,6 @@ export class Router Date: Fri, 3 Jan 2025 11:24:52 +0100 Subject: [PATCH 05/21] remove unused import --- .../core/http/core-http-router-server-internal/src/router.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index a16781b2d67bd..0592a45d71455 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -46,7 +46,6 @@ import { injectVersionHeader, prepareRouteConfigValidation, getRouteFullPath, - isSafeMethod, formatErrorMeta, } from './util'; import { stripIllegalHttp2Headers } from './strip_illegal_http2_headers'; From 14bdbcbe52309792ec9aed8128dd7f8159e5fe9c Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 12:07:33 +0100 Subject: [PATCH 06/21] wip 2 --- .../src/route.ts | 78 +++++-- .../src/router.ts | 207 ++++-------------- .../src/util.ts | 45 ++++ packages/core/http/core-http-server/index.ts | 1 + .../http/core-http-server/src/router/index.ts | 1 + .../core-http-server/src/router/request.ts | 5 + 6 files changed, 151 insertions(+), 186 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/route.ts b/packages/core/http/core-http-router-server-internal/src/route.ts index 4b283d7cc4f7c..18e6c1b295e5c 100644 --- a/packages/core/http/core-http-router-server-internal/src/route.ts +++ b/packages/core/http/core-http-router-server-internal/src/route.ts @@ -11,20 +11,28 @@ import { type RouteMethod, type SafeRouteMethod, type RouteConfig, - type KibanaRequest, getRequestValidation, } from '@kbn/core-http-server'; -import type { RouteSecurityGetter, RouteSecurity } from '@kbn/core-http-server'; -import type { Request } from '@hapi/hapi'; +import type { RouteSecurityGetter, RouteSecurity, AnyKibanaRequest } from '@kbn/core-http-server'; import { isConfigSchema } from '@kbn/config-schema'; import { isZod } from '@kbn/zod'; import type { Logger } from '@kbn/logging'; -import { RequestHandlerEnhanced, Router } from './router'; +import type { DeepPartial } from '@kbn/utility-types'; +import { Request } from '@hapi/hapi'; +import type { InternalRouterRoute, RequestHandlerEnhanced, Router } from './router'; import { CoreKibanaRequest } from './request'; import { RouteValidator } from './validator'; import { BASE_PUBLIC_VERSION } from './versioned_router'; import { kibanaResponseFactory } from './response'; -import { getVersionHeader, injectVersionHeader, formatErrorMeta } from './util'; +import { + getVersionHeader, + injectVersionHeader, + formatErrorMeta, + getRouteFullPath, + validOptions, + prepareRouteConfigValidation, +} from './util'; +import { validRouteSecurity } from './security_route_config_validator'; export function isSafeMethod(method: RouteMethod): method is SafeRouteMethod { return method === 'get' || method === 'options'; @@ -39,30 +47,54 @@ export type InternalRouteConfig = Omit< }; /** @internal */ -interface InternalRouteHandlerArgs { +interface Dependencies { router: Router; - route: InternalRouteConfig; - request: Request; + route: InternalRouteConfig; + handler: RequestHandlerEnhanced; log: Logger; - handler: RequestHandlerEnhanced; + method: RouteMethod; } -/** @internal */ -export async function handle({ - router, - route, - request, +export function buildRoute({ handler, log, -}: InternalRouteHandlerArgs) { - const routeSchemas = routeSchemasFromRouteConfig(route, request.method as Method); + route, + router, + method, +}: Dependencies): InternalRouterRoute { + route = prepareRouteConfigValidation(route); + const routeSchemas = routeSchemasFromRouteConfig(route, method); + return { + handler: async (req) => { + return await handle(req, { + handler, + log, + method, + route, + router, + routeSchemas, + }); + }, + method, + path: getRouteFullPath(router.routerPath, route.path), + options: validOptions(method, route), + security: validRouteSecurity(route.security as DeepPartial, route.options), + validationSchemas: route.validate, + isVersioned: false, + }; +} - let kibanaRequest: KibanaRequest< - P, - Q, - B, - typeof request.method extends RouteMethod ? typeof request.method : any - >; +/** @internal */ +interface HandlerDependencies extends Dependencies { + routeSchemas?: RouteValidator; +} + +/** @internal */ +export const handle = async ( + request: Request, + { router, route, handler, routeSchemas, log }: HandlerDependencies +) => { + let kibanaRequest: AnyKibanaRequest; try { kibanaRequest = CoreKibanaRequest.from(request, routeSchemas); } catch (error) { @@ -95,7 +127,7 @@ export async function handle({ } return kibanaResponse; -} +}; function isPublicAccessRoute({ options, diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index 0592a45d71455..6f3728a0ea856 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -27,30 +27,22 @@ import type { RequestHandler, VersionedRouter, RouteRegistrar, - RouteSecurity, PostValidationMetadata, + IKibanaResponse, } from '@kbn/core-http-server'; import { isZod } from '@kbn/zod'; -import { validBodyOutput, getRequestValidation } from '@kbn/core-http-server'; +import { getRequestValidation } from '@kbn/core-http-server'; import type { RouteSecurityGetter } from '@kbn/core-http-server'; -import type { DeepPartial } from '@kbn/utility-types'; import { RouteValidator } from './validator'; -import { BASE_PUBLIC_VERSION, CoreVersionedRouter } from './versioned_router'; -import { CoreKibanaRequest } from './request'; +import { CoreVersionedRouter } from './versioned_router'; +import { CoreKibanaRequest, getProtocolFromRequest } from './request'; import { kibanaResponseFactory } from './response'; import { HapiResponseAdapter } from './response_adapter'; import { wrapErrors } from './error_wrapper'; import { Method } from './versioned_router/types'; -import { - getVersionHeader, - injectVersionHeader, - prepareRouteConfigValidation, - getRouteFullPath, - formatErrorMeta, -} from './util'; +import { formatErrorMeta } from './util'; import { stripIllegalHttp2Headers } from './strip_illegal_http2_headers'; -import { validRouteSecurity } from './security_route_config_validator'; -import { InternalRouteConfig } from './route'; +import { InternalRouteConfig, buildRoute } from './route'; export type ContextEnhancer< P, @@ -60,6 +52,15 @@ export type ContextEnhancer< Context extends RequestHandlerContextBase > = (handler: RequestHandler) => RequestHandlerEnhanced; +/** + * @internal + */ +export type InternalRouteHandler = (request: Request) => Promise; + +export type InternalRouterRoute = Omit & { + handler: InternalRouteHandler; +}; + /** * Create the validation schemas for a route * @@ -91,49 +92,6 @@ function routeSchemasFromRouteConfig( } } -/** - * Create a valid options object with "sensible" defaults + adding some validation to the options fields - * - * @param method HTTP verb for these options - * @param routeConfig The route config definition - */ -function validOptions( - method: RouteMethod, - routeConfig: InternalRouteConfig -) { - const shouldNotHavePayload = ['head', 'get'].includes(method); - const { options = {}, validate } = routeConfig; - const shouldValidateBody = (validate && !!getRequestValidation(validate).body) || !!options.body; - - const { output } = options.body || {}; - if (typeof output === 'string' && !validBodyOutput.includes(output)) { - throw new Error( - `[options.body.output: '${output}'] in route ${method.toUpperCase()} ${ - routeConfig.path - } is not valid. Only '${validBodyOutput.join("' or '")}' are valid.` - ); - } - - // @ts-expect-error to eliminate problems with `security` in the options for route factories abstractions - if (options.security) { - throw new Error('`options.security` is not allowed in route config. Use `security` instead.'); - } - - const body = shouldNotHavePayload - ? undefined - : { - // If it's not a GET (requires payload) but no body validation is required (or no body options are specified), - // We assume the route does not care about the body => use the memory-cheapest approach (stream and no parsing) - output: !shouldValidateBody ? ('stream' as const) : undefined, - parse: !shouldValidateBody ? false : undefined, - - // User's settings should overwrite any of the "desired" values - ...options.body, - }; - - return { ...options, body }; -} - /** @internal */ export interface RouterOptions { /** Whether we are running in development */ @@ -151,17 +109,6 @@ export interface RouterOptions { }; } -/** @internal */ -export interface InternalRegistrarOptions { - /** @default false */ - isVersioned: boolean; - /** - * Whether this route should emit "route events" like postValidate - * @default true - */ - events: boolean; -} - /** @internal */ export type VersionedRouteConfig = Omit< RouteConfig, @@ -173,8 +120,7 @@ export type VersionedRouteConfig = Omit< /** @internal */ export type InternalRegistrar = ( route: InternalRouteConfig, - handler: RequestHandler, - internalOpts?: InternalRegistrarOptions + handler: RequestHandler ) => ReturnType>; /** @internal */ @@ -196,11 +142,11 @@ export class Router> = []; public pluginId?: symbol; - public get: InternalRegistrar<'get', Context>; - public post: InternalRegistrar<'post', Context>; - public delete: InternalRegistrar<'delete', Context>; - public put: InternalRegistrar<'put', Context>; - public patch: InternalRegistrar<'patch', Context>; + public get: RouteRegistrar<'get', Context>; + public post: RouteRegistrar<'post', Context>; + public delete: RouteRegistrar<'delete', Context>; + public put: RouteRegistrar<'put', Context>; + public patch: RouteRegistrar<'patch', Context>; constructor( public readonly routerPath: string, @@ -213,40 +159,17 @@ export class Router(method: Method) => ( route: InternalRouteConfig, - handler: RequestHandler, - { isVersioned, events }: InternalRegistrarOptions = { isVersioned: false, events: true } + handler: RequestHandler ) => { - route = prepareRouteConfigValidation(route); - const routeSchemas = routeSchemasFromRouteConfig(route, method); - const isPublicUnversionedRoute = - !isVersioned && - route.options?.access === 'public' && - // We do not consider HTTP resource routes as APIs - route.options?.httpResource !== true; - - this.routes.push({ - handler: async (req, responseToolkit) => { - return await this.handle({ - routeSchemas, - request: req, - responseToolkit, - isPublicUnversionedRoute, - handler: this.enhanceWithContext(handler), - emit: events ? { onPostValidation: this.emitPostValidate } : undefined, - }); - }, - method, - path: getRouteFullPath(this.routerPath, route.path), - options: validOptions(method, route), - // For the versioned route security is validated in the versioned router - security: isVersioned - ? route.security - : validRouteSecurity(route.security as DeepPartial, route.options), - validationSchemas: route.validate, - // @ts-ignore using isVersioned: false in the type instead of boolean - // for typeguarding between versioned and unversioned RouterRoute types - isVersioned, - }); + this.registerRoute( + buildRoute({ + handler: this.enhanceWithContext(handler), + log: this.log, + method, + route, + router: this, + }) + ); }; this.get = buildMethod('get'); @@ -285,70 +208,28 @@ export class Router + await this.handle({ request, responseToolkit, handler: route.handler }), + }); + } + private async handle({ - routeSchemas, request, responseToolkit, - emit, - isPublicUnversionedRoute, handler, }: { request: Request; responseToolkit: ResponseToolkit; - emit?: { - onPostValidation: (req: KibanaRequest, metadata: PostValidationMetadata) => void; - }; - isPublicUnversionedRoute: boolean; - handler: RequestHandlerEnhanced< - P, - Q, - B, - // request.method's type contains way more verbs than we currently support - typeof request.method extends RouteMethod ? typeof request.method : any - >; - routeSchemas?: RouteValidator; + handler: InternalRouteHandler; }) { - let kibanaRequest: KibanaRequest< - P, - Q, - B, - typeof request.method extends RouteMethod ? typeof request.method : any - >; const hapiResponseAdapter = new HapiResponseAdapter(responseToolkit); try { - kibanaRequest = CoreKibanaRequest.from(request, routeSchemas); - } catch (error) { - this.log.error('400 Bad Request', formatErrorMeta(400, { request, error })); - const response = hapiResponseAdapter.toBadRequest(error.message); - if (isPublicUnversionedRoute) { - response.output.headers = { - ...response.output.headers, - ...getVersionHeader(BASE_PUBLIC_VERSION), - }; - } - - // Emit onPostValidation even if validation fails. - const req = CoreKibanaRequest.from(request); - emit?.onPostValidation(req, { - deprecated: req.route.options.deprecated, - isInternalApiRequest: req.isInternalApiRequest, - isPublicAccess: req.route.options.access === 'public', - }); - return response; - } - - emit?.onPostValidation(kibanaRequest, { - deprecated: kibanaRequest.route.options.deprecated, - isInternalApiRequest: kibanaRequest.isInternalApiRequest, - isPublicAccess: kibanaRequest.route.options.access === 'public', - }); - - try { - const kibanaResponse = await handler(kibanaRequest, kibanaResponseFactory); - if (isPublicUnversionedRoute) { - injectVersionHeader(BASE_PUBLIC_VERSION, kibanaResponse); - } - if (kibanaRequest.protocol === 'http2' && kibanaResponse.options.headers) { + const kibanaResponse = await handler(request); + if (getProtocolFromRequest(request) === 'http2' && kibanaResponse.options.headers) { kibanaResponse.options.headers = stripIllegalHttp2Headers({ headers: kibanaResponse.options.headers, isDev: this.options.isDev ?? false, @@ -408,6 +289,6 @@ type WithoutHeadArgument = T extends (first: any, ...rest: infer Params) => i ? (...rest: Params) => Return : never; -type RequestHandlerEnhanced = WithoutHeadArgument< +export type RequestHandlerEnhanced = WithoutHeadArgument< RequestHandler >; diff --git a/packages/core/http/core-http-router-server-internal/src/util.ts b/packages/core/http/core-http-router-server-internal/src/util.ts index 2877fed211586..fa90af60058ec 100644 --- a/packages/core/http/core-http-router-server-internal/src/util.ts +++ b/packages/core/http/core-http-router-server-internal/src/util.ts @@ -13,6 +13,8 @@ import { type RouteValidatorFullConfigResponse, type RouteMethod, type RouteValidator, + getRequestValidation, + validBodyOutput, } from '@kbn/core-http-server'; import type { Mutable } from 'utility-types'; import type { IKibanaResponse, ResponseHeaders, SafeRouteMethod } from '@kbn/core-http-server'; @@ -123,3 +125,46 @@ export function getRouteFullPath(routerPath: string, routePath: string) { export function isSafeMethod(method: RouteMethod): method is SafeRouteMethod { return method === 'get' || method === 'options'; } + +/** + * Create a valid options object with "sensible" defaults + adding some validation to the options fields + * + * @param method HTTP verb for these options + * @param routeConfig The route config definition + */ +export function validOptions( + method: RouteMethod, + routeConfig: InternalRouteConfig +) { + const shouldNotHavePayload = ['head', 'get'].includes(method); + const { options = {}, validate } = routeConfig; + const shouldValidateBody = (validate && !!getRequestValidation(validate).body) || !!options.body; + + const { output } = options.body || {}; + if (typeof output === 'string' && !validBodyOutput.includes(output)) { + throw new Error( + `[options.body.output: '${output}'] in route ${method.toUpperCase()} ${ + routeConfig.path + } is not valid. Only '${validBodyOutput.join("' or '")}' are valid.` + ); + } + + // @ts-expect-error to eliminate problems with `security` in the options for route factories abstractions + if (options.security) { + throw new Error('`options.security` is not allowed in route config. Use `security` instead.'); + } + + const body = shouldNotHavePayload + ? undefined + : { + // If it's not a GET (requires payload) but no body validation is required (or no body options are specified), + // We assume the route does not care about the body => use the memory-cheapest approach (stream and no parsing) + output: !shouldValidateBody ? ('stream' as const) : undefined, + parse: !shouldValidateBody ? false : undefined, + + // User's settings should overwrite any of the "desired" values + ...options.body, + }; + + return { ...options, body }; +} diff --git a/packages/core/http/core-http-server/index.ts b/packages/core/http/core-http-server/index.ts index 7b79dfe313bd6..d0e9475cb1899 100644 --- a/packages/core/http/core-http-server/index.ts +++ b/packages/core/http/core-http-server/index.ts @@ -124,6 +124,7 @@ export type { InternalRouteSecurity, RouteDeprecationInfo, PostValidationMetadata, + AnyKibanaRequest, } from './src/router'; export { validBodyOutput, diff --git a/packages/core/http/core-http-server/src/router/index.ts b/packages/core/http/core-http-server/src/router/index.ts index 166fcad324953..278ab761cf8d2 100644 --- a/packages/core/http/core-http-server/src/router/index.ts +++ b/packages/core/http/core-http-server/src/router/index.ts @@ -31,6 +31,7 @@ export type { KibanaRouteOptions, RouteSecurityGetter, InternalRouteSecurity, + AnyKibanaRequest, } from './request'; export type { RequestHandlerWrapper, RequestHandler } from './request_handler'; export type { RequestHandlerContextBase } from './request_handler_context'; diff --git a/packages/core/http/core-http-server/src/router/request.ts b/packages/core/http/core-http-server/src/router/request.ts index 6fff29eb42b54..7a84fbc8e8bf6 100644 --- a/packages/core/http/core-http-server/src/router/request.ts +++ b/packages/core/http/core-http-server/src/router/request.ts @@ -214,3 +214,8 @@ export interface KibanaRequest< */ readonly body: Body; } + +/** + * @remark Convenience type, use when the concrete values of P, Q, B and route method do not matter. + */ +export type AnyKibanaRequest = KibanaRequest; From 9b8e30a1e58bd6a75c64607f21d611b11e5c3272 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 12:10:43 +0100 Subject: [PATCH 07/21] some test fixes, mainly for http resources and returning the version header --- .../core-http-router-server-internal/src/route.ts | 12 ++++++------ .../src/router.test.ts | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/route.ts b/packages/core/http/core-http-router-server-internal/src/route.ts index 18e6c1b295e5c..a050df63b04f1 100644 --- a/packages/core/http/core-http-router-server-internal/src/route.ts +++ b/packages/core/http/core-http-router-server-internal/src/route.ts @@ -101,7 +101,7 @@ export const handle = async ( log.error('400 Bad Request', formatErrorMeta(400, { request, error })); const response = kibanaResponseFactory.badRequest({ body: error.message, - headers: isPublicAccessRoute(route) ? getVersionHeader(BASE_PUBLIC_VERSION) : undefined, + headers: isPublicAccessApiRoute(route) ? getVersionHeader(BASE_PUBLIC_VERSION) : undefined, }); // Emit onPostValidation even if validation fails. @@ -109,7 +109,7 @@ export const handle = async ( router.emitPostValidate(req, { deprecated: req.route.options.deprecated, isInternalApiRequest: req.isInternalApiRequest, - isPublicAccess: isPublicAccessRoute(route), + isPublicAccess: isPublicAccessApiRoute(route), }); return response; } @@ -117,22 +117,22 @@ export const handle = async ( router.emitPostValidate(kibanaRequest, { deprecated: kibanaRequest.route.options.deprecated, isInternalApiRequest: kibanaRequest.isInternalApiRequest, - isPublicAccess: isPublicAccessRoute(route), + isPublicAccess: isPublicAccessApiRoute(route), }); const kibanaResponse = await handler(kibanaRequest, kibanaResponseFactory); - if (isPublicAccessRoute(route)) { + if (isPublicAccessApiRoute(route)) { injectVersionHeader(BASE_PUBLIC_VERSION, kibanaResponse); } return kibanaResponse; }; -function isPublicAccessRoute({ +function isPublicAccessApiRoute({ options, }: InternalRouteConfig): boolean { - return options?.access === 'public'; + return !options?.httpResource && options?.access === 'public'; } /** diff --git a/packages/core/http/core-http-router-server-internal/src/router.test.ts b/packages/core/http/core-http-router-server-internal/src/router.test.ts index f0eaa96879d42..cd1616b1e1731 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.test.ts @@ -100,8 +100,7 @@ describe('Router', () => { path: '/versioned', validate: { body: validation, query: validation, params: validation }, }, - (context, req, res) => res.ok(), - { isVersioned: true, events: false } + (context, req, res) => res.ok() ); router.get( { From ebd8add10f27cf3d5a3a8c37b84ae7aac9bccbab Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 12:11:43 +0100 Subject: [PATCH 08/21] delete unused code --- .../src/router.ts | 36 ------------------- 1 file changed, 36 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index 6f3728a0ea856..707be5880cb9b 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -10,7 +10,6 @@ import { EventEmitter } from 'node:events'; import type { Request, ResponseToolkit } from '@hapi/hapi'; import apm from 'elastic-apm-node'; -import { isConfigSchema } from '@kbn/config-schema'; import type { Logger } from '@kbn/logging'; import { isUnauthorizedError as isElasticsearchUnauthorizedError, @@ -30,10 +29,7 @@ import type { PostValidationMetadata, IKibanaResponse, } from '@kbn/core-http-server'; -import { isZod } from '@kbn/zod'; -import { getRequestValidation } from '@kbn/core-http-server'; import type { RouteSecurityGetter } from '@kbn/core-http-server'; -import { RouteValidator } from './validator'; import { CoreVersionedRouter } from './versioned_router'; import { CoreKibanaRequest, getProtocolFromRequest } from './request'; import { kibanaResponseFactory } from './response'; @@ -61,37 +57,6 @@ export type InternalRouterRoute = Omit & { handler: InternalRouteHandler; }; -/** - * Create the validation schemas for a route - * - * @returns Route schemas if `validate` is specified on the route, otherwise - * undefined. - */ -function routeSchemasFromRouteConfig( - route: InternalRouteConfig, - routeMethod: RouteMethod -) { - // The type doesn't allow `validate` to be undefined, but it can still - // happen when it's used from JavaScript. - if (route.validate === undefined) { - throw new Error( - `The [${routeMethod}] at [${route.path}] does not have a 'validate' specified. Use 'false' as the value if you want to bypass validation.` - ); - } - - if (route.validate !== false) { - const validation = getRequestValidation(route.validate); - Object.entries(validation).forEach(([key, schema]) => { - if (!(isConfigSchema(schema) || isZod(schema) || typeof schema === 'function')) { - throw new Error( - `Expected a valid validation logic declared with '@kbn/config-schema' package, '@kbn/zod' package or a RouteValidationFunction at key: [${key}].` - ); - } - }); - return RouteValidator.from(validation); - } -} - /** @internal */ export interface RouterOptions { /** Whether we are running in development */ @@ -196,7 +161,6 @@ export class Router Date: Fri, 3 Jan 2025 13:02:45 +0100 Subject: [PATCH 09/21] wiiip 3 --- .../src/route.ts | 60 ++++++--- .../src/router.test.ts | 11 +- .../src/router.ts | 3 +- .../core_versioned_route.test.ts | 37 +++--- .../versioned_router/core_versioned_route.ts | 122 ++++++++---------- .../core_versioned_router.test.ts | 9 +- .../versioned_router/core_versioned_router.ts | 8 +- .../versioned_router/route_version_utils.ts | 2 +- .../core-http-server/src/router/router.ts | 2 +- 9 files changed, 142 insertions(+), 112 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/route.ts b/packages/core/http/core-http-router-server-internal/src/route.ts index a050df63b04f1..ec6e2ab765e64 100644 --- a/packages/core/http/core-http-router-server-internal/src/route.ts +++ b/packages/core/http/core-http-router-server-internal/src/route.ts @@ -13,7 +13,14 @@ import { type RouteConfig, getRequestValidation, } from '@kbn/core-http-server'; -import type { RouteSecurityGetter, RouteSecurity, AnyKibanaRequest } from '@kbn/core-http-server'; +import type { + RouteSecurityGetter, + RouteSecurity, + AnyKibanaRequest, + IKibanaResponse, + RouteAccess, + RouteConfigOptions, +} from '@kbn/core-http-server'; import { isConfigSchema } from '@kbn/config-schema'; import { isZod } from '@kbn/zod'; import type { Logger } from '@kbn/logging'; @@ -90,18 +97,22 @@ interface HandlerDependencies extends Dependencies { } /** @internal */ -export const handle = async ( +export function validateHapiRequest( request: Request, - { router, route, handler, routeSchemas, log }: HandlerDependencies -) => { + routeInfo: Pick, 'access' | 'httpResource'>, + router: Router, + log: Logger, + routeSchemas?: RouteValidator +): { ok: AnyKibanaRequest; error?: never } | { ok?: never; error: IKibanaResponse } { let kibanaRequest: AnyKibanaRequest; try { kibanaRequest = CoreKibanaRequest.from(request, routeSchemas); } catch (error) { log.error('400 Bad Request', formatErrorMeta(400, { request, error })); + const isPublicApi = isPublicAccessApiRoute(routeInfo); const response = kibanaResponseFactory.badRequest({ body: error.message, - headers: isPublicAccessApiRoute(route) ? getVersionHeader(BASE_PUBLIC_VERSION) : undefined, + headers: isPublicApi ? getVersionHeader(BASE_PUBLIC_VERSION) : undefined, }); // Emit onPostValidation even if validation fails. @@ -109,30 +120,49 @@ export const handle = async ( router.emitPostValidate(req, { deprecated: req.route.options.deprecated, isInternalApiRequest: req.isInternalApiRequest, - isPublicAccess: isPublicAccessApiRoute(route), + isPublicAccess: isPublicApi, }); - return response; + return { error: response }; } router.emitPostValidate(kibanaRequest, { deprecated: kibanaRequest.route.options.deprecated, isInternalApiRequest: kibanaRequest.isInternalApiRequest, - isPublicAccess: isPublicAccessApiRoute(route), + isPublicAccess: isPublicAccessApiRoute(routeInfo), }); + return { ok: kibanaRequest }; +} +/** @internal */ +export const handle = async ( + request: Request, + { router, route, handler, routeSchemas, log }: HandlerDependencies +) => { + const { error, ok: kibanaRequest } = validateHapiRequest( + request, + { access: route.options?.access, httpResource: route.options?.httpResource }, + router, + log, + routeSchemas + ); + if (error) { + return error; + } const kibanaResponse = await handler(kibanaRequest, kibanaResponseFactory); - - if (isPublicAccessApiRoute(route)) { + if (isPublicAccessApiRoute(route.options)) { injectVersionHeader(BASE_PUBLIC_VERSION, kibanaResponse); } - return kibanaResponse; }; -function isPublicAccessApiRoute({ - options, -}: InternalRouteConfig): boolean { - return !options?.httpResource && options?.access === 'public'; +function isPublicAccessApiRoute({ + access, + httpResource, +}: { + access?: RouteAccess; + httpResource?: boolean; +} = {}): boolean { + return !httpResource && access === 'public'; } /** diff --git a/packages/core/http/core-http-router-server-internal/src/router.test.ts b/packages/core/http/core-http-router-server-internal/src/router.test.ts index cd1616b1e1731..ee1b3d234b71f 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.test.ts @@ -95,13 +95,12 @@ describe('Router', () => { it('can exclude versioned routes', () => { const router = new Router('', logger, enhanceWithContext, routerOptions); const validation = schema.object({ foo: schema.string() }); - router.post( - { + router.versioned + .post({ path: '/versioned', - validate: { body: validation, query: validation, params: validation }, - }, - (context, req, res) => res.ok() - ); + access: 'internal', + }) + .addVersion({ version: '999', validate: false }, async (ctx, req, res) => res.ok()); router.get( { path: '/unversioned', diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index 707be5880cb9b..265ef0d825ddc 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -116,7 +116,7 @@ export class Router, + public readonly enhanceWithContext: ContextEnhancer, private readonly options: RouterOptions ) { this.pluginId = options.pluginId; @@ -227,6 +227,7 @@ export class Router { let router: Router; + let versionedRouter: CoreVersionedRouter; let responseFactory: jest.Mocked; let testValidation: ReturnType; const handlerFn: RequestHandler = async (ctx, req, res) => res.ok({ body: { foo: 1 } }); @@ -46,6 +48,10 @@ describe('Versioned route', () => { })), } as any; router = createRouter(); + versionedRouter = CoreVersionedRouter.from({ + router, + log: loggingSystemMock.createLogger(), + }); }); afterEach(() => { @@ -54,7 +60,6 @@ describe('Versioned route', () => { describe('#getRoutes', () => { it('returns the expected metadata', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); versionedRouter .get({ path: '/test/{id}', @@ -93,7 +98,6 @@ describe('Versioned route', () => { }); it('can register multiple handlers', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); versionedRouter .get({ path: '/test/{id}', access: 'internal' }) .addVersion({ version: '1', validate: false }, handlerFn) @@ -108,7 +112,6 @@ describe('Versioned route', () => { }); it('does not allow specifying a handler for the same version more than once', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); expect(() => versionedRouter .get({ path: '/test/{id}', access: 'internal' }) @@ -121,7 +124,6 @@ describe('Versioned route', () => { }); it('only allows versions that are numbers greater than 0 for internal APIs', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); expect(() => versionedRouter .get({ path: '/test/{id}', access: 'internal' }) @@ -145,7 +147,6 @@ describe('Versioned route', () => { }); it('only allows correctly formatted version date strings for public APIs', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); expect(() => versionedRouter .get({ path: '/test/{id}', access: 'public' }) @@ -169,7 +170,6 @@ describe('Versioned route', () => { }); it('passes through the expected values to the IRouter registrar', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); const opts: Parameters[0] = { path: '/test/{id}', access: 'internal', @@ -204,7 +204,7 @@ describe('Versioned route', () => { it('allows public versions other than "2023-10-31"', () => { expect(() => - CoreVersionedRouter.from({ router, isDev: false }) + CoreVersionedRouter.from({ router, log: loggingSystemMock.createLogger(), isDev: false }) .get({ access: 'public', path: '/foo' }) .addVersion({ version: '2023-01-31', validate: false }, (ctx, req, res) => res.ok()) ).not.toThrow(); @@ -218,7 +218,6 @@ describe('Versioned route', () => { testValidation; (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); - const versionedRouter = CoreVersionedRouter.from({ router }); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { version: '1', @@ -259,7 +258,6 @@ describe('Versioned route', () => { fooValidation.response[404].body = lazyResponse404; (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); - const versionedRouter = CoreVersionedRouter.from({ router }); const lazyValidation = jest.fn(() => fooValidation); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { @@ -309,7 +307,7 @@ describe('Versioned route', () => { // NOTE: Temporary test to ensure single public API version is enforced it('only allows "2023-10-31" as public route versions', () => { expect(() => - CoreVersionedRouter.from({ router, isDev: true }) + CoreVersionedRouter.from({ router, isDev: true, log: loggingSystemMock.createLogger() }) .get({ access: 'public', path: '/foo' }) .addVersion({ version: '2023-01-31', validate: false }, (ctx, req, res) => res.ok()) ).toThrow(/Invalid public version/); @@ -321,7 +319,6 @@ describe('Versioned route', () => { testValidation; (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); - const versionedRouter = CoreVersionedRouter.from({ router, isDev: true }); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { version: '1', @@ -352,7 +349,11 @@ describe('Versioned route', () => { let handler: RequestHandler; (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); - const versionedRouter = CoreVersionedRouter.from({ router, isDev: true }); + versionedRouter = CoreVersionedRouter.from({ + router, + isDev: true, + log: loggingSystemMock.createLogger(), + }); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { version: '1', @@ -383,7 +384,11 @@ describe('Versioned route', () => { const custom = jest.fn(() => ({ value: 1 })); fooValidation.response[200].body = { custom } as any; (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); - const versionedRouter = CoreVersionedRouter.from({ router, isDev: true }); + versionedRouter = CoreVersionedRouter.from({ + router, + isDev: true, + log: loggingSystemMock.createLogger(), + }); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { version: '1', @@ -413,9 +418,10 @@ describe('Versioned route', () => { }); it('allows using default resolution for specific internal routes', async () => { - const versionedRouter = CoreVersionedRouter.from({ + versionedRouter = CoreVersionedRouter.from({ router, isDev: true, + log: loggingSystemMock.createLogger(), useVersionResolutionStrategyForInternalPaths: ['/bypass_me/{id?}'], }); @@ -477,7 +483,6 @@ describe('Versioned route', () => { }); it('can register multiple handlers with different security configurations', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); const securityConfig1: RouteSecurity = { authz: { requiredPrivileges: ['foo'], @@ -537,7 +542,6 @@ describe('Versioned route', () => { }); it('falls back to default security configuration if it is not specified for specific version', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); const securityConfigDefault: RouteSecurity = { authz: { requiredPrivileges: ['foo', 'bar', 'baz'], @@ -620,7 +624,6 @@ describe('Versioned route', () => { }); it('validates security configuration', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); const validSecurityConfig: RouteSecurity = { authz: { requiredPrivileges: ['foo'], diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts index fa6a31011b0cd..83bb9b57cba7f 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts @@ -14,9 +14,6 @@ import { } from '@kbn/core-http-common'; import type { RequestHandler, - RequestHandlerContextBase, - KibanaRequest, - KibanaResponseFactory, ApiVersion, VersionedRoute, VersionedRouteConfig, @@ -26,9 +23,10 @@ import type { RouteSecurity, RouteMethod, VersionedRouterRoute, - PostValidationMetadata, + AnyKibanaRequest, } from '@kbn/core-http-server'; -import type { Mutable } from 'utility-types'; +import { Request } from '@hapi/hapi'; +import { Logger } from '@kbn/logging'; import type { HandlerResolutionStrategy, Method, Options } from './types'; import { validate } from './validate'; @@ -37,15 +35,18 @@ import { isValidRouteVersion, hasQueryVersion, readVersion, - removeQueryVersion, } from './route_version_utils'; -import { getVersionHeader, injectVersionHeader } from '../util'; +import { injectVersionHeader } from '../util'; import { validRouteSecurity } from '../security_route_config_validator'; import { resolvers } from './handler_resolvers'; import { prepareVersionedRouteValidation, unwrapVersionedResponseBodyValidation } from './util'; import type { RequestLike } from './route_version_utils'; -import { Router } from '../router'; +import { RequestHandlerEnhanced, Router } from '../router'; +import { kibanaResponseFactory as responseFactory } from '../response'; +import { validateHapiRequest } from '../route'; +import { RouteValidator } from '../validator'; +import { CoreKibanaRequest } from '../request'; interface InternalVersionedRouteConfig extends VersionedRouteConfig { isDev: boolean; @@ -70,23 +71,25 @@ export class CoreVersionedRoute implements VersionedRoute { public readonly handlers = new Map< ApiVersion, { - fn: RequestHandler; + fn: RequestHandlerEnhanced; options: Options; } >(); public static from({ router, + log, method, path, options, }: { router: Router; + log: Logger; method: Method; path: string; options: InternalVersionedRouteConfig; }) { - return new CoreVersionedRoute(router, method, path, options); + return new CoreVersionedRoute(router, log, method, path, options); } public readonly options: VersionedRouteConfig; @@ -99,6 +102,7 @@ export class CoreVersionedRoute implements VersionedRoute { private defaultHandlerResolutionStrategy: HandlerResolutionStrategy; private constructor( private readonly router: Router, + private readonly log: Logger, public readonly method: Method, public readonly path: string, internalOptions: InternalVersionedRouteConfig @@ -117,17 +121,14 @@ export class CoreVersionedRoute implements VersionedRoute { this.enableQueryVersion = options.enableQueryVersion === true; this.defaultSecurityConfig = validRouteSecurity(options.security, options.options); this.options = options; - this.router[this.method]( - { - path: this.path, - validate: passThroughValidation, - // @ts-expect-error upgrade typescript v5.1.6 - options: this.getRouteConfigOptions(), - security: this.getSecurity, - }, - this.requestHandler, - { isVersioned: true, events: false } - ); + this.router.registerRoute({ + path: this.path, + options: this.getRouteConfigOptions(), + security: this.getSecurity, + handler: (request) => this.handle(request), + isVersioned: true, + method: this.method, + }); } private getRouteConfigOptions(): RouteConfigOptions { @@ -167,81 +168,65 @@ export class CoreVersionedRoute implements VersionedRoute { return version; } - private requestHandler = async ( - ctx: RequestHandlerContextBase, - originalReq: KibanaRequest, - res: KibanaResponseFactory - ): Promise => { + private handle = async (request: Request): Promise => { if (this.handlers.size <= 0) { - return res.custom({ + return responseFactory.custom({ statusCode: 500, body: `No handlers registered for [${this.method}] [${this.path}].`, }); } - const req = originalReq as Mutable; - const version = this.getVersion(req); - req.apiVersion = version; + const version = this.getVersion(request); + // request.apiVersion = version; if (!version) { - return res.badRequest({ + return responseFactory.badRequest({ body: `Please specify a version via ${ELASTIC_HTTP_VERSION_HEADER} header. Available versions: ${this.versionsToString()}`, }); } - if (hasQueryVersion(req)) { - if (this.enableQueryVersion) { - // This endpoint has opted-in to query versioning, so we remove the query parameter as it is reserved - removeQueryVersion(req); - } else - return res.badRequest({ - body: `Use of query parameter "${ELASTIC_HTTP_VERSION_QUERY_PARAM}" is not allowed. Please specify the API version using the "${ELASTIC_HTTP_VERSION_HEADER}" header.`, - }); + if (hasQueryVersion(request) && !this.enableQueryVersion) { + // if () { + // // This endpoint has opted-in to query versioning, so we remove the query parameter as it is reserved + // removeQueryVersion(req); + // } else + return responseFactory.badRequest({ + body: `Use of query parameter "${ELASTIC_HTTP_VERSION_QUERY_PARAM}" is not allowed. Please specify the API version using the "${ELASTIC_HTTP_VERSION_HEADER}" header.`, + }); } const invalidVersionMessage = isValidRouteVersion(this.isPublic, version); if (invalidVersionMessage) { - return res.badRequest({ body: invalidVersionMessage }); + return responseFactory.badRequest({ body: invalidVersionMessage }); } const handler = this.handlers.get(version); if (!handler) { - return res.badRequest({ + return responseFactory.badRequest({ body: `No version "${version}" available for [${this.method}] [${ this.path }]. Available versions are: ${this.versionsToString()}`, }); } const validation = extractValidationSchemaFromHandler(handler); - const postValidateMetadata: PostValidationMetadata = { - deprecated: handler.options.options?.deprecated, - isInternalApiRequest: req.isInternalApiRequest, - isPublicAccess: this.isPublic, - }; + let kibanaRequest: AnyKibanaRequest; if ( validation?.request && Boolean(validation.request.body || validation.request.params || validation.request.query) ) { - try { - const { body, params, query } = validate(req, validation.request); - req.body = body; - req.params = params; - req.query = query; - } catch (e) { - // Emit onPostValidation even if validation fails. - - this.router.emitPostValidate(req, postValidateMetadata); - return res.badRequest({ body: e.message, headers: getVersionHeader(version) }); - } + const { error, ok } = validateHapiRequest( + request, + this.getRouteConfigOptions(), + this.router, + this.log, + RouteValidator.from(validation.request) + ); + if (error) return error; + kibanaRequest = ok; } else { - // Preserve behavior of not passing through unvalidated data - req.body = {}; - req.params = {}; - req.query = {}; + kibanaRequest = CoreKibanaRequest.from(request); } - this.router.emitPostValidate(req, postValidateMetadata); - - const response = await handler.fn(ctx, req, res); + const response = await handler.fn(kibanaRequest, responseFactory); if (this.isDev && validation?.response?.[response.status]?.body) { const { [response.status]: responseValidation, unsafe } = validation.response; @@ -254,7 +239,7 @@ export class CoreVersionedRoute implements VersionedRoute { } ); } catch (e) { - return res.custom({ + return responseFactory.custom({ statusCode: 500, body: `Failed output validation: ${e.message}`, }); @@ -292,13 +277,16 @@ export class CoreVersionedRoute implements VersionedRoute { this.validateVersion(options.version); options = prepareVersionedRouteValidation(options); this.handlers.set(options.version, { - fn: handler, + fn: this.router.enhanceWithContext(handler), options, }); return this; } - public getHandlers(): Array<{ fn: RequestHandler; options: Options }> { + public getHandlers(): Array<{ + fn: RequestHandlerEnhanced; + options: Options; + }> { return [...this.handlers.values()]; } diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts index a3ffffc0ef219..933eafca8b9b8 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts @@ -7,18 +7,23 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; import { Router } from '../router'; import { CoreVersionedRouter } from '.'; import { createRouter } from './mocks'; describe('Versioned router', () => { let router: Router; + let versionedRouter: CoreVersionedRouter; beforeEach(() => { router = createRouter(); + versionedRouter = CoreVersionedRouter.from({ + router, + log: loggingSystemMock.createLogger(), + }); }); it('can register multiple routes', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); versionedRouter.get({ path: '/test/{id}', access: 'internal' }); versionedRouter.post({ path: '/test', access: 'internal' }); versionedRouter.delete({ path: '/test', access: 'internal' }); @@ -27,12 +32,10 @@ describe('Versioned router', () => { it('registers pluginId if router has one', () => { const pluginId = Symbol('test'); - const versionedRouter = CoreVersionedRouter.from({ router: createRouter({ pluginId }) }); expect(versionedRouter.pluginId).toBe(pluginId); }); it('provides the expected metadata', () => { - const versionedRouter = CoreVersionedRouter.from({ router }); versionedRouter.get({ path: '/test/{id}', access: 'internal', diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts index ef1f8255420ae..1fdf72625bcef 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts @@ -14,13 +14,16 @@ import type { VersionedRouterRoute, } from '@kbn/core-http-server'; import { omit } from 'lodash'; +import { Logger } from '@kbn/logging'; import { CoreVersionedRoute } from './core_versioned_route'; import type { HandlerResolutionStrategy, Method } from './types'; -import { getRouteFullPath, type Router } from '../router'; +import type { Router } from '../router'; +import { getRouteFullPath } from '../util'; /** @internal */ export interface VersionedRouterArgs { router: Router; + log: Logger; /** * Which route resolution algo to use. * @note default to "oldest", but when running in dev default to "none" @@ -52,12 +55,14 @@ export class CoreVersionedRouter implements VersionedRouter { public pluginId?: symbol; public static from({ router, + log, defaultHandlerResolutionStrategy, isDev, useVersionResolutionStrategyForInternalPaths, }: VersionedRouterArgs) { return new CoreVersionedRouter( router, + log, defaultHandlerResolutionStrategy, isDev, useVersionResolutionStrategyForInternalPaths @@ -65,6 +70,7 @@ export class CoreVersionedRouter implements VersionedRouter { } private constructor( public readonly router: Router, + private readonly log: Logger, public readonly defaultHandlerResolutionStrategy: HandlerResolutionStrategy = 'oldest', public readonly isDev: boolean = false, useVersionResolutionStrategyForInternalPaths: string[] = [] diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts index 2b82dcc12acd2..f2edbf046fcae 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts @@ -60,7 +60,7 @@ export interface RequestLike { } export function hasQueryVersion( - request: Mutable + request: RequestLike ): request is Mutable { return isObject(request.query) && ELASTIC_HTTP_VERSION_QUERY_PARAM in request.query; } diff --git a/packages/core/http/core-http-server/src/router/router.ts b/packages/core/http/core-http-server/src/router/router.ts index f770cfcc45e4b..f6a039a4130a9 100644 --- a/packages/core/http/core-http-server/src/router/router.ts +++ b/packages/core/http/core-http-server/src/router/router.ts @@ -139,7 +139,7 @@ export interface RouterRoute { req: Request, responseToolkit: ResponseToolkit ) => Promise>; - isVersioned: false; + isVersioned: boolean; } /** @public */ From eec58bdfb06b9a7f23bf441edc30e11ca59f72c1 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 15:50:56 +0100 Subject: [PATCH 10/21] unrefactor minor test change --- .../src/route.ts | 42 ++--- .../src/router.ts | 5 +- .../core_versioned_route.test.ts | 149 ++++++++---------- .../core_versioned_route.test.util.ts | 19 +-- .../versioned_router/core_versioned_route.ts | 58 ++++--- .../core_versioned_router.test.ts | 4 +- .../versioned_router/core_versioned_router.ts | 1 + .../src/versioned_router/mocks.ts | 2 + .../route_version_utils.test.ts | 8 +- .../versioned_router/route_version_utils.ts | 3 +- .../core-http-server/src/versioning/types.ts | 2 +- 11 files changed, 137 insertions(+), 156 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/route.ts b/packages/core/http/core-http-router-server-internal/src/route.ts index ec6e2ab765e64..12af1da33c7bd 100644 --- a/packages/core/http/core-http-router-server-internal/src/route.ts +++ b/packages/core/http/core-http-router-server-internal/src/route.ts @@ -26,6 +26,7 @@ import { isZod } from '@kbn/zod'; import type { Logger } from '@kbn/logging'; import type { DeepPartial } from '@kbn/utility-types'; import { Request } from '@hapi/hapi'; +import { Mutable } from 'utility-types'; import type { InternalRouterRoute, RequestHandlerEnhanced, Router } from './router'; import { CoreKibanaRequest } from './request'; import { RouteValidator } from './validator'; @@ -96,32 +97,38 @@ interface HandlerDependencies extends Dependencies { routeSchemas?: RouteValidator; } +interface ValidationContext { + routeInfo: Pick, 'access' | 'httpResource'>; + router: Router; + log: Logger; + routeSchemas?: RouteValidator; + version?: string; +} + /** @internal */ export function validateHapiRequest( request: Request, - routeInfo: Pick, 'access' | 'httpResource'>, - router: Router, - log: Logger, - routeSchemas?: RouteValidator + { routeInfo, router, log, routeSchemas, version }: ValidationContext ): { ok: AnyKibanaRequest; error?: never } | { ok?: never; error: IKibanaResponse } { - let kibanaRequest: AnyKibanaRequest; + let kibanaRequest: Mutable; try { kibanaRequest = CoreKibanaRequest.from(request, routeSchemas); + kibanaRequest.apiVersion = version; } catch (error) { log.error('400 Bad Request', formatErrorMeta(400, { request, error })); const isPublicApi = isPublicAccessApiRoute(routeInfo); + // Emit onPostValidation even if validation fails. + const failedRequest: Mutable = CoreKibanaRequest.from(request); + router.emitPostValidate(failedRequest, { + deprecated: failedRequest.route.options.deprecated, + isInternalApiRequest: failedRequest.isInternalApiRequest, + isPublicAccess: isPublicApi, + }); + const response = kibanaResponseFactory.badRequest({ body: error.message, headers: isPublicApi ? getVersionHeader(BASE_PUBLIC_VERSION) : undefined, }); - - // Emit onPostValidation even if validation fails. - const req = CoreKibanaRequest.from(request); - router.emitPostValidate(req, { - deprecated: req.route.options.deprecated, - isInternalApiRequest: req.isInternalApiRequest, - isPublicAccess: isPublicApi, - }); return { error: response }; } @@ -138,13 +145,12 @@ export const handle = async ( request: Request, { router, route, handler, routeSchemas, log }: HandlerDependencies ) => { - const { error, ok: kibanaRequest } = validateHapiRequest( - request, - { access: route.options?.access, httpResource: route.options?.httpResource }, + const { error, ok: kibanaRequest } = validateHapiRequest(request, { + routeInfo: { access: route.options?.access, httpResource: route.options?.httpResource }, router, log, - routeSchemas - ); + routeSchemas, + }); if (error) { return error; } diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index 265ef0d825ddc..e9a663754699d 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -48,11 +48,10 @@ export type ContextEnhancer< Context extends RequestHandlerContextBase > = (handler: RequestHandler) => RequestHandlerEnhanced; -/** - * @internal - */ +/** @internal */ export type InternalRouteHandler = (request: Request) => Promise; +/** @internal */ export type InternalRouterRoute = Omit & { handler: InternalRouteHandler; }; diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts index 7c6b2576c77c8..2b381329549d2 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts @@ -9,18 +9,14 @@ import type { ApiVersion } from '@kbn/core-http-common'; import type { - KibanaResponseFactory, RequestHandler, - RouteConfig, VersionedRouteValidation, RouteSecurity, } from '@kbn/core-http-server'; -import { Router } from '../router'; +import { InternalRouteHandler, Router } from '../router'; import { createFooValidation } from '../router.test.util'; import { createRouter } from './mocks'; import { CoreVersionedRouter, unwrapVersionedResponseBodyValidation } from '.'; -import { passThroughValidation } from './core_versioned_route'; -import { Method } from './types'; import { createRequest } from './core_versioned_route.test.util'; import { isConfigSchema } from '@kbn/config-schema'; import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common'; @@ -29,24 +25,10 @@ import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; describe('Versioned route', () => { let router: Router; let versionedRouter: CoreVersionedRouter; - let responseFactory: jest.Mocked; let testValidation: ReturnType; const handlerFn: RequestHandler = async (ctx, req, res) => res.ok({ body: { foo: 1 } }); beforeEach(() => { testValidation = createFooValidation(); - responseFactory = { - custom: jest.fn(({ body, statusCode }) => ({ - options: {}, - status: statusCode, - payload: body, - })), - badRequest: jest.fn(({ body }) => ({ status: 400, payload: body, options: {} })), - ok: jest.fn(({ body } = {}) => ({ - options: {}, - status: 200, - payload: body, - })), - } as any; router = createRouter(); versionedRouter = CoreVersionedRouter.from({ router, @@ -108,7 +90,15 @@ describe('Versioned route', () => { const [route] = routes; expect(route.handlers).toHaveLength(3); // We only register one route with the underlying router - expect(router.get).toHaveBeenCalledTimes(1); + expect(router.registerRoute).toHaveBeenCalledTimes(1); + expect(router.registerRoute).toHaveBeenCalledWith({ + isVersioned: true, + handler: expect.any(Function), + security: expect.any(Function), + method: 'get', + options: { access: 'internal' }, + path: '/test/{id}', + }); }); it('does not allow specifying a handler for the same version more than once', () => { @@ -169,7 +159,7 @@ describe('Versioned route', () => { ).not.toThrow(); }); - it('passes through the expected values to the IRouter registrar', () => { + it('passes through all expected values to the router registrar', () => { const opts: Parameters[0] = { path: '/test/{id}', access: 'internal', @@ -186,20 +176,24 @@ describe('Versioned route', () => { }; versionedRouter.post(opts); - expect(router.post).toHaveBeenCalledTimes(1); - const { access, options } = opts; - - const expectedRouteConfig: RouteConfig = { - path: opts.path, - options: { access, ...options }, - validate: passThroughValidation, - }; - expect(router.post).toHaveBeenCalledWith( - expect.objectContaining(expectedRouteConfig), - expect.any(Function), - { isVersioned: true, events: false } - ); + expect(router.registerRoute).toHaveBeenCalledTimes(1); + expect(router.registerRoute).toHaveBeenCalledWith({ + handler: expect.any(Function), + isVersioned: true, + method: 'post', + options: { + access: 'internal', + authRequired: true, + excludeFromOAS: true, + httpResource: true, + tags: ['access:test'], + timeout: { idleSocket: 10_000, payload: 60_000 }, + xsrfRequired: false, + }, + path: '/test/{id}', + security: expect.any(Function), + }); }); it('allows public versions other than "2023-10-31"', () => { @@ -213,11 +207,11 @@ describe('Versioned route', () => { it.each([['static' as const], ['lazy' as const]])( 'runs %s request validations', async (staticOrLazy) => { - let handler: RequestHandler; + let handler: InternalRouteHandler; const { fooValidation, validateBodyFn, validateOutputFn, validateParamsFn, validateQueryFn } = testValidation; - (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); + (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { version: '1', @@ -227,14 +221,12 @@ describe('Versioned route', () => { ); const kibanaResponse = await handler!( - {} as any, createRequest({ version: '1', body: { foo: 1 }, params: { foo: 1 }, query: { foo: 1 }, - }), - responseFactory + }) ); expect(kibanaResponse.status).toBe(200); @@ -246,7 +238,7 @@ describe('Versioned route', () => { ); it('constructs lazily provided validations once (idempotency)', async () => { - let handler: RequestHandler; + let handler: InternalRouteHandler; const { fooValidation } = testValidation; const response200 = fooValidation.response[200].body; @@ -257,7 +249,7 @@ describe('Versioned route', () => { const lazyResponse404 = jest.fn(() => response404()); fooValidation.response[404].body = lazyResponse404; - (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); + (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); const lazyValidation = jest.fn(() => fooValidation); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { @@ -269,14 +261,12 @@ describe('Versioned route', () => { for (let i = 0; i < 10; i++) { const { status } = await handler!( - {} as any, createRequest({ version: '1', body: { foo: 1 }, params: { foo: 1 }, query: { foo: 1 }, - }), - responseFactory + }) ); const [route] = versionedRouter.getRoutes(); const [ @@ -304,21 +294,28 @@ describe('Versioned route', () => { }); describe('when in dev', () => { + beforeEach(() => { + versionedRouter = CoreVersionedRouter.from({ + router, + isDev: true, + log: loggingSystemMock.createLogger(), + }); + }); // NOTE: Temporary test to ensure single public API version is enforced it('only allows "2023-10-31" as public route versions', () => { expect(() => - CoreVersionedRouter.from({ router, isDev: true, log: loggingSystemMock.createLogger() }) + versionedRouter .get({ access: 'public', path: '/foo' }) .addVersion({ version: '2023-01-31', validate: false }, (ctx, req, res) => res.ok()) ).toThrow(/Invalid public version/); }); it('runs response validations', async () => { - let handler: RequestHandler; + let handler: InternalRouteHandler; const { fooValidation, validateBodyFn, validateOutputFn, validateParamsFn, validateQueryFn } = testValidation; - (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); + (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( { version: '1', @@ -328,14 +325,12 @@ describe('Versioned route', () => { ); const kibanaResponse = await handler!( - {} as any, createRequest({ version: '1', body: { foo: 1 }, params: { foo: 1 }, query: { foo: 1 }, - }), - responseFactory + }) ); expect(kibanaResponse.status).toBe(200); @@ -346,9 +341,9 @@ describe('Versioned route', () => { }); it('handles "undefined" response schemas', async () => { - let handler: RequestHandler; + let handler: InternalRouteHandler; - (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); + (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); versionedRouter = CoreVersionedRouter.from({ router, isDev: true, @@ -364,26 +359,24 @@ describe('Versioned route', () => { await expect( handler!( - {} as any, createRequest({ version: '1', body: { foo: 1 }, params: { foo: 1 }, query: { foo: 1 }, - }), - responseFactory + }) ) ).resolves.not.toThrow(); }); it('runs custom response validations', async () => { - let handler: RequestHandler; + let handler: InternalRouteHandler; const { fooValidation, validateBodyFn, validateOutputFn, validateParamsFn, validateQueryFn } = testValidation; const custom = jest.fn(() => ({ value: 1 })); fooValidation.response[200].body = { custom } as any; - (router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn)); + (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); versionedRouter = CoreVersionedRouter.from({ router, isDev: true, @@ -398,14 +391,12 @@ describe('Versioned route', () => { ); const kibanaResponse = await handler!( - {} as any, createRequest({ version: '1', body: { foo: 1 }, params: { foo: 1 }, query: { foo: 1 }, - }), - responseFactory + }) ); expect(kibanaResponse.status).toBe(200); @@ -425,9 +416,9 @@ describe('Versioned route', () => { useVersionResolutionStrategyForInternalPaths: ['/bypass_me/{id?}'], }); - let bypassVersionHandler: RequestHandler; - (router.post as jest.Mock).mockImplementation( - (opts: unknown, fn) => (bypassVersionHandler = fn) + let bypassVersionHandler: InternalRouteHandler; + (router.registerRoute as jest.Mock).mockImplementation( + (opts) => (bypassVersionHandler = opts.handler) ); versionedRouter.post({ path: '/bypass_me/{id?}', access: 'internal' }).addVersion( { @@ -437,8 +428,10 @@ describe('Versioned route', () => { handlerFn ); - let doNotBypassHandler1: RequestHandler; - (router.put as jest.Mock).mockImplementation((opts: unknown, fn) => (doNotBypassHandler1 = fn)); + let doNotBypassHandler1: InternalRouteHandler; + (router.registerRoute as jest.Mock).mockImplementation( + (opts) => (doNotBypassHandler1 = opts.handler) + ); versionedRouter.put({ path: '/do_not_bypass_me/{id}', access: 'internal' }).addVersion( { version: '1', @@ -447,8 +440,10 @@ describe('Versioned route', () => { handlerFn ); - let doNotBypassHandler2: RequestHandler; - (router.get as jest.Mock).mockImplementation((opts: unknown, fn) => (doNotBypassHandler2 = fn)); + let doNotBypassHandler2: InternalRouteHandler; + (router.registerRoute as jest.Mock).mockImplementation( + (opts) => (doNotBypassHandler2 = opts.handler) + ); versionedRouter.get({ path: '/do_not_bypass_me_either', access: 'internal' }).addVersion( { version: '1', @@ -458,22 +453,12 @@ describe('Versioned route', () => { ); const byPassedVersionResponse = await bypassVersionHandler!( - {} as any, - createRequest({ version: undefined }), - responseFactory + createRequest({ version: undefined }) ); - const doNotBypassResponse1 = await doNotBypassHandler1!( - {} as any, - createRequest({ version: undefined }), - responseFactory - ); + const doNotBypassResponse1 = await doNotBypassHandler1!(createRequest({ version: undefined })); - const doNotBypassResponse2 = await doNotBypassHandler2!( - {} as any, - createRequest({ version: undefined }), - responseFactory - ); + const doNotBypassResponse2 = await doNotBypassHandler2!(createRequest({ version: undefined })); expect(byPassedVersionResponse.status).toBe(200); expect(doNotBypassResponse1.status).toBe(400); @@ -538,7 +523,7 @@ describe('Versioned route', () => { expect(route.handlers[0].options.security).toStrictEqual(securityConfig1); expect(route.handlers[1].options.security).toStrictEqual(securityConfig2); expect(route.handlers[2].options.security).toStrictEqual(securityConfig3); - expect(router.get).toHaveBeenCalledTimes(1); + expect(router.registerRoute).toHaveBeenCalledTimes(1); }); it('falls back to default security configuration if it is not specified for specific version', () => { @@ -620,7 +605,7 @@ describe('Versioned route', () => { headers: { [ELASTIC_HTTP_VERSION_HEADER]: '99' }, }) ).toStrictEqual(securityConfigDefault); - expect(router.get).toHaveBeenCalledTimes(1); + expect(router.registerRoute).toHaveBeenCalledTimes(1); }); it('validates security configuration', () => { diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.util.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.util.ts index c7c8d30666990..190bf79e38fa3 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.util.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.util.ts @@ -10,8 +10,6 @@ // eslint-disable-next-line @kbn/imports/no_boundary_crossing import { hapiMocks } from '@kbn/hapi-mocks'; import { ApiVersion, ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common'; -import { CoreKibanaRequest } from '../request'; -import { passThroughValidation } from './core_versioned_route'; export function createRequest( { @@ -23,14 +21,11 @@ export function createRequest( version: '1', } ) { - return CoreKibanaRequest.from( - hapiMocks.createRequest({ - payload: body, - params, - query, - headers: { [ELASTIC_HTTP_VERSION_HEADER]: version }, - app: { requestId: 'fakeId' }, - }), - passThroughValidation - ); + return hapiMocks.createRequest({ + payload: body, + params, + query, + headers: { [ELASTIC_HTTP_VERSION_HEADER]: version }, + app: { requestId: 'fakeId' }, + }); } diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts index 83bb9b57cba7f..c6554b9cdcefd 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts @@ -7,7 +7,6 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { schema } from '@kbn/config-schema'; import { ELASTIC_HTTP_VERSION_HEADER, ELASTIC_HTTP_VERSION_QUERY_PARAM, @@ -27,6 +26,7 @@ import type { } from '@kbn/core-http-server'; import { Request } from '@hapi/hapi'; import { Logger } from '@kbn/logging'; +import { Mutable } from 'utility-types'; import type { HandlerResolutionStrategy, Method, Options } from './types'; import { validate } from './validate'; @@ -35,6 +35,7 @@ import { isValidRouteVersion, hasQueryVersion, readVersion, + removeQueryVersion, } from './route_version_utils'; import { injectVersionHeader } from '../util'; import { validRouteSecurity } from '../security_route_config_validator'; @@ -54,13 +55,6 @@ interface InternalVersionedRouteConfig extends VersionedR defaultHandlerResolutionStrategy: HandlerResolutionStrategy; } -// This validation is a pass-through so that we can apply our version-specific validation later -export const passThroughValidation = { - body: schema.nullable(schema.any()), - params: schema.nullable(schema.any()), - query: schema.nullable(schema.any()), -}; - function extractValidationSchemaFromHandler(handler: VersionedRouterRoute['handlers'][0]) { if (handler.options.validate === false) return undefined; if (typeof handler.options.validate === 'function') return handler.options.validate(); @@ -168,29 +162,27 @@ export class CoreVersionedRoute implements VersionedRoute { return version; } - private handle = async (request: Request): Promise => { + private handle = async (hapiRequest: Request): Promise => { if (this.handlers.size <= 0) { return responseFactory.custom({ statusCode: 500, body: `No handlers registered for [${this.method}] [${this.path}].`, }); } - const version = this.getVersion(request); - // request.apiVersion = version; + const version = this.getVersion(hapiRequest); if (!version) { return responseFactory.badRequest({ body: `Please specify a version via ${ELASTIC_HTTP_VERSION_HEADER} header. Available versions: ${this.versionsToString()}`, }); } - if (hasQueryVersion(request) && !this.enableQueryVersion) { - // if () { - // // This endpoint has opted-in to query versioning, so we remove the query parameter as it is reserved - // removeQueryVersion(req); - // } else - return responseFactory.badRequest({ - body: `Use of query parameter "${ELASTIC_HTTP_VERSION_QUERY_PARAM}" is not allowed. Please specify the API version using the "${ELASTIC_HTTP_VERSION_HEADER}" header.`, - }); + if (hasQueryVersion(hapiRequest)) { + if (!this.enableQueryVersion) { + return responseFactory.badRequest({ + body: `Use of query parameter "${ELASTIC_HTTP_VERSION_QUERY_PARAM}" is not allowed. Please specify the API version using the "${ELASTIC_HTTP_VERSION_HEADER}" header.`, + }); + } + removeQueryVersion(hapiRequest); } const invalidVersionMessage = isValidRouteVersion(this.isPublic, version); @@ -208,22 +200,26 @@ export class CoreVersionedRoute implements VersionedRoute { } const validation = extractValidationSchemaFromHandler(handler); - let kibanaRequest: AnyKibanaRequest; + let kibanaRequest: Mutable = CoreKibanaRequest.from(hapiRequest); + kibanaRequest.apiVersion = version; if ( validation?.request && Boolean(validation.request.body || validation.request.params || validation.request.query) ) { - const { error, ok } = validateHapiRequest( - request, - this.getRouteConfigOptions(), - this.router, - this.log, - RouteValidator.from(validation.request) - ); - if (error) return error; - kibanaRequest = ok; - } else { - kibanaRequest = CoreKibanaRequest.from(request); + const { error, ok: validatedRequest } = validateHapiRequest(hapiRequest, { + routeInfo: { + access: this.options.access, + httpResource: this.options.options?.httpResource, + }, + router: this.router, + log: this.log, + routeSchemas: RouteValidator.from(validation.request), + version, + }); + if (error) { + return injectVersionHeader(version, error); + } + kibanaRequest = validatedRequest; } const response = await handler.fn(kibanaRequest, responseFactory); diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts index 933eafca8b9b8..7b9fbbf938807 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.test.ts @@ -12,11 +12,12 @@ import { Router } from '../router'; import { CoreVersionedRouter } from '.'; import { createRouter } from './mocks'; +const pluginId = Symbol('test'); describe('Versioned router', () => { let router: Router; let versionedRouter: CoreVersionedRouter; beforeEach(() => { - router = createRouter(); + router = createRouter({ pluginId }); versionedRouter = CoreVersionedRouter.from({ router, log: loggingSystemMock.createLogger(), @@ -31,7 +32,6 @@ describe('Versioned router', () => { }); it('registers pluginId if router has one', () => { - const pluginId = Symbol('test'); expect(versionedRouter.pluginId).toBe(pluginId); }); diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts index 1fdf72625bcef..0570ac3cf099c 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts @@ -86,6 +86,7 @@ export class CoreVersionedRouter implements VersionedRouter { (options: VersionedRouteConfig): VersionedRoute => { const route = CoreVersionedRoute.from({ router: this.router, + log: this.log, method: routeMethod, path: options.path, options: { diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/mocks.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/mocks.ts index 36a672ca6a9f7..88e719b5033dc 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/mocks.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/mocks.ts @@ -21,6 +21,8 @@ export function createRouter(opts: CreateMockRouterOptions = {}) { getRoutes: jest.fn(), handleLegacyErrors: jest.fn(), emitPostValidate: jest.fn(), + registerRoute: jest.fn(), + enhanceWithContext: jest.fn((fn) => fn.bind(null, {})), patch: jest.fn(), routerPath: '', versioned: {} as any, diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.test.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.test.ts index da1e63c9ccca3..249867d2417c0 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.test.ts @@ -7,10 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { KibanaRequest } from '@kbn/core-http-server'; import { hapiMocks } from '@kbn/hapi-mocks'; -import { CoreKibanaRequest } from '../request'; -import { passThroughValidation } from './core_versioned_route'; import { isValidRouteVersion, isAllowedPublicVersion, @@ -65,9 +62,8 @@ describe('isValidRouteVersion', () => { }); }); -function getRequest(arg: { headers?: any; query?: any } = {}): KibanaRequest { - const request = hapiMocks.createRequest({ ...arg }); - return CoreKibanaRequest.from(request, passThroughValidation); +function getRequest(arg: { headers?: any; query?: any } = {}) { + return hapiMocks.createRequest({ ...arg }); } describe('readVersion', () => { diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts index f2edbf046fcae..d0871d9f7c93d 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts @@ -16,6 +16,7 @@ import { isObject, get } from 'lodash'; import { KibanaRequest } from '@kbn/core-http-server'; import moment from 'moment'; import type { Mutable } from 'utility-types'; +import { Request } from '@hapi/hapi'; const PUBLIC_VERSION_REGEX = /^[0-9]{4}-[0-9]{2}-[0-9]{2}$/; const INTERNAL_VERSION_REGEX = /^[1-9][0-9]*$/; @@ -64,7 +65,7 @@ export function hasQueryVersion( ): request is Mutable { return isObject(request.query) && ELASTIC_HTTP_VERSION_QUERY_PARAM in request.query; } -export function removeQueryVersion(request: Mutable): void { +export function removeQueryVersion(request: Request): void { delete request.query[ELASTIC_HTTP_VERSION_QUERY_PARAM]; } diff --git a/packages/core/http/core-http-server/src/versioning/types.ts b/packages/core/http/core-http-server/src/versioning/types.ts index fa08810ce1fb7..d94f01fe9f5fb 100644 --- a/packages/core/http/core-http-server/src/versioning/types.ts +++ b/packages/core/http/core-http-server/src/versioning/types.ts @@ -370,6 +370,6 @@ export interface VersionedRouterRoute

{ method: string; path: string; options: Omit, 'path'>; - handlers: Array<{ fn: RequestHandler; options: AddVersionOpts }>; + handlers: Array<{ fn: Function; options: AddVersionOpts }>; isVersioned: true; } From b382675cc2e32bf8dd459e80e32ebb4a014927bb Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 15:57:57 +0100 Subject: [PATCH 11/21] added missing test coverage --- .../src/route.test.ts | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 packages/core/http/core-http-router-server-internal/src/route.test.ts diff --git a/packages/core/http/core-http-router-server-internal/src/route.test.ts b/packages/core/http/core-http-router-server-internal/src/route.test.ts new file mode 100644 index 0000000000000..048c10803dd2d --- /dev/null +++ b/packages/core/http/core-http-router-server-internal/src/route.test.ts @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { hapiMocks } from '@kbn/hapi-mocks'; +import { validateHapiRequest } from './route'; +import { createRouter } from './versioned_router/mocks'; +import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; +import { Logger } from '@kbn/logging'; +import { RouteValidator } from './validator'; +import { schema } from '@kbn/config-schema'; +import { Router } from './router'; +import { RouteAccess } from '@kbn/core-http-server'; + +describe('validateHapiRequest', () => { + let router: Router; + let log: Logger; + beforeEach(() => { + router = createRouter(); + log = loggingSystemMock.createLogger(); + }); + it('validates hapi requests and returns kibana requests: ok case', () => { + const { ok, error } = validateHapiRequest(hapiMocks.createRequest({ payload: { ok: true } }), { + log, + routeInfo: { access: 'public', httpResource: false }, + router, + routeSchemas: RouteValidator.from({ body: schema.object({ ok: schema.literal(true) }) }), + }); + expect(ok?.body).toEqual({ ok: true }); + expect(error).toBeUndefined(); + expect(log.error).not.toHaveBeenCalled(); + }); + it('validates hapi requests and returns kibana requests: error case', () => { + const { ok, error } = validateHapiRequest(hapiMocks.createRequest({ payload: { ok: false } }), { + log, + routeInfo: { access: 'public', httpResource: false }, + router, + routeSchemas: RouteValidator.from({ body: schema.object({ ok: schema.literal(true) }) }), + }); + expect(ok).toBeUndefined(); + expect(error?.status).toEqual(400); + expect(error?.payload).toMatch(/expected value to equal/); + expect(log.error).toHaveBeenCalledTimes(1); + expect(log.error).toHaveBeenCalledWith('400 Bad Request', { + error: { message: '[request body.ok]: expected value to equal [true]' }, + http: { request: { method: undefined, path: undefined }, response: { status_code: 400 } }, + }); + }); + + it('emits post validation events on the router', () => { + const deps = { + log, + routeInfo: { access: 'public' as RouteAccess, httpResource: false }, + router, + routeSchemas: RouteValidator.from({ body: schema.object({ ok: schema.literal(true) }) }), + }; + { + const { ok, error } = validateHapiRequest( + hapiMocks.createRequest({ payload: { ok: false } }), + deps + ); + expect(ok).toBeUndefined(); + expect(error).toBeDefined(); + expect(router.emitPostValidate).toHaveBeenCalledTimes(1); + expect(router.emitPostValidate).toHaveBeenCalledWith(expect.any(Object), { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: true, + }); + } + { + const { ok, error } = validateHapiRequest( + hapiMocks.createRequest({ payload: { ok: true } }), + deps + ); + expect(ok).toBeDefined(); + expect(error).toBeUndefined(); + expect(router.emitPostValidate).toHaveBeenCalledTimes(2); + expect(router.emitPostValidate).toHaveBeenNthCalledWith(2, expect.any(Object), { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: true, + }); + } + }); +}); From cdb39be59b4a5a69cc018474273352ab2fea6902 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 16:01:39 +0100 Subject: [PATCH 12/21] remove unnecessary code --- .../versioned_router/core_versioned_route.ts | 13 ++++------ .../src/versioned_router/validate.ts | 24 ------------------- 2 files changed, 5 insertions(+), 32 deletions(-) delete mode 100644 packages/core/http/core-http-router-server-internal/src/versioned_router/validate.ts diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts index c6554b9cdcefd..13febddf9be92 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts @@ -29,7 +29,6 @@ import { Logger } from '@kbn/logging'; import { Mutable } from 'utility-types'; import type { HandlerResolutionStrategy, Method, Options } from './types'; -import { validate } from './validate'; import { isAllowedPublicVersion, isValidRouteVersion, @@ -227,13 +226,11 @@ export class CoreVersionedRoute implements VersionedRoute { if (this.isDev && validation?.response?.[response.status]?.body) { const { [response.status]: responseValidation, unsafe } = validation.response; try { - validate( - { body: response.payload }, - { - body: unwrapVersionedResponseBodyValidation(responseValidation.body!), - unsafe: { body: unsafe?.body }, - } - ); + const validator = RouteValidator.from({ + body: unwrapVersionedResponseBodyValidation(responseValidation.body!), + unsafe: { body: unsafe?.body }, + }); + validator.getBody(response.payload, 'response body'); } catch (e) { return responseFactory.custom({ statusCode: 500, diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/validate.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/validate.ts deleted file mode 100644 index e1ed81a4ca2ac..0000000000000 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/validate.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import type { RouteValidatorFullConfigRequest } from '@kbn/core-http-server'; -import { RouteValidator } from '../validator'; - -/** Will throw if any of the validation checks fail */ -export function validate( - data: { body?: unknown; params?: unknown; query?: unknown }, - runtimeSchema: RouteValidatorFullConfigRequest -): { body: unknown; params: unknown; query: unknown } { - const validator = RouteValidator.from(runtimeSchema); - return { - params: validator.getParams(data.params, 'request params'), - query: validator.getQuery(data.query, 'request query'), - body: validator.getBody(data.body, 'request body'), - }; -} From 6c55327cfc86b32aa3246e0426d07c1fd19bfbb6 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 16:12:54 +0100 Subject: [PATCH 13/21] minor tweaks --- .../src/versioned_router/route_version_utils.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts index d0871d9f7c93d..26c7fd9bbbcda 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts @@ -65,8 +65,10 @@ export function hasQueryVersion( ): request is Mutable { return isObject(request.query) && ELASTIC_HTTP_VERSION_QUERY_PARAM in request.query; } -export function removeQueryVersion(request: Request): void { - delete request.query[ELASTIC_HTTP_VERSION_QUERY_PARAM]; +export function removeQueryVersion(request: RequestLike): void { + if (request.query) { + delete (request.query as { [key: string]: string })[ELASTIC_HTTP_VERSION_QUERY_PARAM]; + } } function readQueryVersion(request: RequestLike): undefined | ApiVersion { From 43be1ad24053212106938c963cca4441f1333782 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 16:14:45 +0100 Subject: [PATCH 14/21] remove unused import --- .../src/versioned_router/route_version_utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts index 26c7fd9bbbcda..e6151148928d2 100644 --- a/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts +++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/route_version_utils.ts @@ -16,7 +16,6 @@ import { isObject, get } from 'lodash'; import { KibanaRequest } from '@kbn/core-http-server'; import moment from 'moment'; import type { Mutable } from 'utility-types'; -import { Request } from '@hapi/hapi'; const PUBLIC_VERSION_REGEX = /^[0-9]{4}-[0-9]{2}-[0-9]{2}$/; const INTERNAL_VERSION_REGEX = /^[1-9][0-9]*$/; From 97d3375d4257f9670288d3246c8725f083baa4b3 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 16:16:24 +0100 Subject: [PATCH 15/21] remove unused type --- .../core/http/core-http-router-server-internal/index.ts | 2 +- .../http/core-http-router-server-internal/src/router.ts | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/index.ts b/packages/core/http/core-http-router-server-internal/index.ts index 0e048c59144df..f0cd26b1ba90b 100644 --- a/packages/core/http/core-http-router-server-internal/index.ts +++ b/packages/core/http/core-http-router-server-internal/index.ts @@ -16,7 +16,7 @@ export { type HandlerResolutionStrategy, } from './src/versioned_router'; export { Router } from './src/router'; -export type { RouterOptions, InternalRegistrar, InternalRegistrarOptions } from './src/router'; +export type { RouterOptions } from './src/router'; export { isKibanaRequest, isRealRequest, ensureRawRequest, CoreKibanaRequest } from './src/request'; export { isSafeMethod } from './src/route'; export { HapiResponseAdapter } from './src/response_adapter'; diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index e9a663754699d..ce43c0a5b9895 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -35,7 +35,6 @@ import { CoreKibanaRequest, getProtocolFromRequest } from './request'; import { kibanaResponseFactory } from './response'; import { HapiResponseAdapter } from './response_adapter'; import { wrapErrors } from './error_wrapper'; -import { Method } from './versioned_router/types'; import { formatErrorMeta } from './util'; import { stripIllegalHttp2Headers } from './strip_illegal_http2_headers'; import { InternalRouteConfig, buildRoute } from './route'; @@ -81,12 +80,6 @@ export type VersionedRouteConfig = Omit< security?: RouteSecurityGetter; }; -/** @internal */ -export type InternalRegistrar = ( - route: InternalRouteConfig, - handler: RequestHandler -) => ReturnType>; - /** @internal */ type RouterEvents = /** Called after route validation, regardless of success or failure */ From d6f5e4efe6b9fde45f7fdde58704a855e29fe81b Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 16:20:30 +0100 Subject: [PATCH 16/21] added comment --- .../src/router.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index ce43c0a5b9895..0eb506174f729 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -50,7 +50,22 @@ export type ContextEnhancer< /** @internal */ export type InternalRouteHandler = (request: Request) => Promise; -/** @internal */ +/** + * We have at least two implementations of InternalRouterRoutes: + * (1) Router route + * (2) Versioned router route {@link CoreVersionedRoute} + * + * The former registers internal handlers when users call `route.put(...)` while + * the latter registers an internal handler for `router.versioned.put(...)`. + * + * This enables us to expose internal details to each of these types routes so + * that implementation has freedom to change what it needs to in each case, like: + * + * validation: versioned routes only know what validation to run after inspecting + * special version values, whereas "regular" routes only ever have one validation + * that is predetermined to always run. + * @internal + */ export type InternalRouterRoute = Omit & { handler: InternalRouteHandler; }; From 427b61979ee21806268579a0b7736f8cccdfe35c Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 16:21:51 +0100 Subject: [PATCH 17/21] remove unused generics --- .../core/http/core-http-router-server-internal/src/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index 0eb506174f729..fcf70341b0c41 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -188,7 +188,7 @@ export class Router({ + private async handle({ request, responseToolkit, handler, From 0c4643ce7d6a01e9f445df11479c2dc6fc70c227 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 3 Jan 2025 16:52:54 +0100 Subject: [PATCH 18/21] fix types --- .../http/core-http-server-internal/src/http_server.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/core/http/core-http-server-internal/src/http_server.ts b/packages/core/http/core-http-server-internal/src/http_server.ts index 1d80c9c4ab0dc..b278ebca963fa 100644 --- a/packages/core/http/core-http-server-internal/src/http_server.ts +++ b/packages/core/http/core-http-server-internal/src/http_server.ts @@ -37,6 +37,7 @@ import type { IAuthHeadersStorage, RouterDeprecatedApiDetails, RouteMethod, + VersionedRouterRoute, } from '@kbn/core-http-server'; import { performance } from 'perf_hooks'; import { isBoom } from '@hapi/boom'; @@ -410,10 +411,12 @@ export class HttpServer { .map((route) => { const access = route.options.access; if (route.isVersioned === true) { - return [...route.handlers.entries()].map(([_, { options }]) => { - const deprecated = options.options?.deprecated; - return { route, version: `${options.version}`, deprecated, access }; - }); + return [...(route as VersionedRouterRoute).handlers.entries()].map( + ([_, { options }]) => { + const deprecated = options.options?.deprecated; + return { route, version: `${options.version}`, deprecated, access }; + } + ); } return { route, version: undefined, deprecated: route.options.deprecated, access }; From c96fd9b141d56b798409d5f2184ede293b4331e6 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 6 Jan 2025 12:47:37 +0100 Subject: [PATCH 19/21] fix validation behaviour on versioned route and added more test coverage --- .../router-server-internal/src/route.test.ts | 71 +++++++++++++++- .../http/router-server-internal/src/route.ts | 35 ++++---- .../core_versioned_route.test.ts | 83 +++++++++++++++++++ .../core_versioned_route.test.util.ts | 2 +- .../versioned_router/core_versioned_route.ts | 35 +++----- 5 files changed, 186 insertions(+), 40 deletions(-) diff --git a/src/core/packages/http/router-server-internal/src/route.test.ts b/src/core/packages/http/router-server-internal/src/route.test.ts index 048c10803dd2d..193cdc29df4cd 100644 --- a/src/core/packages/http/router-server-internal/src/route.test.ts +++ b/src/core/packages/http/router-server-internal/src/route.test.ts @@ -8,7 +8,7 @@ */ import { hapiMocks } from '@kbn/hapi-mocks'; -import { validateHapiRequest } from './route'; +import { validateHapiRequest, handle } from './route'; import { createRouter } from './versioned_router/mocks'; import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; import { Logger } from '@kbn/logging'; @@ -16,6 +16,75 @@ import { RouteValidator } from './validator'; import { schema } from '@kbn/config-schema'; import { Router } from './router'; import { RouteAccess } from '@kbn/core-http-server'; +import { createRequest } from './versioned_router/core_versioned_route.test.util'; +import { kibanaResponseFactory } from './response'; + +describe('handle', () => { + let handler: jest.Func; + let log: Logger; + let router: Router; + beforeEach(() => { + router = createRouter(); + handler = jest.fn(async () => kibanaResponseFactory.ok()); + log = loggingSystemMock.createLogger(); + }); + describe('post validation events', () => { + it('emits with validation schemas provided', async () => { + const validate = { body: schema.object({ foo: schema.number() }) }; + await handle(createRequest({ body: { foo: 1 } }), { + router, + handler, + log, + method: 'get', + route: { path: '/test', validate }, + routeSchemas: RouteValidator.from(validate), + }); + // Failure + await handle(createRequest({ body: { foo: 'bar' } }), { + router, + handler, + log, + method: 'get', + route: { path: '/test', validate }, + routeSchemas: RouteValidator.from(validate), + }); + + expect(handler).toHaveBeenCalledTimes(1); + expect(router.emitPostValidate).toHaveBeenCalledTimes(2); + + expect(router.emitPostValidate).toHaveBeenNthCalledWith(1, expect.any(Object), { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: false, + }); + expect(router.emitPostValidate).toHaveBeenNthCalledWith(2, expect.any(Object), { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: false, + }); + }); + + it('emits with no validation schemas provided', async () => { + await handle(createRequest({ body: { foo: 1 } }), { + router, + handler, + log, + method: 'get', + route: { path: '/test', validate: false }, + routeSchemas: undefined, + }); + + expect(handler).toHaveBeenCalledTimes(1); + expect(router.emitPostValidate).toHaveBeenCalledTimes(1); + + expect(router.emitPostValidate).toHaveBeenCalledWith(expect.any(Object), { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: false, + }); + }); + }); +}); describe('validateHapiRequest', () => { let router: Router; diff --git a/src/core/packages/http/router-server-internal/src/route.ts b/src/core/packages/http/router-server-internal/src/route.ts index 12af1da33c7bd..ed0bfd164e838 100644 --- a/src/core/packages/http/router-server-internal/src/route.ts +++ b/src/core/packages/http/router-server-internal/src/route.ts @@ -97,8 +97,10 @@ interface HandlerDependencies extends Dependencies { routeSchemas?: RouteValidator; } +type RouteInfo = Pick, 'access' | 'httpResource'>; + interface ValidationContext { - routeInfo: Pick, 'access' | 'httpResource'>; + routeInfo: RouteInfo; router: Router; log: Logger; routeSchemas?: RouteValidator; @@ -115,28 +117,23 @@ export function validateHapiRequest( kibanaRequest = CoreKibanaRequest.from(request, routeSchemas); kibanaRequest.apiVersion = version; } catch (error) { + kibanaRequest = CoreKibanaRequest.from(request); log.error('400 Bad Request', formatErrorMeta(400, { request, error })); - const isPublicApi = isPublicAccessApiRoute(routeInfo); - // Emit onPostValidation even if validation fails. - const failedRequest: Mutable = CoreKibanaRequest.from(request); - router.emitPostValidate(failedRequest, { - deprecated: failedRequest.route.options.deprecated, - isInternalApiRequest: failedRequest.isInternalApiRequest, - isPublicAccess: isPublicApi, - }); const response = kibanaResponseFactory.badRequest({ body: error.message, - headers: isPublicApi ? getVersionHeader(BASE_PUBLIC_VERSION) : undefined, + headers: isPublicAccessApiRoute(routeInfo) + ? getVersionHeader(BASE_PUBLIC_VERSION) + : undefined, }); return { error: response }; + } finally { + router.emitPostValidate( + kibanaRequest!, + getPostValidateEventMetadata(kibanaRequest!, routeInfo) + ); } - router.emitPostValidate(kibanaRequest, { - deprecated: kibanaRequest.route.options.deprecated, - isInternalApiRequest: kibanaRequest.isInternalApiRequest, - isPublicAccess: isPublicAccessApiRoute(routeInfo), - }); return { ok: kibanaRequest }; } @@ -201,3 +198,11 @@ function routeSchemasFromRouteConfig( return RouteValidator.from(validation); } } + +function getPostValidateEventMetadata(request: AnyKibanaRequest, routeInfo: RouteInfo) { + return { + deprecated: request.route.options.deprecated, + isInternalApiRequest: request.isInternalApiRequest, + isPublicAccess: isPublicAccessApiRoute(routeInfo), + }; +} diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts index 2b381329549d2..5aa074f7497ac 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts @@ -657,4 +657,87 @@ describe('Versioned route', () => { - [authz.requiredPrivileges.0.1]: expected value of type [string] but got [Object]" `); }); + + describe('emits post validation events on the router', () => { + let handler: InternalRouteHandler; + + it('for routes with validation', async () => { + const { fooValidation } = testValidation; + (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); + versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( + { + version: '1', + validate: fooValidation, + }, + handlerFn + ); + versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( + { + version: '1', + validate: false, + }, + handlerFn + ); + + await handler!( + createRequest({ + version: '1', + body: { foo: 1 }, + params: { foo: 1 }, + query: { foo: 1 }, + }) + ); + // Failed validation + await handler!(createRequest({ version: '1' })); + + expect(router.emitPostValidate).toHaveBeenCalledTimes(2); + expect(router.emitPostValidate).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ apiVersion: '1' }), + { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: false, + } + ); + expect(router.emitPostValidate).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ apiVersion: '1' }), + { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: false, + } + ); + }); + + it('for routes without validation', async () => { + (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); + versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( + { + version: '1', + validate: false, + }, + handlerFn + ); + versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( + { + version: '1', + validate: false, + }, + handlerFn + ); + + await handler!(createRequest({ version: '1' })); + expect(router.emitPostValidate).toHaveBeenCalledTimes(1); + expect(router.emitPostValidate).toHaveBeenCalledWith( + expect.objectContaining({ apiVersion: '1' }), + { + deprecated: undefined, + isInternalApiRequest: false, + isPublicAccess: false, + } + ); + }); + }); }); diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.util.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.util.ts index 190bf79e38fa3..58f29442e94da 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.util.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.util.ts @@ -17,7 +17,7 @@ export function createRequest( body, params, query, - }: { version: undefined | ApiVersion; body?: object; params?: object; query?: object } = { + }: { version?: undefined | ApiVersion; body?: object; params?: object; query?: object } = { version: '1', } ) { diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts index 13febddf9be92..32a90dbffb3cd 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts @@ -22,11 +22,9 @@ import type { RouteSecurity, RouteMethod, VersionedRouterRoute, - AnyKibanaRequest, } from '@kbn/core-http-server'; import { Request } from '@hapi/hapi'; import { Logger } from '@kbn/logging'; -import { Mutable } from 'utility-types'; import type { HandlerResolutionStrategy, Method, Options } from './types'; import { @@ -46,7 +44,6 @@ import { RequestHandlerEnhanced, Router } from '../router'; import { kibanaResponseFactory as responseFactory } from '../response'; import { validateHapiRequest } from '../route'; import { RouteValidator } from '../validator'; -import { CoreKibanaRequest } from '../request'; interface InternalVersionedRouteConfig extends VersionedRouteConfig { isDev: boolean; @@ -199,26 +196,18 @@ export class CoreVersionedRoute implements VersionedRoute { } const validation = extractValidationSchemaFromHandler(handler); - let kibanaRequest: Mutable = CoreKibanaRequest.from(hapiRequest); - kibanaRequest.apiVersion = version; - if ( - validation?.request && - Boolean(validation.request.body || validation.request.params || validation.request.query) - ) { - const { error, ok: validatedRequest } = validateHapiRequest(hapiRequest, { - routeInfo: { - access: this.options.access, - httpResource: this.options.options?.httpResource, - }, - router: this.router, - log: this.log, - routeSchemas: RouteValidator.from(validation.request), - version, - }); - if (error) { - return injectVersionHeader(version, error); - } - kibanaRequest = validatedRequest; + const { error, ok: kibanaRequest } = validateHapiRequest(hapiRequest, { + routeInfo: { + access: this.options.access, + httpResource: this.options.options?.httpResource, + }, + router: this.router, + log: this.log, + routeSchemas: validation?.request ? RouteValidator.from(validation.request) : undefined, + version, + }); + if (error) { + return injectVersionHeader(version, error); } const response = await handler.fn(kibanaRequest, responseFactory); From 532280b8b8d3b99ed35de4ffaaafc1ec184bba1c Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 6 Jan 2025 16:29:32 +0100 Subject: [PATCH 20/21] fix passing through versioned deprecation meta info --- .../router-server-internal/src/route.test.ts | 36 +++++++++++++-- .../http/router-server-internal/src/route.ts | 12 +++-- .../core_versioned_route.test.ts | 46 ++++++++++++------- .../versioned_router/core_versioned_route.ts | 1 + 4 files changed, 71 insertions(+), 24 deletions(-) diff --git a/src/core/packages/http/router-server-internal/src/route.test.ts b/src/core/packages/http/router-server-internal/src/route.test.ts index 193cdc29df4cd..3e61235347ab4 100644 --- a/src/core/packages/http/router-server-internal/src/route.test.ts +++ b/src/core/packages/http/router-server-internal/src/route.test.ts @@ -45,7 +45,17 @@ describe('handle', () => { handler, log, method: 'get', - route: { path: '/test', validate }, + route: { + path: '/test', + validate, + options: { + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, + }, + }, routeSchemas: RouteValidator.from(validate), }); @@ -58,7 +68,11 @@ describe('handle', () => { isPublicAccess: false, }); expect(router.emitPostValidate).toHaveBeenNthCalledWith(2, expect.any(Object), { - deprecated: undefined, + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, isInternalApiRequest: false, isPublicAccess: false, }); @@ -70,7 +84,17 @@ describe('handle', () => { handler, log, method: 'get', - route: { path: '/test', validate: false }, + route: { + path: '/test', + validate: false, + options: { + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, + }, + }, routeSchemas: undefined, }); @@ -78,7 +102,11 @@ describe('handle', () => { expect(router.emitPostValidate).toHaveBeenCalledTimes(1); expect(router.emitPostValidate).toHaveBeenCalledWith(expect.any(Object), { - deprecated: undefined, + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, isInternalApiRequest: false, isPublicAccess: false, }); diff --git a/src/core/packages/http/router-server-internal/src/route.ts b/src/core/packages/http/router-server-internal/src/route.ts index ed0bfd164e838..54f8bc0206900 100644 --- a/src/core/packages/http/router-server-internal/src/route.ts +++ b/src/core/packages/http/router-server-internal/src/route.ts @@ -97,7 +97,7 @@ interface HandlerDependencies extends Dependencies { routeSchemas?: RouteValidator; } -type RouteInfo = Pick, 'access' | 'httpResource'>; +type RouteInfo = Pick, 'access' | 'httpResource' | 'deprecated'>; interface ValidationContext { routeInfo: RouteInfo; @@ -118,6 +118,8 @@ export function validateHapiRequest( kibanaRequest.apiVersion = version; } catch (error) { kibanaRequest = CoreKibanaRequest.from(request); + kibanaRequest.apiVersion = version; + log.error('400 Bad Request', formatErrorMeta(400, { request, error })); const response = kibanaResponseFactory.badRequest({ @@ -143,7 +145,11 @@ export const handle = async ( { router, route, handler, routeSchemas, log }: HandlerDependencies ) => { const { error, ok: kibanaRequest } = validateHapiRequest(request, { - routeInfo: { access: route.options?.access, httpResource: route.options?.httpResource }, + routeInfo: { + access: route.options?.access, + httpResource: route.options?.httpResource, + deprecated: route.options?.deprecated, + }, router, log, routeSchemas, @@ -201,7 +207,7 @@ function routeSchemasFromRouteConfig( function getPostValidateEventMetadata(request: AnyKibanaRequest, routeInfo: RouteInfo) { return { - deprecated: request.route.options.deprecated, + deprecated: routeInfo.deprecated, isInternalApiRequest: request.isInternalApiRequest, isPublicAccess: isPublicAccessApiRoute(routeInfo), }; diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts index 5aa074f7497ac..a5b883e76e949 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts @@ -668,13 +668,13 @@ describe('Versioned route', () => { { version: '1', validate: fooValidation, - }, - handlerFn - ); - versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( - { - version: '1', - validate: false, + options: { + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, + }, }, handlerFn ); @@ -695,7 +695,11 @@ describe('Versioned route', () => { 1, expect.objectContaining({ apiVersion: '1' }), { - deprecated: undefined, + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, isInternalApiRequest: false, isPublicAccess: false, } @@ -704,7 +708,11 @@ describe('Versioned route', () => { 2, expect.objectContaining({ apiVersion: '1' }), { - deprecated: undefined, + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, isInternalApiRequest: false, isPublicAccess: false, } @@ -717,13 +725,13 @@ describe('Versioned route', () => { { version: '1', validate: false, - }, - handlerFn - ); - versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( - { - version: '1', - validate: false, + options: { + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, + }, }, handlerFn ); @@ -733,7 +741,11 @@ describe('Versioned route', () => { expect(router.emitPostValidate).toHaveBeenCalledWith( expect.objectContaining({ apiVersion: '1' }), { - deprecated: undefined, + deprecated: { + severity: 'warning', + reason: { type: 'bump', newApiVersion: '123' }, + documentationUrl: 'http://test.foo', + }, isInternalApiRequest: false, isPublicAccess: false, } diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts index 32a90dbffb3cd..220f93f333d37 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts @@ -200,6 +200,7 @@ export class CoreVersionedRoute implements VersionedRoute { routeInfo: { access: this.options.access, httpResource: this.options.options?.httpResource, + deprecated: handler.options?.options?.deprecated, }, router: this.router, log: this.log, From 6010378ed124ef5d8755976752b75365041ef52c Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 10 Jan 2025 11:20:13 +0100 Subject: [PATCH 21/21] Address nit --- src/core/packages/http/router-server-internal/src/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/packages/http/router-server-internal/src/util.ts b/src/core/packages/http/router-server-internal/src/util.ts index fa90af60058ec..b4027d9211890 100644 --- a/src/core/packages/http/router-server-internal/src/util.ts +++ b/src/core/packages/http/router-server-internal/src/util.ts @@ -102,8 +102,8 @@ export function formatErrorMeta( error, request, }: { - request: Request; error: Error; + request: Request; } ) { return {