From ea71ea75504020deb429b52e0ac6867349713c78 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 11:42:51 +0100 Subject: [PATCH 01/10] kind of working --- src/framework/ajv/options.ts | 20 +- src/framework/types.ts | 1 + test/request.validation.on.error.spec.ts | 104 ++++++++ test/resources/request.validation.yaml | 287 +++++++++++++++++++++++ test/resources/response.validation.yaml | 4 +- 5 files changed, 404 insertions(+), 12 deletions(-) create mode 100644 test/request.validation.on.error.spec.ts create mode 100644 test/resources/request.validation.yaml diff --git a/src/framework/ajv/options.ts b/src/framework/ajv/options.ts index bde6859c..8df5a431 100644 --- a/src/framework/ajv/options.ts +++ b/src/framework/ajv/options.ts @@ -30,14 +30,19 @@ export class AjvOptions { } get request(): RequestValidatorOptions { - const { allErrors, allowUnknownQueryParameters, coerceTypes, removeAdditional } = < - ValidateRequestOpts - >this.options.validateRequests; + const { + allErrors, + allowUnknownQueryParameters, + coerceTypes, + removeAdditional, + onError, + } = this.options.validateRequests; return { ...this.baseOptions(), allErrors, allowUnknownQueryParameters, coerceTypes, + onError, removeAdditional, }; } @@ -47,13 +52,8 @@ export class AjvOptions { } private baseOptions(): Options { - const { - coerceTypes, - formats, - validateFormats, - serDes, - ajvFormats, - } = this.options; + const { coerceTypes, formats, validateFormats, serDes, ajvFormats } = + this.options; const serDesMap = {}; for (const serDesObject of serDes) { if (!serDesMap[serDesObject.format]) { diff --git a/src/framework/types.ts b/src/framework/types.ts index b75adc31..b3d31cb4 100644 --- a/src/framework/types.ts +++ b/src/framework/types.ts @@ -61,6 +61,7 @@ export type ValidateRequestOpts = { allowUnknownQueryParameters?: boolean; coerceTypes?: boolean | 'array'; removeAdditional?: boolean | 'all' | 'failing'; + onError?: (err: InternalServerError, json: any, req: Request) => void; }; export type ValidateResponseOpts = { diff --git a/test/request.validation.on.error.spec.ts b/test/request.validation.on.error.spec.ts new file mode 100644 index 00000000..f174dc92 --- /dev/null +++ b/test/request.validation.on.error.spec.ts @@ -0,0 +1,104 @@ +import * as path from 'path'; +import { expect } from 'chai'; +import * as request from 'supertest'; +import { createApp } from './common/app'; +import * as packageJson from '../package.json'; +import { AppWithServer } from './common/app.common'; + +const apiSpecPath = path.join('test', 'resources', 'request.validation.yaml'); + +describe(packageJson.name, () => { + let app: AppWithServer; + + let onErrorArgs: any[] | null; + before(async () => { + // set up express app + app = await createApp( + { + apiSpec: apiSpecPath, + validateResponses: false, + validateRequests: { + onError: function (_err, body, req) { + console.log(`XXX in onError`); + onErrorArgs = Array.from(arguments); + if (req.query['mode'] === 'bad_type_throw') { + throw new Error('error in onError handler'); + } + console.log(`XXX not throwing`); + }, + }, + }, + 3005, + (app) => { + app.get(`${app.basePath}/users`, (_req, res) => { + const json = ['user1', 'user2', 'user3']; + res.json(json); + }); + app.get(`${app.basePath}/pets`, (req, res) => { + let json = {}; + if (req.query.mode === 'bad_type') { + json = [{ id: 'bad_id', name: 'name', tag: 'tag' }]; + } else if (req.query.mode === 'bad_type_throw') { + json = [{ id: 'bad_id_throw', name: 'name', tag: 'tag' }]; + } + res.json(json); + }); + app.use((err, _req, res, _next) => { + res.status(err.status ?? 500).json({ + message: err.message, + code: err.status ?? 500, + }); + }); + }, + false, + ); + }); + + afterEach(() => { + onErrorArgs = null; + }); + + after(() => { + app.server.close(); + }); + + it('custom error handler invoked if request query field has the wrong type', async () => + request(app) + .get(`${app.basePath}/pets?limit=not_an_integer`) + .expect(200) + .then((r: any) => { + const data = [{ id: 'bad_id', name: 'name', tag: 'tag' }]; + expect(r.body).to.eql(data); + expect(onErrorArgs?.length).to.equal(3); + expect(onErrorArgs![0].message).to.equal( + '/response/0/id must be integer', + ); + expect(onErrorArgs![1]).to.eql(data); + expect(onErrorArgs![2].query).to.eql({ + mode: 'bad_type', + }); + })); + + it('custom error handler not invoked on valid response', async () => + request(app) + .get(`${app.basePath}/pets?limit=3`) + .expect(200) + .then((r: any) => { + expect(r.body).is.an('array').with.length(3); + expect(onErrorArgs).to.equal(null); + })); + + it('returns error if custom error handler throws', async () => + request(app) + .get(`${app.basePath}/pets?mode=bad_type_throw`) + .expect(500) + .then((r: any) => { + const data = [{ id: 'bad_id_throw', name: 'name', tag: 'tag' }]; + expect(r.body.message).to.equal('error in onError handler'); + expect(onErrorArgs!.length).to.equal(3); + expect(onErrorArgs![0].message).to.equal( + '/response/0/id must be integer', + ); + expect(onErrorArgs![1]).to.eql(data); + })); +}); diff --git a/test/resources/request.validation.yaml b/test/resources/request.validation.yaml new file mode 100644 index 00000000..550c4b8e --- /dev/null +++ b/test/resources/request.validation.yaml @@ -0,0 +1,287 @@ +openapi: '3.0.0' +info: + version: 1.0.0 + title: Swagger Petstore + description: A sample API that uses a petstore as an example to demonstrate features in the OpenAPI 3.0 specification + termsOfService: http://swagger.io/terms/ + contact: + name: Swagger API Team + email: apiteam@swagger.io + url: http://swagger.io + license: + name: Apache 2.0 + url: https://www.apache.org/licenses/LICENSE-2.0.html +servers: + - url: /v1 +paths: + /error: + get: + responses: + '400': + description: empty + content: + application/json: + schema: + type: object + required: + - oops + properties: + oops: + type: string + + /ref_response_body: + get: + operationId: refResponseBody + responses: + '200': + $ref: '#/components/responses/ResponseBody' + /empty_response: + description: get empty response + summary: get empty response + get: + description: get empty response + operationId: getEmptyResponse + parameters: + - name: mode + in: query + schema: + type: string + responses: + '204': + description: empty + + /boolean: + description: get boolean responses + get: + operationId: boolean + parameters: + - name: value + in: query + schema: + type: boolean + responses: + '200': + description: boolean + content: + application/json: + schema: + type: boolean + /object: + description: endpoints for pets + summary: endpoints for pets + get: + operationId: object + parameters: + - name: mode + in: query + schema: + type: string + responses: + '200': + description: pet response + content: + application/json: + schema: + $ref: '#/components/schemas/Object' + post: + operationId: postObject + parameters: + - name: mode + in: query + schema: + type: string + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' + responses: + '200': + description: pet response + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' + + /pets: + description: endpoints for pets + summary: endpoints for pets + get: + description: find pets + operationId: findPets + parameters: + - name: mode + in: query + schema: + type: string + responses: + '200': + description: pet response + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/Pet' + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + /humans: + get: + description: find humans + operationId: humans + responses: + '200': + description: humans response + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/Human' + multipart/form-data: + schema: + properties: + stuff: + type: string + default: + description: unexpected error + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + /users: + get: + tags: + - Users + summary: Get users + operationId: getUsers + parameters: [] + responses: + '200': + description: the users + content: + application/json: + schema: + $ref: '#/components/schemas/Users' + + /no_additional_props: + post: + description: Get todos + operationId: getTodos + # security: + # - bearerAuth: [] + responses: + '200': + description: no additional props + content: + application/json: + schema: + $ref: '#/components/schemas/NoAdditionalProps' + +components: + responses: + ResponseBody: + description: Sample response + content: + application/json: + schema: + $ref: '#/components/schemas/Human' + + schemas: + Object: + type: object + required: + - name + - id + properties: + id: + type: integer + format: int64 + bought_at: + type: string + format: date-time + nullable: true + name: + type: string + nullable: true + tag: + type: string + NewPet: + required: + - name + properties: + bought_at: + type: string + format: date-time + nullable: true + name: + type: string + nullable: true + tag: + type: string + + Pet: + allOf: + - $ref: '#/components/schemas/NewPet' + - type: object # need to specify type object or e.g. array will pass validation + required: + - id + properties: + id: + type: integer + format: int64 + + Users: + description: 'Generic list of values from database schema' + type: array + items: + type: string + + Human: + required: + - id + properties: + id: + type: integer + format: int64 + name: + type: string + kids: + type: array + items: + $ref: '#/components/schemas/Human' + + NoAdditionalProps: + type: object + additionalProperties: false + properties: + token_type: + type: string + expires_in: + type: integer + access_token: + type: string + refresh_token: + type: string + user: + additionalProperties: false + type: object + required: + - id + properties: + id: + type: integer + Error: + required: + - code + - message + properties: + code: + type: integer + format: int32 + message: + type: string diff --git a/test/resources/response.validation.yaml b/test/resources/response.validation.yaml index 550c4b8e..4c0deb33 100644 --- a/test/resources/response.validation.yaml +++ b/test/resources/response.validation.yaml @@ -110,10 +110,10 @@ paths: description: find pets operationId: findPets parameters: - - name: mode + - name: limit in: query schema: - type: string + type: integer responses: '200': description: pet response From 794aceb5235a29575aacb5da63a7e826605a4b12 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 11:51:19 +0100 Subject: [PATCH 02/10] test files --- test/request.validation.on.error.spec.ts | 22 +++++++++++----------- test/resources/request.validation.yaml | 4 ++-- test/resources/response.validation.yaml | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/test/request.validation.on.error.spec.ts b/test/request.validation.on.error.spec.ts index f174dc92..a4dbcf40 100644 --- a/test/request.validation.on.error.spec.ts +++ b/test/request.validation.on.error.spec.ts @@ -30,16 +30,16 @@ describe(packageJson.name, () => { }, 3005, (app) => { - app.get(`${app.basePath}/users`, (_req, res) => { - const json = ['user1', 'user2', 'user3']; - res.json(json); - }); app.get(`${app.basePath}/pets`, (req, res) => { - let json = {}; - if (req.query.mode === 'bad_type') { - json = [{ id: 'bad_id', name: 'name', tag: 'tag' }]; - } else if (req.query.mode === 'bad_type_throw') { - json = [{ id: 'bad_id_throw', name: 'name', tag: 'tag' }]; + let json = [ + { id: '1', name: 'fido' }, + { id: '2', name: 'rex' }, + { id: '3', name: 'spot' }, + ]; + if (req.query.limit === 'not_an_integer') { + json = [{ id: 'bad_limit', name: 'not an int' }]; + } else if (req.query.limit === 'bad_type_throw') { + json = [{ id: 'bad_limit_throw', name: 'name' }]; } res.json(json); }); @@ -67,7 +67,7 @@ describe(packageJson.name, () => { .get(`${app.basePath}/pets?limit=not_an_integer`) .expect(200) .then((r: any) => { - const data = [{ id: 'bad_id', name: 'name', tag: 'tag' }]; + const data = [{ id: 'bad_limit', name: 'not an int' }]; expect(r.body).to.eql(data); expect(onErrorArgs?.length).to.equal(3); expect(onErrorArgs![0].message).to.equal( @@ -93,7 +93,7 @@ describe(packageJson.name, () => { .get(`${app.basePath}/pets?mode=bad_type_throw`) .expect(500) .then((r: any) => { - const data = [{ id: 'bad_id_throw', name: 'name', tag: 'tag' }]; + const data = [{ id: 'bad_limit_throw', name: 'name' }]; expect(r.body.message).to.equal('error in onError handler'); expect(onErrorArgs!.length).to.equal(3); expect(onErrorArgs![0].message).to.equal( diff --git a/test/resources/request.validation.yaml b/test/resources/request.validation.yaml index 550c4b8e..4c0deb33 100644 --- a/test/resources/request.validation.yaml +++ b/test/resources/request.validation.yaml @@ -110,10 +110,10 @@ paths: description: find pets operationId: findPets parameters: - - name: mode + - name: limit in: query schema: - type: string + type: integer responses: '200': description: pet response diff --git a/test/resources/response.validation.yaml b/test/resources/response.validation.yaml index 4c0deb33..550c4b8e 100644 --- a/test/resources/response.validation.yaml +++ b/test/resources/response.validation.yaml @@ -110,10 +110,10 @@ paths: description: find pets operationId: findPets parameters: - - name: limit + - name: mode in: query schema: - type: integer + type: string responses: '200': description: pet response From a41ec31935158edef1f09f8993ad531975ab85e7 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 11:54:03 +0100 Subject: [PATCH 03/10] fix types maybe --- src/middlewares/openapi.request.validator.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index afaf4064..f390f6e2 100644 --- a/src/middlewares/openapi.request.validator.ts +++ b/src/middlewares/openapi.request.validator.ts @@ -46,6 +46,7 @@ export class RequestValidator { delete this.apiDoc.components?.examples; this.requestOpts.allowUnknownQueryParameters = options.allowUnknownQueryParameters; + this.requestOpts.onError = options.onError; this.ajv = createRequestAjv( apiDoc, @@ -157,11 +158,19 @@ export class RequestValidator { mutator.modifyRequest(req); if (!allowUnknownQueryParameters) { + try { this.processQueryParam( req.query, schemaProperties.query, securityQueryParam, ); + } catch (error) { + if (this.requestOpts.onError) { + this.requestOpts.onError(error, body, req); + } else { + throw error; + } + } } const schemaBody = validator?.schemaBody; @@ -218,7 +227,11 @@ export class RequestValidator { message: message, }); error.errors = err.errors; - throw error; + if (this.requestOpts.onError) { + this.requestOpts.onError(error, body, req); + } else { + throw error; + } } }; } From f6fd06c13d2688d70fc6c3d818a645e77588836a Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 12:12:25 +0100 Subject: [PATCH 04/10] working but wrong args --- src/middlewares/openapi.request.validator.ts | 66 +++++++++++++------- test/request.validation.on.error.spec.ts | 9 ++- test/resources/request.validation.yaml | 5 +- 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index f390f6e2..7308cc80 100644 --- a/src/middlewares/openapi.request.validator.ts +++ b/src/middlewares/openapi.request.validator.ts @@ -80,7 +80,12 @@ export class RequestValidator { const key = `${req.method}-${path}-${contentTypeKey}`; if (!this.middlewareCache[key]) { - const middleware = this.buildMiddleware(path, reqSchema, contentType, key); + const middleware = this.buildMiddleware( + path, + reqSchema, + contentType, + key, + ); this.middlewareCache[key] = middleware; } return this.middlewareCache[key](req, res, next); @@ -104,17 +109,23 @@ export class RequestValidator { path: string, reqSchema: OperationObject, contentType: ContentType, - ajvCacheKey: string + ajvCacheKey: string, ): RequestHandler { const apiDoc = this.apiDoc; const schemaParser = new ParametersSchemaParser(this.ajv, apiDoc); const parameters = schemaParser.parse(path, reqSchema.parameters); const securityQueryParam = Security.queryParam(apiDoc, reqSchema); const body = new BodySchemaParser().parse(path, reqSchema, contentType); - const validator = new Validator(this.apiDoc, parameters, body, { - general: this.ajv, - body: this.ajvBody, - }, ajvCacheKey); + const validator = new Validator( + this.apiDoc, + parameters, + body, + { + general: this.ajv, + body: this.ajvBody, + }, + ajvCacheKey, + ); const allowUnknownQueryParameters = !!( reqSchema['x-eov-allow-unknown-query-parameters'] ?? @@ -157,21 +168,25 @@ export class RequestValidator { mutator.modifyRequest(req); + debugger; + if (!allowUnknownQueryParameters) { try { - this.processQueryParam( - req.query, - schemaProperties.query, - securityQueryParam, - ); - } catch (error) { + this.processQueryParam( + req.query, + schemaProperties.query, + securityQueryParam, + ); + } catch (error) { if (this.requestOpts.onError) { - this.requestOpts.onError(error, body, req); - } else { - throw error; - } - } + this.requestOpts.onError(error, body, req); + console.log(`XXX invoked errorhandler`); + } else { + throw error; + } + } } + console.log(`XXX done`); const schemaBody = validator?.schemaBody; if (contentType.mediaType === 'multipart/form-data') { @@ -227,11 +242,12 @@ export class RequestValidator { message: message, }); error.errors = err.errors; - if (this.requestOpts.onError) { - this.requestOpts.onError(error, body, req); - } else { + if (this.requestOpts.onError) { + this.requestOpts.onError(error, body, req); + next(); + } else { throw error; - } + } } }; } @@ -319,7 +335,7 @@ class Validator { general: Ajv; body: Ajv; }, - ajvCacheKey: string + ajvCacheKey: string, ) { this.apiDoc = apiDoc; this.schemaGeneral = this._schemaGeneral(parametersSchema); @@ -328,7 +344,11 @@ class Validator { ...(this.schemaGeneral).properties, // query, header, params props body: (this.schemaBody).properties.body, // body props }; - this.validatorGeneral = useAjvCache(ajv.general, this.schemaGeneral, ajvCacheKey); + this.validatorGeneral = useAjvCache( + ajv.general, + this.schemaGeneral, + ajvCacheKey, + ); this.validatorBody = useAjvCache(ajv.body, this.schemaBody, ajvCacheKey); } diff --git a/test/request.validation.on.error.spec.ts b/test/request.validation.on.error.spec.ts index a4dbcf40..768680cf 100644 --- a/test/request.validation.on.error.spec.ts +++ b/test/request.validation.on.error.spec.ts @@ -21,7 +21,7 @@ describe(packageJson.name, () => { onError: function (_err, body, req) { console.log(`XXX in onError`); onErrorArgs = Array.from(arguments); - if (req.query['mode'] === 'bad_type_throw') { + if (req.query['limit'] === 'bad_type_throw') { throw new Error('error in onError handler'); } console.log(`XXX not throwing`); @@ -31,6 +31,8 @@ describe(packageJson.name, () => { 3005, (app) => { app.get(`${app.basePath}/pets`, (req, res) => { + debugger; + console.log(`XXX in route`); let json = [ { id: '1', name: 'fido' }, { id: '2', name: 'rex' }, @@ -41,6 +43,7 @@ describe(packageJson.name, () => { } else if (req.query.limit === 'bad_type_throw') { json = [{ id: 'bad_limit_throw', name: 'name' }]; } + console.log(`XXX returning json`, json); res.json(json); }); app.use((err, _req, res, _next) => { @@ -71,7 +74,7 @@ describe(packageJson.name, () => { expect(r.body).to.eql(data); expect(onErrorArgs?.length).to.equal(3); expect(onErrorArgs![0].message).to.equal( - '/response/0/id must be integer', + 'request/query/limit must be integer', ); expect(onErrorArgs![1]).to.eql(data); expect(onErrorArgs![2].query).to.eql({ @@ -90,7 +93,7 @@ describe(packageJson.name, () => { it('returns error if custom error handler throws', async () => request(app) - .get(`${app.basePath}/pets?mode=bad_type_throw`) + .get(`${app.basePath}/pets?limit=bad_type_throw`) .expect(500) .then((r: any) => { const data = [{ id: 'bad_limit_throw', name: 'name' }]; diff --git a/test/resources/request.validation.yaml b/test/resources/request.validation.yaml index 4c0deb33..03ff62f0 100644 --- a/test/resources/request.validation.yaml +++ b/test/resources/request.validation.yaml @@ -232,8 +232,9 @@ components: - id properties: id: - type: integer - format: int64 + type: string + name: + type: string Users: description: 'Generic list of values from database schema' From 8b9dd78ad920f21edad8055e402f78ed69e36fc4 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 12:29:27 +0100 Subject: [PATCH 05/10] tests are passing --- src/framework/types.ts | 69 ++++++++++++-------- src/middlewares/openapi.request.validator.ts | 4 +- test/request.validation.on.error.spec.ts | 19 +++--- 3 files changed, 54 insertions(+), 38 deletions(-) diff --git a/src/framework/types.ts b/src/framework/types.ts index b3d31cb4..c0cdd49b 100644 --- a/src/framework/types.ts +++ b/src/framework/types.ts @@ -7,7 +7,7 @@ import AjvDraft4 from 'ajv-draft-04'; import Ajv2020 from 'ajv/dist/2020'; export { OpenAPIFrameworkArgs }; -export type AjvInstance = AjvDraft4 | Ajv2020 +export type AjvInstance = AjvDraft4 | Ajv2020; export type BodySchema = | OpenAPIV3.ReferenceObject @@ -48,7 +48,7 @@ export interface Options extends ajv.Options { ajvFormats?: FormatsPluginOptions; } -export interface RequestValidatorOptions extends Options, ValidateRequestOpts { } +export interface RequestValidatorOptions extends Options, ValidateRequestOpts {} export type ValidateRequestOpts = { /** @@ -61,7 +61,7 @@ export type ValidateRequestOpts = { allowUnknownQueryParameters?: boolean; coerceTypes?: boolean | 'array'; removeAdditional?: boolean | 'all' | 'failing'; - onError?: (err: InternalServerError, json: any, req: Request) => void; + onError?: (err: InternalServerError, req: Request) => void; }; export type ValidateResponseOpts = { @@ -126,32 +126,42 @@ export class SerDesSingleton implements SerDes { serialize: param.serialize, }; } -}; +} export type SerDesMap = { - [format: string]: SerDes + [format: string]: SerDes; }; -type Primitive = undefined | null | boolean | string | number | Function +type Primitive = undefined | null | boolean | string | number | Function; -type Immutable = - T extends Primitive ? T : - T extends Array ? ReadonlyArray : - T extends Map ? ReadonlyMap : Readonly +type Immutable = T extends Primitive + ? T + : T extends Array + ? ReadonlyArray + : T extends Map + ? ReadonlyMap + : Readonly; -type DeepImmutable = - T extends Primitive ? T : - T extends Array ? DeepImmutableArray : - T extends Map ? DeepImmutableMap : DeepImmutableObject +type DeepImmutable = T extends Primitive + ? T + : T extends Array + ? DeepImmutableArray + : T extends Map + ? DeepImmutableMap + : DeepImmutableObject; interface DeepImmutableArray extends ReadonlyArray> {} -interface DeepImmutableMap extends ReadonlyMap, DeepImmutable> {} +interface DeepImmutableMap + extends ReadonlyMap, DeepImmutable> {} type DeepImmutableObject = { - readonly [K in keyof T]: DeepImmutable -} + readonly [K in keyof T]: DeepImmutable; +}; export interface OpenApiValidatorOpts { - apiSpec: DeepImmutable | DeepImmutable | string; + apiSpec: + | DeepImmutable + | DeepImmutable + | string; validateApiSpec?: boolean; validateResponses?: boolean | ValidateResponseOpts; validateRequests?: boolean | ValidateRequestOpts; @@ -206,17 +216,18 @@ export namespace OpenAPIV3 { } interface ComponentsV3_1 extends ComponentsObject { - pathItems?: { [path: string]: PathItemObject | ReferenceObject } + pathItems?: { [path: string]: PathItemObject | ReferenceObject }; } - export interface DocumentV3_1 extends Omit { + export interface DocumentV3_1 + extends Omit { openapi: `3.1.${string}`; paths?: DocumentV3['paths']; info: InfoObjectV3_1; components: ComponentsV3_1; webhooks: { - [name: string]: PathItemObject | ReferenceObject - } + [name: string]: PathItemObject | ReferenceObject; + }; } export interface InfoObject { @@ -300,7 +311,7 @@ export namespace OpenAPIV3 { in: string; } - export interface HeaderObject extends ParameterBaseObject { } + export interface HeaderObject extends ParameterBaseObject {} interface ParameterBaseObject { description?: string; @@ -324,14 +335,18 @@ export namespace OpenAPIV3 { | 'integer'; export type ArraySchemaObjectType = 'array'; - export type SchemaObject = ArraySchemaObject | NonArraySchemaObject | CompositionSchemaObject; + export type SchemaObject = + | ArraySchemaObject + | NonArraySchemaObject + | CompositionSchemaObject; - export interface ArraySchemaObject extends BaseSchemaObject { + export interface ArraySchemaObject + extends BaseSchemaObject { items: ReferenceObject | SchemaObject; } - export interface NonArraySchemaObject extends BaseSchemaObject { - } + export interface NonArraySchemaObject + extends BaseSchemaObject {} export interface CompositionSchemaObject extends BaseSchemaObject { // JSON schema allowed properties, adjusted for OpenAPI diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index 7308cc80..c134bbb7 100644 --- a/src/middlewares/openapi.request.validator.ts +++ b/src/middlewares/openapi.request.validator.ts @@ -179,7 +179,7 @@ export class RequestValidator { ); } catch (error) { if (this.requestOpts.onError) { - this.requestOpts.onError(error, body, req); + this.requestOpts.onError(error, req); console.log(`XXX invoked errorhandler`); } else { throw error; @@ -243,7 +243,7 @@ export class RequestValidator { }); error.errors = err.errors; if (this.requestOpts.onError) { - this.requestOpts.onError(error, body, req); + this.requestOpts.onError(error, req); next(); } else { throw error; diff --git a/test/request.validation.on.error.spec.ts b/test/request.validation.on.error.spec.ts index 768680cf..da8eca4f 100644 --- a/test/request.validation.on.error.spec.ts +++ b/test/request.validation.on.error.spec.ts @@ -18,9 +18,9 @@ describe(packageJson.name, () => { apiSpec: apiSpecPath, validateResponses: false, validateRequests: { - onError: function (_err, body, req) { + onError: function (_err, req) { console.log(`XXX in onError`); - onErrorArgs = Array.from(arguments); + onErrorArgs = [_err, req]; if (req.query['limit'] === 'bad_type_throw') { throw new Error('error in onError handler'); } @@ -72,13 +72,12 @@ describe(packageJson.name, () => { .then((r: any) => { const data = [{ id: 'bad_limit', name: 'not an int' }]; expect(r.body).to.eql(data); - expect(onErrorArgs?.length).to.equal(3); + expect(onErrorArgs?.length).to.equal(2); expect(onErrorArgs![0].message).to.equal( 'request/query/limit must be integer', ); - expect(onErrorArgs![1]).to.eql(data); - expect(onErrorArgs![2].query).to.eql({ - mode: 'bad_type', + expect(onErrorArgs![1].query).to.eql({ + limit: 'not_an_integer', }); })); @@ -98,10 +97,12 @@ describe(packageJson.name, () => { .then((r: any) => { const data = [{ id: 'bad_limit_throw', name: 'name' }]; expect(r.body.message).to.equal('error in onError handler'); - expect(onErrorArgs!.length).to.equal(3); + expect(onErrorArgs!.length).to.equal(2); expect(onErrorArgs![0].message).to.equal( - '/response/0/id must be integer', + 'request/query/limit must be integer', ); - expect(onErrorArgs![1]).to.eql(data); + expect(onErrorArgs![1].query).to.eql({ + limit: 'bad_type_throw', + }); })); }); From 4a595809e964b4c3ba87a2d9ead36c143606d729 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 12:38:37 +0100 Subject: [PATCH 06/10] works without allowing --- test/request.validation.on.error.spec.ts | 116 ++++++++++++++--------- 1 file changed, 72 insertions(+), 44 deletions(-) diff --git a/test/request.validation.on.error.spec.ts b/test/request.validation.on.error.spec.ts index da8eca4f..ed51c5f2 100644 --- a/test/request.validation.on.error.spec.ts +++ b/test/request.validation.on.error.spec.ts @@ -7,54 +7,66 @@ import { AppWithServer } from './common/app.common'; const apiSpecPath = path.join('test', 'resources', 'request.validation.yaml'); +let onErrorArgs: any[] | null; + +async function buildApp({ + allowUnknownQueryParameters, +}: { + allowUnknownQueryParameters: boolean; +}): Promise { + return await createApp( + { + apiSpec: apiSpecPath, + validateResponses: false, + validateRequests: { + allowUnknownQueryParameters, + onError: function (_err, req) { + console.log(`XXX in onError`); + onErrorArgs = [_err, req]; + if (req.query['limit'] === 'bad_type_throw') { + throw new Error('error in onError handler'); + } + console.log(`XXX not throwing`); + }, + }, + }, + 3005, + (app) => { + app.get(`${app.basePath}/pets`, (req, res) => { + debugger; + console.log(`XXX in route`); + let json = [ + { id: '1', name: 'fido' }, + { id: '2', name: 'rex' }, + { id: '3', name: 'spot' }, + ]; + if (req.query.limit === 'not_an_integer') { + json = [{ id: 'bad_limit', name: 'not an int' }]; + } else if (req.query.unknown_param === '123') { + json = [{ id: 'unknown_param', name: 'unknown' }]; + } else if (req.query.limit === 'bad_type_throw') { + json = [{ id: 'bad_limit_throw', name: 'name' }]; + } + console.log(`XXX returning json`, json); + res.json(json); + }); + app.use((err, _req, res, _next) => { + res.status(err.status ?? 500).json({ + message: err.message, + code: err.status ?? 500, + }); + }); + }, + false, + ); +} + describe(packageJson.name, () => { let app: AppWithServer; - let onErrorArgs: any[] | null; before(async () => { // set up express app - app = await createApp( - { - apiSpec: apiSpecPath, - validateResponses: false, - validateRequests: { - onError: function (_err, req) { - console.log(`XXX in onError`); - onErrorArgs = [_err, req]; - if (req.query['limit'] === 'bad_type_throw') { - throw new Error('error in onError handler'); - } - console.log(`XXX not throwing`); - }, - }, - }, - 3005, - (app) => { - app.get(`${app.basePath}/pets`, (req, res) => { - debugger; - console.log(`XXX in route`); - let json = [ - { id: '1', name: 'fido' }, - { id: '2', name: 'rex' }, - { id: '3', name: 'spot' }, - ]; - if (req.query.limit === 'not_an_integer') { - json = [{ id: 'bad_limit', name: 'not an int' }]; - } else if (req.query.limit === 'bad_type_throw') { - json = [{ id: 'bad_limit_throw', name: 'name' }]; - } - console.log(`XXX returning json`, json); - res.json(json); - }); - app.use((err, _req, res, _next) => { - res.status(err.status ?? 500).json({ - message: err.message, - code: err.status ?? 500, - }); - }); - }, - false, - ); + app = await buildApp({ allowUnknownQueryParameters: false }); }); afterEach(() => { @@ -65,6 +77,23 @@ describe(packageJson.name, () => { app.server.close(); }); + it('custom error handler invoked if has unknown query parameter', async () => + request(app) + .get(`${app.basePath}/pets?limit=3&unknown_param=123`) + .expect(200) + .then((r: any) => { + const data = [{ id: 'unknown_param', name: 'unknown' }]; + expect(r.body).to.eql(data); + expect(onErrorArgs?.length).to.equal(2); + expect(onErrorArgs![0].message).to.equal( + "Unknown query parameter 'unknown_param'", + ); + expect(onErrorArgs![1].query).to.eql({ + limit: 3, + unknown_param: '123', + }); + })); + it('custom error handler invoked if request query field has the wrong type', async () => request(app) .get(`${app.basePath}/pets?limit=not_an_integer`) @@ -95,7 +124,6 @@ describe(packageJson.name, () => { .get(`${app.basePath}/pets?limit=bad_type_throw`) .expect(500) .then((r: any) => { - const data = [{ id: 'bad_limit_throw', name: 'name' }]; expect(r.body.message).to.equal('error in onError handler'); expect(onErrorArgs!.length).to.equal(2); expect(onErrorArgs![0].message).to.equal( From ee9ff2b636c5a43476befbdd272cd3687821791d Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 12:40:29 +0100 Subject: [PATCH 07/10] all tests passing --- test/request.validation.on.error.spec.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/request.validation.on.error.spec.ts b/test/request.validation.on.error.spec.ts index ed51c5f2..77786679 100644 --- a/test/request.validation.on.error.spec.ts +++ b/test/request.validation.on.error.spec.ts @@ -94,6 +94,19 @@ describe(packageJson.name, () => { }); })); + it('custom error handler not invoked if has unknown query parameter, but is allowed', async () => { + app.server.close(); + app = await buildApp({ allowUnknownQueryParameters: true }); + + request(app) + .get(`${app.basePath}/pets?limit=3&unknown_param=123`) + .expect(200) + .then((r: any) => { + expect(r.body).is.an('array').with.length(3); + expect(onErrorArgs).to.equal(null); + }); + }); + it('custom error handler invoked if request query field has the wrong type', async () => request(app) .get(`${app.basePath}/pets?limit=not_an_integer`) From f8a41428188608502eae0e9b05b0c3b29741e0c9 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 12:41:02 +0100 Subject: [PATCH 08/10] runs clean --- src/middlewares/openapi.request.validator.ts | 2 -- test/request.validation.on.error.spec.ts | 4 ---- 2 files changed, 6 deletions(-) diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index c134bbb7..6f9ed1b4 100644 --- a/src/middlewares/openapi.request.validator.ts +++ b/src/middlewares/openapi.request.validator.ts @@ -180,13 +180,11 @@ export class RequestValidator { } catch (error) { if (this.requestOpts.onError) { this.requestOpts.onError(error, req); - console.log(`XXX invoked errorhandler`); } else { throw error; } } } - console.log(`XXX done`); const schemaBody = validator?.schemaBody; if (contentType.mediaType === 'multipart/form-data') { diff --git a/test/request.validation.on.error.spec.ts b/test/request.validation.on.error.spec.ts index 77786679..8a711fda 100644 --- a/test/request.validation.on.error.spec.ts +++ b/test/request.validation.on.error.spec.ts @@ -21,12 +21,10 @@ async function buildApp({ validateRequests: { allowUnknownQueryParameters, onError: function (_err, req) { - console.log(`XXX in onError`); onErrorArgs = [_err, req]; if (req.query['limit'] === 'bad_type_throw') { throw new Error('error in onError handler'); } - console.log(`XXX not throwing`); }, }, }, @@ -34,7 +32,6 @@ async function buildApp({ (app) => { app.get(`${app.basePath}/pets`, (req, res) => { debugger; - console.log(`XXX in route`); let json = [ { id: '1', name: 'fido' }, { id: '2', name: 'rex' }, @@ -47,7 +44,6 @@ async function buildApp({ } else if (req.query.limit === 'bad_type_throw') { json = [{ id: 'bad_limit_throw', name: 'name' }]; } - console.log(`XXX returning json`, json); res.json(json); }); app.use((err, _req, res, _next) => { From 192ac5eb4f8ac10b90a6ecfe2ada6c835780e452 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 12:43:16 +0100 Subject: [PATCH 09/10] maybe --- src/framework/types.ts | 67 ++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/src/framework/types.ts b/src/framework/types.ts index c0cdd49b..c44a1917 100644 --- a/src/framework/types.ts +++ b/src/framework/types.ts @@ -7,7 +7,7 @@ import AjvDraft4 from 'ajv-draft-04'; import Ajv2020 from 'ajv/dist/2020'; export { OpenAPIFrameworkArgs }; -export type AjvInstance = AjvDraft4 | Ajv2020; +export type AjvInstance = AjvDraft4 | Ajv2020 export type BodySchema = | OpenAPIV3.ReferenceObject @@ -48,7 +48,7 @@ export interface Options extends ajv.Options { ajvFormats?: FormatsPluginOptions; } -export interface RequestValidatorOptions extends Options, ValidateRequestOpts {} +export interface RequestValidatorOptions extends Options, ValidateRequestOpts { } export type ValidateRequestOpts = { /** @@ -126,42 +126,32 @@ export class SerDesSingleton implements SerDes { serialize: param.serialize, }; } -} +}; export type SerDesMap = { - [format: string]: SerDes; + [format: string]: SerDes }; -type Primitive = undefined | null | boolean | string | number | Function; +type Primitive = undefined | null | boolean | string | number | Function -type Immutable = T extends Primitive - ? T - : T extends Array - ? ReadonlyArray - : T extends Map - ? ReadonlyMap - : Readonly; +type Immutable = + T extends Primitive ? T : + T extends Array ? ReadonlyArray : + T extends Map ? ReadonlyMap : Readonly -type DeepImmutable = T extends Primitive - ? T - : T extends Array - ? DeepImmutableArray - : T extends Map - ? DeepImmutableMap - : DeepImmutableObject; +type DeepImmutable = + T extends Primitive ? T : + T extends Array ? DeepImmutableArray : + T extends Map ? DeepImmutableMap : DeepImmutableObject interface DeepImmutableArray extends ReadonlyArray> {} -interface DeepImmutableMap - extends ReadonlyMap, DeepImmutable> {} +interface DeepImmutableMap extends ReadonlyMap, DeepImmutable> {} type DeepImmutableObject = { - readonly [K in keyof T]: DeepImmutable; -}; + readonly [K in keyof T]: DeepImmutable +} export interface OpenApiValidatorOpts { - apiSpec: - | DeepImmutable - | DeepImmutable - | string; + apiSpec: DeepImmutable | DeepImmutable | string; validateApiSpec?: boolean; validateResponses?: boolean | ValidateResponseOpts; validateRequests?: boolean | ValidateRequestOpts; @@ -216,18 +206,17 @@ export namespace OpenAPIV3 { } interface ComponentsV3_1 extends ComponentsObject { - pathItems?: { [path: string]: PathItemObject | ReferenceObject }; + pathItems?: { [path: string]: PathItemObject | ReferenceObject } } - export interface DocumentV3_1 - extends Omit { + export interface DocumentV3_1 extends Omit { openapi: `3.1.${string}`; paths?: DocumentV3['paths']; info: InfoObjectV3_1; components: ComponentsV3_1; webhooks: { - [name: string]: PathItemObject | ReferenceObject; - }; + [name: string]: PathItemObject | ReferenceObject + } } export interface InfoObject { @@ -311,7 +300,7 @@ export namespace OpenAPIV3 { in: string; } - export interface HeaderObject extends ParameterBaseObject {} + export interface HeaderObject extends ParameterBaseObject { } interface ParameterBaseObject { description?: string; @@ -335,18 +324,14 @@ export namespace OpenAPIV3 { | 'integer'; export type ArraySchemaObjectType = 'array'; - export type SchemaObject = - | ArraySchemaObject - | NonArraySchemaObject - | CompositionSchemaObject; + export type SchemaObject = ArraySchemaObject | NonArraySchemaObject | CompositionSchemaObject; - export interface ArraySchemaObject - extends BaseSchemaObject { + export interface ArraySchemaObject extends BaseSchemaObject { items: ReferenceObject | SchemaObject; } - export interface NonArraySchemaObject - extends BaseSchemaObject {} + export interface NonArraySchemaObject extends BaseSchemaObject { + } export interface CompositionSchemaObject extends BaseSchemaObject { // JSON schema allowed properties, adjusted for OpenAPI From 6435e7c9fe8574d90762ffb72ac0457c49211480 Mon Sep 17 00:00:00 2001 From: Sam Sudar Date: Wed, 23 Jul 2025 12:46:20 +0100 Subject: [PATCH 10/10] reformat --- src/middlewares/openapi.request.validator.ts | 33 +++++--------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index 6f9ed1b4..b24a6234 100644 --- a/src/middlewares/openapi.request.validator.ts +++ b/src/middlewares/openapi.request.validator.ts @@ -80,12 +80,7 @@ export class RequestValidator { const key = `${req.method}-${path}-${contentTypeKey}`; if (!this.middlewareCache[key]) { - const middleware = this.buildMiddleware( - path, - reqSchema, - contentType, - key, - ); + const middleware = this.buildMiddleware(path, reqSchema, contentType, key); this.middlewareCache[key] = middleware; } return this.middlewareCache[key](req, res, next); @@ -109,23 +104,17 @@ export class RequestValidator { path: string, reqSchema: OperationObject, contentType: ContentType, - ajvCacheKey: string, + ajvCacheKey: string ): RequestHandler { const apiDoc = this.apiDoc; const schemaParser = new ParametersSchemaParser(this.ajv, apiDoc); const parameters = schemaParser.parse(path, reqSchema.parameters); const securityQueryParam = Security.queryParam(apiDoc, reqSchema); const body = new BodySchemaParser().parse(path, reqSchema, contentType); - const validator = new Validator( - this.apiDoc, - parameters, - body, - { - general: this.ajv, - body: this.ajvBody, - }, - ajvCacheKey, - ); + const validator = new Validator(this.apiDoc, parameters, body, { + general: this.ajv, + body: this.ajvBody, + }, ajvCacheKey); const allowUnknownQueryParameters = !!( reqSchema['x-eov-allow-unknown-query-parameters'] ?? @@ -168,8 +157,6 @@ export class RequestValidator { mutator.modifyRequest(req); - debugger; - if (!allowUnknownQueryParameters) { try { this.processQueryParam( @@ -333,7 +320,7 @@ class Validator { general: Ajv; body: Ajv; }, - ajvCacheKey: string, + ajvCacheKey: string ) { this.apiDoc = apiDoc; this.schemaGeneral = this._schemaGeneral(parametersSchema); @@ -342,11 +329,7 @@ class Validator { ...(this.schemaGeneral).properties, // query, header, params props body: (this.schemaBody).properties.body, // body props }; - this.validatorGeneral = useAjvCache( - ajv.general, - this.schemaGeneral, - ajvCacheKey, - ); + this.validatorGeneral = useAjvCache(ajv.general, this.schemaGeneral, ajvCacheKey); this.validatorBody = useAjvCache(ajv.body, this.schemaBody, ajvCacheKey); }