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..c44a1917 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, req: Request) => void; }; export type ValidateResponseOpts = { diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index afaf4064..b24a6234 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) { - this.processQueryParam( - req.query, - schemaProperties.query, - securityQueryParam, - ); + try { + this.processQueryParam( + req.query, + schemaProperties.query, + securityQueryParam, + ); + } catch (error) { + if (this.requestOpts.onError) { + this.requestOpts.onError(error, req); + } else { + throw error; + } + } } const schemaBody = validator?.schemaBody; @@ -218,7 +227,12 @@ export class RequestValidator { message: message, }); error.errors = err.errors; - throw error; + if (this.requestOpts.onError) { + 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 new file mode 100644 index 00000000..8a711fda --- /dev/null +++ b/test/request.validation.on.error.spec.ts @@ -0,0 +1,145 @@ +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'); + +let onErrorArgs: any[] | null; + +async function buildApp({ + allowUnknownQueryParameters, +}: { + allowUnknownQueryParameters: boolean; +}): Promise { + return await createApp( + { + apiSpec: apiSpecPath, + validateResponses: false, + validateRequests: { + allowUnknownQueryParameters, + onError: function (_err, req) { + onErrorArgs = [_err, req]; + if (req.query['limit'] === 'bad_type_throw') { + throw new Error('error in onError handler'); + } + }, + }, + }, + 3005, + (app) => { + app.get(`${app.basePath}/pets`, (req, res) => { + debugger; + 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' }]; + } + 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; + + before(async () => { + // set up express app + app = await buildApp({ allowUnknownQueryParameters: false }); + }); + + afterEach(() => { + onErrorArgs = null; + }); + + after(() => { + 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 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`) + .expect(200) + .then((r: any) => { + const data = [{ id: 'bad_limit', name: 'not an int' }]; + expect(r.body).to.eql(data); + expect(onErrorArgs?.length).to.equal(2); + expect(onErrorArgs![0].message).to.equal( + 'request/query/limit must be integer', + ); + expect(onErrorArgs![1].query).to.eql({ + limit: 'not_an_integer', + }); + })); + + 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?limit=bad_type_throw`) + .expect(500) + .then((r: any) => { + expect(r.body.message).to.equal('error in onError handler'); + expect(onErrorArgs!.length).to.equal(2); + expect(onErrorArgs![0].message).to.equal( + 'request/query/limit must be integer', + ); + expect(onErrorArgs![1].query).to.eql({ + limit: 'bad_type_throw', + }); + })); +}); diff --git a/test/resources/request.validation.yaml b/test/resources/request.validation.yaml new file mode 100644 index 00000000..03ff62f0 --- /dev/null +++ b/test/resources/request.validation.yaml @@ -0,0 +1,288 @@ +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: limit + in: query + schema: + type: integer + 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: string + name: + type: string + + 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