fix: fix object type validations (#242)

* Fixed object type validation for headers and properties with type=array
* Fixed multiple object type checks to avoid nulls and arrays
* Changes usage of lodash isObject to isPlainObject, as it checks for actual objects instead of objects, instances, null and others

Docs reference:
- isObject: https://lodash.com/docs/4.17.15#isObject
- isPlainObject: https://lodash.com/docs/4.17.15#isPlainObject

src/cli-validator/runValidator.js

@@ -3,6 +3,7 @@ const util = require('util');
 const fs = require('fs');
 const path = require('path');
 const readYaml = require('js-yaml');
+const isPlainObject = require('lodash/isPlainObject');
 const last = require('lodash/last');
 const chalk = require('chalk');
 const jsonValidator = require('json-dup-key-validator');
@@ -201,7 +202,7 @@ const processInput = async function(program) {
         input = readYaml.safeLoad(originalFile);
       }
 
-      if (typeof input !== 'object') {
+      if (!isPlainObject(input)) {
         throw `The given input in ${validFile} is not a valid object.`;
       }
 

src/cli-validator/utils/circular-references-ibm.js

@@ -1,5 +1,8 @@
 // find the circular references,
 // correct them,
+
+const isPlainObject = require('lodash/isPlainObject');
+
 // and return them as problems if applicable
 const validate = function({ jsSpec, resolvedSpec }, config) {
   const result = { error: [], warning: [] };
@@ -26,7 +29,7 @@ const correctSpec = function(resolvedSpec) {
       return null;
     }
 
-    if (typeof object !== 'object') {
+    if (!isPlainObject(object)) {
       return null;
     }
 
@@ -37,7 +40,7 @@ const correctSpec = function(resolvedSpec) {
     }
 
     return keys.forEach(function(key) {
-      if (typeof object[key] === 'object' && !Array.isArray(object[key])) {
+      if (isPlainObject(object[key])) {
         if (visitedObjects.includes(object[key])) {
           paths.push([...path, key]);
           object[key] = '[Circular]';

src/plugins/ast/ast.js

@@ -1,5 +1,6 @@
 const YAML = require('yaml-js');
 const isArray = require('lodash/isArray');
+const isPlainObject = require('lodash/isPlainObject');
 const lodashFind = require('lodash/find');
 const memoize = require('lodash/memoize');
 
@@ -163,7 +164,7 @@ const pathForPosition = function(yaml, position) {
     throw new TypeError('yaml should be a string');
   }
   if (
-    typeof position !== 'object' ||
+    !isPlainObject(position) ||
     typeof position.line !== 'number' ||
     typeof position.column !== 'number'
   ) {

src/plugins/validation/2and3/semantic-validators/info.js

@@ -4,13 +4,14 @@
 // Assertation 2:
 // making sure that the required version and title are defined properly
 
+const isPlainObject = require('lodash/isPlainObject');
 const MessageCarrier = require('../../../utils/messageCarrier');
 
 module.exports.validate = function({ jsSpec }) {
   const messages = new MessageCarrier();
 
   const info = jsSpec.info;
-  const hasInfo = info && typeof info === 'object';
+  const hasInfo = info && isPlainObject(info);
   if (!hasInfo) {
     messages.addMessage(
       ['info'],

src/plugins/validation/2and3/semantic-validators/items-required-for-array-objects.js

@@ -11,6 +11,7 @@
 const { walk } = require('../../../utils');
 const MessageCarrier = require('../../../utils/messageCarrier');
 const at = require('lodash/at');
+const isPlainObject = require('lodash/isPlainObject');
 
 const reduceObj = function(jsSpec, obj) {
   if (obj['$ref']) {
@@ -62,7 +63,7 @@ module.exports.validate = function({ jsSpec }, config) {
       // if parent is 'schema', or we're in a model definition
 
       // Assertation 1
-      if (obj.type === 'array' && typeof obj.items !== 'object') {
+      if (obj.type === 'array' && !isPlainObject(obj.items)) {
         messages.addMessage(
           path.join('.'),
           "Schema objects with 'array' type require an 'items' property",
@@ -89,7 +90,7 @@ module.exports.validate = function({ jsSpec }, config) {
 
     // this only applies to Swagger 2
     if (path[path.length - 2] === 'headers') {
-      if (obj.type === 'array' && typeof obj.items !== 'object') {
+      if (obj.type === 'array' && !isPlainObject(obj.items)) {
         messages.addMessage(
           path,
           "Headers with 'array' type require an 'items' property",

src/plugins/validation/2and3/semantic-validators/paths.js

@@ -18,7 +18,7 @@
 
 const each = require('lodash/each');
 const findIndex = require('lodash/findIndex');
-const isObject = require('lodash/isObject');
+const isPlainObject = require('lodash/isPlainObject');
 const MessageCarrier = require('../../../utils/messageCarrier');
 
 const templateRegex = /\{(.*?)\}/g;
@@ -68,7 +68,7 @@ module.exports.validate = function({ resolvedSpec }) {
     const parametersFromPath = path.parameters ? path.parameters.slice() : [];
 
     const availableParameters = parametersFromPath.map((param, i) => {
-      if (!isObject(param)) {
+      if (!isPlainObject(param)) {
         return;
       }
       param.$$path = `paths.${pathName}.parameters[${i}]`;
@@ -83,7 +83,7 @@ module.exports.validate = function({ resolvedSpec }) {
       ) {
         availableParameters.push(
           ...operation.parameters.map((param, i) => {
-            if (!isObject(param)) {
+            if (!isPlainObject(param)) {
               return;
             }
             param.$$path = `paths.${pathName}.${operationName}.parameters[${i}]`;
@@ -131,7 +131,7 @@ module.exports.validate = function({ resolvedSpec }) {
     // Assertation 1
     each(availableParameters, (parameterDefinition, i) => {
       if (
-        isObject(parameterDefinition) &&
+        isPlainObject(parameterDefinition) &&
         parameterDefinition.in === 'path' &&
         pathTemplates.indexOf(parameterDefinition.name) === -1
       ) {

src/plugins/validation/oas3/semantic-validators/discriminator.js

@@ -13,6 +13,7 @@
 const each = require('lodash/each');
 const has = require('lodash/has');
 const get = require('lodash/get');
+const isPlainObject = require('lodash/isPlainObject');
 const MessageCarrier = require('../../../utils/messageCarrier');
 
 module.exports.validate = function({ jsSpec }) {
@@ -27,7 +28,7 @@ module.exports.validate = function({ jsSpec }) {
       const { discriminator } = schema;
 
       // If discriminator is not an object, error out and return
-      if (typeof discriminator === 'object') {
+      if (isPlainObject(discriminator)) {
         if (!has(discriminator, 'propertyName')) {
           messages.addMessage(
             basePath.concat([schemaName, 'discriminator']).join('.'),

src/plugins/validation/swagger2/semantic-validators/form-data.js

@@ -1,5 +1,4 @@
-// Assertions are in the following order ( bailing as soon as we hit the firs assertion )
-//
+// Assertions are in the following order ( bailing as soon as we hit the first assertion )
 
 // Assertation typo
 // If a paramter with `in: formdata` exists, warn about typo ( it should be formData )
@@ -15,14 +14,14 @@
 // Assertation 3:
 // If a parameter with `in: formData` exists a consumes property ( inherited or inline ) my contain `application/x-www-form-urlencoded` or `multipart/form-data`
 
-const isObject = require('lodash/isObject');
+const isPlainObject = require('lodash/isPlainObject');
 const getIn = require('lodash/get');
 const MessageCarrier = require('../../../utils/messageCarrier');
 
 module.exports.validate = function({ resolvedSpec }) {
   const messages = new MessageCarrier();
 
-  if (!isObject(resolvedSpec)) {
+  if (!isPlainObject(resolvedSpec)) {
     return;
   }
 
@@ -74,7 +73,7 @@ module.exports.validate = function({ resolvedSpec }) {
   // Checks the operation for the presences of a consumes
   function hasConsumes(operation, consumes) {
     return (
-      isObject(operation) &&
+      isPlainObject(operation) &&
       Array.isArray(operation.consumes) &&
       operation.consumes.some(c => c === consumes)
     );
@@ -83,7 +82,7 @@ module.exports.validate = function({ resolvedSpec }) {
   // Warn about a typo, formdata => formData
   function assertationTypo(params, path) {
     const formDataWithTypos = params.filter(
-      p => isObject(p) && p['in'] === 'formdata'
+      p => isPlainObject(p) && p['in'] === 'formdata'
     );
 
     if (formDataWithTypos.length) {
@@ -106,9 +105,11 @@ module.exports.validate = function({ resolvedSpec }) {
   function assertationOne(params, path) {
     // Assertion 1
     const inBodyIndex = params.findIndex(
-      p => isObject(p) && p['in'] === 'body'
+      p => isPlainObject(p) && p['in'] === 'body'
+    );
+    const formData = params.filter(
+      p => isPlainObject(p) && p['in'] === 'formData'
     );
-    const formData = params.filter(p => isObject(p) && p['in'] === 'formData');
     const hasFormData = !!formData.length;
 
     if (~inBodyIndex && hasFormData) {
@@ -128,7 +129,7 @@ module.exports.validate = function({ resolvedSpec }) {
   // - b. The consumes property must have `multipart/form-data`
   function assertationTwo(params, path, operation) {
     const typeFileIndex = params.findIndex(
-      p => isObject(p) && p.type === 'file'
+      p => isPlainObject(p) && p.type === 'file'
     );
     // No type: file?
     if (!~typeFileIndex) {
@@ -163,7 +164,9 @@ module.exports.validate = function({ resolvedSpec }) {
 
   // If a parameter with `in: formData` exists, a consumes property ( inherited or inline ) my contain `application/x-www-form-urlencoded` or `multipart/form-data`
   function assertationThree(params, path, operation) {
-    const hasFormData = params.some(p => isObject(p) && p['in'] === 'formData');
+    const hasFormData = params.some(
+      p => isPlainObject(p) && p['in'] === 'formData'
+    );
 
     if (!hasFormData) {
       return;

src/plugins/validation/swagger2/semantic-validators/parameters.js

@@ -1,6 +1,7 @@
 // Assertation 1:
 // The items property for a parameter is required when its type is set to array
 
+const isPlainObject = require('lodash/isPlainObject');
 const { isParameterObject, walk } = require('../../../utils');
 const MessageCarrier = require('../../../utils/messageCarrier');
 
@@ -12,7 +13,7 @@ module.exports.validate = function({ resolvedSpec }) {
 
     // 1
     if (isContentsOfParameterObject) {
-      if (obj.type === 'array' && typeof obj.items !== 'object') {
+      if (obj.type === 'array' && !isPlainObject(obj.items)) {
         messages.addMessage(
           path,
           "Parameters with 'array' type require an 'items' property.",

src/plugins/validation/swagger2/semantic-validators/security-definitions.js

@@ -6,6 +6,7 @@
 // Assertation 5: "oauth2" security flow "accessCode" must have required string "tokenUrl", string "authorizationUrl" and object "scopes" parameters
 // Assertation 6: "oauth2" security flow "application" must have required string "tokenUrl", string "authorizationUrl" and object "scopes" parameters
 
+const isPlainObject = require('lodash/isPlainObject');
 const MessageCarrier = require('../../../utils/messageCarrier');
 
 module.exports.validate = function({ jsSpec }) {
@@ -109,7 +110,7 @@ module.exports.validate = function({ jsSpec }) {
           }
         }
 
-        if (typeof scopes !== 'object') {
+        if (!isPlainObject(scopes)) {
           messages.addMessageWithAuthId(
             path,
             "'scopes' is required property type object. The available scopes for the OAuth2 security scheme.",

test/plugins/validation/2and3/items-required-for-array-objects.test.js

@@ -174,6 +174,50 @@ describe('validation plugin - semantic - items required for array objects - Open
     expect(res.warnings.length).toEqual(0);
   });
 
+  it('should return an error when an array header object has a non-object `items` property', () => {
+    const spec = {
+      paths: {
+        '/pets': {
+          get: {
+            description:
+              'Returns all pets from the system that the user has access to',
+            responses: {
+              '200': {
+                description: 'pet response',
+                headers: {
+                  'X-MyHeader': {
+                    description: 'fake header',
+                    schema: {
+                      type: 'array',
+                      items: [
+                        {
+                          type: 'string'
+                        }
+                      ]
+                    }
+                  }
+                }
+              },
+              default: {
+                description: 'unexpected error'
+              }
+            }
+          }
+        }
+      }
+    };
+
+    const res = validate({ jsSpec: spec }, config);
+    expect(res.errors.length).toEqual(1);
+    expect(res.errors[0].path).toEqual(
+      'paths./pets.get.responses.200.headers.X-MyHeader.schema'
+    );
+    expect(res.errors[0].message).toEqual(
+      "Schema objects with 'array' type require an 'items' property"
+    );
+    expect(res.warnings.length).toEqual(0);
+  });
+
   it('should return a warning when a model does not define a required property', () => {
     const spec = {
       components: {

test/plugins/validation/oas3/discriminator.test.js

@@ -70,7 +70,7 @@ describe('validation plugin - semantic - oas3 discriminator', () => {
       ['components', 'schemas', 'Pet', 'discriminator'].join('.')
     );
     expect(res.errors[0].message).toEqual(
-      'Discriminator must be of type object with field name propertyName'
+      'Discriminator must be of type object'
     );
     expect(res.errors[1].path).toEqual(
       ['components', 'schemas', 'Food', 'discriminator'].join('.')