(Revisions) Support setting allErrors for AJV validation

- Allow allErrors to be set on requests and responses independently

src/framework/ajv/options.ts

@@ -17,23 +17,25 @@ export class AjvOptions {
   }
 
   get response(): Options {
-    const { coerceTypes, removeAdditional } = <ValidateResponseOpts>(
+    const { allErrors, coerceTypes, removeAdditional } = <ValidateResponseOpts>(
       this.options.validateResponses
     );
     return {
       ...this.baseOptions(),
+      allErrors,
       useDefaults: false,
       coerceTypes,
       removeAdditional,
     };
   }
 
   get request(): RequestValidatorOptions {
-    const { allowUnknownQueryParameters, coerceTypes, removeAdditional } = <
+    const { allErrors, allowUnknownQueryParameters, coerceTypes, removeAdditional } = <
       ValidateRequestOpts
     >this.options.validateRequests;
     return {
       ...this.baseOptions(),
+      allErrors,
       allowUnknownQueryParameters,
       coerceTypes,
       removeAdditional,
@@ -50,7 +52,6 @@ export class AjvOptions {
       formats,
       validateFormats,
       serDes,
-      allErrors,
       ajvFormats,
     } = this.options;
     const serDesMap = {};
@@ -79,7 +80,6 @@ export class AjvOptions {
       validateFormats,
       formats,
       serDesMap,
-      allErrors,
       ajvFormats,
     };
 

src/framework/types.ts

@@ -47,12 +47,26 @@ export interface Options extends ajv.Options {
 export interface RequestValidatorOptions extends Options, ValidateRequestOpts { }
 
 export type ValidateRequestOpts = {
+  /**
+   * Whether AJV should check all rules and collect all errors or return after the first error.
+   *
+   * This should not be set to `true` in production. See [AJV: Security risks of trusted
+   * schemas](https://ajv.js.org/security.html#security-risks-of-trusted-schemas).
+   */
+  allErrors?: boolean;
   allowUnknownQueryParameters?: boolean;
   coerceTypes?: boolean | 'array';
   removeAdditional?: boolean | 'all' | 'failing';
 };
 
 export type ValidateResponseOpts = {
+  /**
+   * Whether AJV should check all rules and collect all errors or return after the first error.
+   *
+   * This should not be set to `true` in production. See [AJV: Security risks of trusted
+   * schemas](https://ajv.js.org/security.html#security-risks-of-trusted-schemas).
+   */
+  allErrors?: boolean;
   removeAdditional?: boolean | 'all' | 'failing';
   coerceTypes?: boolean | 'array';
   onError?: (err: InternalServerError, json: any, req: Request) => void;
@@ -150,13 +164,6 @@ export interface OpenApiValidatorOpts {
   serDes?: SerDes[];
   formats?: Format[] | Record<string, ajv.Format>;
   ajvFormats?: FormatsPluginOptions;
-  /**
-   * Whether AJV should check all rules and collect all errors or return after the first error.
-   *
-   * This should not be set to `true` in production. See [AJV: Security risks of trusted
-   * schemas](https://ajv.js.org/security.html#security-risks-of-trusted-schemas).
-   */
-  allErrors?: boolean;
   fileUploader?: boolean | multer.Options;
   multerOpts?: multer.Options;
   $refParser?: {

test/699.spec.ts

@@ -182,7 +182,6 @@ describe('699 serialize response components only', () => {
       3005,
       (app) => {
         app.get([`${app.basePath}/users/:id?`], (req, res) => {
-          debugger;
           if (typeof req.params.id !== 'string') {
             throw new Error("Should be not be deserialized to ObjectId object");
           }

test/881.spec.ts

@@ -50,8 +50,10 @@ function createServer() {
     app.use(
       OpenApiValidator.middleware({
         apiSpec,
+        validateRequests: {
+          allErrors: true,
+        },
         validateResponses: true,
-        allErrors: true,
       }),
     );
 

test/additional.props.spec.ts

@@ -15,14 +15,22 @@ describe(packageJson.name, () => {
       'resources',
       'additional.properties.yaml',
     );
-    app = await createApp({ apiSpec, allErrors: true }, 3005, (app) =>
-      app.use(
-        `${app.basePath}/additional_props`,
-        express
-          .Router()
-          .post(`/false`, (req, res) => res.json(req.body))
-          .post(`/true`, (req, res) => res.json(req.body)),
-      ),
+    app = await createApp(
+      {
+        apiSpec,
+        validateRequests: {
+          allErrors: true,
+        },
+      },
+      3005,
+      (app) =>
+        app.use(
+          `${app.basePath}/additional_props`,
+          express
+            .Router()
+            .post(`/false`, (req, res) => res.json(req.body))
+            .post(`/true`, (req, res) => res.json(req.body)),
+        ),
     );
   });
 

test/ajv.options.spec.ts

@@ -121,21 +121,4 @@ describe('AjvOptions', () => {
     expect(options.serDesMap['custom-1']).has.property('serialize');
     expect(options.serDesMap['custom-1']).has.property('deserialize');
   });
-
-  it('should default to not setting allErrors', () => {
-    const ajv = new AjvOptions({
-      ...baseOptions,
-    });
-    const options = ajv.preprocessor;
-    expect(options.allErrors).to.be.undefined;
-  });
-
-  it('should respect user allErrors setting', () => {
-    const ajv = new AjvOptions({
-      ...baseOptions,
-      allErrors: true,
-    });
-    const options = ajv.preprocessor;
-    expect(options.allErrors).to.be.true;
-  });
 });

test/all-errors.spec.ts

@@ -11,6 +11,9 @@ describe('request body validation with and without allErrors', () => {
     app.post(`${app.basePath}/persons`, (req, res) => {
       res.send({ success: true });
     });
+    app.get(`${app.basePath}/persons`, (req, res) => {
+      res.send({ bname: req.query.bname });
+    });
   };
 
   before(async () => {
@@ -19,8 +22,15 @@ describe('request body validation with and without allErrors', () => {
     allErrorsApp = await createApp(
       {
         apiSpec,
-        formats: { 'starts-with-b': (v) => /^b/i.test(v) },
-        allErrors: true,
+        formats: {
+          'starts-with-b': (v) => /^b/i.test(v),
+        },
+        validateRequests: {
+          allErrors: true,
+        },
+        validateResponses: {
+          allErrors: true,
+        },
       },
       3005,
       defineRoutes,
@@ -30,7 +40,10 @@ describe('request body validation with and without allErrors', () => {
     notAllErrorsApp = await createApp(
       {
         apiSpec,
-        formats: { 'starts-with-b': (v) => /^b/i.test(v) },
+        formats: {
+          'starts-with-b': (v) => /^b/i.test(v),
+        },
+        validateResponses: true,
       },
       3006,
       defineRoutes,
@@ -43,30 +56,51 @@ describe('request body validation with and without allErrors', () => {
     notAllErrorsApp.server.close();
   });
 
-  it('should return 200 if short b-name is provided', async () =>
+  it('should return 200 if short b-name is posted', async () =>
     request(allErrorsApp)
       .post(`${allErrorsApp.basePath}/persons`)
       .set('content-type', 'application/json')
       .send({ bname: 'Bob' })
       .expect(200));
 
-  it('should include all validation errors when allErrors=true', async () =>
+  it('should return 200 if short b-name is fetched', async () =>
+    request(allErrorsApp)
+      .get(`${allErrorsApp.basePath}/persons?bname=Bob`)
+      .expect(200));
+
+  it('should include all request validation errors when allErrors=true', async () =>
     request(allErrorsApp)
       .post(`${allErrorsApp.basePath}/persons`)
       .set('content-type', 'application/json')
       .send({ bname: 'Maximillian' })
       .expect(400)
       .then((res) => {
-        expect(res.body.errors.length).to.equal(2);
+        expect(res.body.errors).to.have.lengthOf(2);
       }));
 
-  it('should include only first validation error when allErrors=false', async () =>
+  it('should include only first request validation error when allErrors=false', async () =>
     request(notAllErrorsApp)
       .post(`${notAllErrorsApp.basePath}/persons`)
       .set('content-type', 'application/json')
       .send({ bname: 'Maximillian' })
       .expect(400)
       .then((res) => {
-        expect(res.body.errors.length).to.equal(1);
+        expect(res.body.errors).to.have.lengthOf(1);
+      }));
+
+  it('should include all response validation errors when allErrors=true', async () =>
+    request(allErrorsApp)
+      .get(`${allErrorsApp.basePath}/persons?bname=Maximillian`)
+      .expect(500)
+      .then((res) => {
+        expect(res.body.errors).to.have.lengthOf(2);
+      }));
+
+  it('should include only first response validation error when allErrors=false', async () =>
+    request(notAllErrorsApp)
+      .get(`${notAllErrorsApp.basePath}/persons?bname=Maximillian`)
+      .expect(500)
+      .then((res) => {
+        expect(res.body.errors).to.have.lengthOf(1);
       }));
 });

test/all.of.spec.ts

@@ -11,11 +11,19 @@ describe(packageJson.name, () => {
   before(async () => {
     // Set up the express app
     const apiSpec = path.join('test', 'resources', 'all.of.yaml');
-    app = await createApp({ apiSpec, allErrors: true }, 3005, (app) =>
-      app.use(
-        `${app.basePath}`,
-        express.Router().post(`/all_of`, (req, res) => res.json(req.body)),
-      ),
+    app = await createApp(
+      {
+        apiSpec,
+        validateRequests: {
+          allErrors: true,
+        },
+      },
+      3005,
+      (app) =>
+        app.use(
+          `${app.basePath}`,
+          express.Router().post(`/all_of`, (req, res) => res.json(req.body)),
+        ),
     );
   });
 

test/empty.servers.spec.ts

@@ -10,13 +10,19 @@ describe('empty servers', () => {
   before(async () => {
     // Set up the express app
     const apiSpec = path.join('test', 'resources', 'empty.servers.yaml');
-    app = await createApp({ apiSpec, allErrors: true }, 3007, app =>
-      app.use(
-        ``,
-        express
-          .Router()
-          .get(`/pets`, (req, res) => res.json(req.body)),
-      ),
+    app = await createApp(
+      {
+        apiSpec,
+        validateRequests: {
+          allErrors: true,
+        },
+      },
+      3007,
+      (app) =>
+        app.use(
+          ``,
+          express.Router().get(`/pets`, (req, res) => res.json(req.body)),
+        ),
     );
   });
 
@@ -28,7 +34,7 @@ describe('empty servers', () => {
     request(app)
       .get(`/pets`)
       .expect(400)
-      .then(r => {
+      .then((r) => {
         expect(r.body.errors).to.be.an('array');
         expect(r.body.errors).to.have.length(2);
       }));

test/multi.spec.spec.ts

@@ -50,7 +50,9 @@ function createServer() {
     app.use(
       OpenApiValidator.middleware({
         apiSpec,
-        allErrors: true,
+        validateRequests: {
+          allErrors: true,
+        },
       }),
     );
 

test/multipart.disabled.spec.ts

@@ -9,15 +9,24 @@ describe(packageJson.name, () => {
   let app = null;
   before(async () => {
     const apiSpec = path.join('test', 'resources', 'multipart.yaml');
-    app = await createApp({ apiSpec, allErrors: true, fileUploader: false }, 3003, app =>
-      app.use(
-        `${app.basePath}`,
-        express
-          .Router()
-          .post(`/sample_2`, (req, res) => res.json(req.body))
-          .post(`/sample_1`, (req, res) => res.json(req.body))
-          .get('/range', (req, res) => res.json(req.body)),
-      ),
+    app = await createApp(
+      {
+        apiSpec,
+        fileUploader: false,
+        validateRequests: {
+          allErrors: true,
+        },
+      },
+      3003,
+      (app) =>
+        app.use(
+          `${app.basePath}`,
+          express
+            .Router()
+            .post(`/sample_2`, (req, res) => res.json(req.body))
+            .post(`/sample_1`, (req, res) => res.json(req.body))
+            .get('/range', (req, res) => res.json(req.body)),
+        ),
     );
   });
   after(() => {

test/openapi.spec.ts

@@ -12,8 +12,24 @@ describe(packageJson.name, () => {
     const apiSpecPath = path.join('test', 'resources', 'openapi.yaml');
     const apiSpecJson = require('./resources/openapi.json');
     return Promise.all([
-      createApp({ apiSpec: apiSpecPath, allErrors: true }, 3001),
-      createApp({ apiSpec: apiSpecJson, allErrors: true }, 3002),
+      createApp(
+        {
+          apiSpec: apiSpecPath,
+          validateRequests: {
+            allErrors: true,
+          },
+        },
+        3001,
+      ),
+      createApp(
+        {
+          apiSpec: apiSpecJson,
+          validateRequests: {
+            allErrors: true,
+          },
+        },
+        3002,
+      ),
     ]).then(([a1, a2]) => {
       apps.push(a1);
       apps.push(a2);