feat: add validator warning for binary string in "application/json" o…

…r parameter - added a findOctetSequences function to handle the logic of finding schema type: string, format: binary for cases of nested arrays, objects, nested arrays of type object, objects with properties that are nested arrays, and objects with properties that are objects, and the simple case where a schema uses type: string, format: binary directly. This function takes a schema object from a resolvedSpec and returns a list of paths to octet sequences (empty list if none found). - added logic to handle application/json request bodies that use schema type: string, format: binary - added logic to handle application/json response bodies of type: string, format: binary - added logic to handle parameters of type: string, format: binary - removed 'binary' as a valid format for type: string parameters. parameters of type: string, format: binary will result in "type+format not well-defined" error - added tests to ensure warnings are issued for request bodies, response bodies, and parameters with schema, type: string, format: binary - added complex tests to exercise combinations of nested arrays and objects that contain schema type: string, format: binary (complex tests done on response bodies) - added "json_or_param_binary_string" as to .defaultsForValidator as a warning in the shared.schemas section - added "json_or_param_binary_string" configuration option to the README.md rules and defaults sections in the schemas section
IBM · Feb 17, 2020 · 1802907 · 1802907
1 parent 7a05a41
commit 1802907
Show file tree

Hide file tree

Showing 11 changed files with 1,373 additions and 46 deletions.
diff --git a/README.md b/README.md
@@ -253,6 +253,7 @@ The supported rules are described below:
 | array_of_arrays             | Flag any schema with a 'property' of type `array` with items of type `array`. | shared   |
 | property_case_convention    | Flag any property with a `name` that does not follow a given case convention. snake_case_only must be 'off' to use. | shared |
 | enum_case_convention        | Flag any enum with a `value` that does not follow a given case convention. snake_case_only must be 'off' to use.    | shared |
+| json_or_param_binary_string | Flag parameters or application/json request and response bodies with schema type: string, format: binary. | shared |
 
 ##### security_definitions
 | Rule                        | Description                                                                           | Spec   |
@@ -424,6 +425,7 @@ The default values for each rule are described below.
 | array_of_arrays             | warning |
 | property_case_convention    | error, lower_snake_case |
 | enum_case_convention        | error, lower_snake_case |
+| json_or_param_binary_string | warning |
 
 ###### walker
 | Rule                          | Default |

diff --git a/src/.defaultsForValidator.js b/src/.defaultsForValidator.js
@@ -64,7 +64,8 @@ const defaults = {
       'description_mentions_json': 'warning',
       'array_of_arrays': 'warning',
       'property_case_convention': [ 'error', 'lower_snake_case'],
-      'enum_case_convention': [ 'error', 'lower_snake_case']
+      'enum_case_convention': [ 'error', 'lower_snake_case'],
+      'binary_string': 'warning'
     },
     'walker': {
       'no_empty_descriptions': 'error',

diff --git a/src/plugins/utils/findOctetSequencePaths.js b/src/plugins/utils/findOctetSequencePaths.js
@@ -0,0 +1,66 @@
+// Finds octet sequences (type: string, format: binary) in schemas including
+// nested arrays, objects, nested arrays of type object, objects with properties
+// that are nested arrays, and objects with properties that are objects This
+// function takes a resolved schema object (no refs) and returns a list of
+// paths to octet sequences (empty list if none found).
+
+const findOctetSequencePaths = (resolvedSchema, path) => {
+  if (!resolvedSchema) {
+    // schema is empty, no octet sequence
+    return [];
+  }
+
+  const pathsToOctetSequence = [];
+
+  if (resolvedSchema.type === 'string' && resolvedSchema.format === 'binary') {
+    pathsToOctetSequence.push(path);
+  } else if (resolvedSchema.type === 'array') {
+    pathsToOctetSequence.push(...arrayOctetSequences(resolvedSchema, path));
+  } else if (resolvedSchema.type === 'object') {
+    pathsToOctetSequence.push(...objectOctetSequences(resolvedSchema, path));
+  }
+
+  return pathsToOctetSequence;
+};
+
+function arrayOctetSequences(resolvedSchema, path) {
+  const arrayPathsToOctetSequence = [];
+  const arrayItems = resolvedSchema.items;
+  if (arrayItems !== undefined) {
+    const arrayPath = `${path}.items`;
+    if (arrayItems.type === 'string' && arrayItems.format === 'binary') {
+      arrayPathsToOctetSequence.push(arrayPath);
+    } else if (arrayItems.type === 'object' || arrayItems.type === 'array') {
+      arrayPathsToOctetSequence.push(
+        ...findOctetSequencePaths(arrayItems, arrayPath)
+      );
+    }
+  }
+  return arrayPathsToOctetSequence;
+}
+
+function objectOctetSequences(resolvedSchema, path) {
+  const objectPathsToOctetSequence = [];
+  const objectProperties = resolvedSchema.properties;
+  if (objectProperties) {
+    Object.keys(objectProperties).forEach(function(prop) {
+      const propPath = `${path}.properties.${prop}`;
+      if (
+        objectProperties[prop].type === 'string' &&
+        objectProperties[prop].format === 'binary'
+      ) {
+        objectPathsToOctetSequence.push(propPath);
+      } else if (
+        objectProperties[prop].type === 'object' ||
+        objectProperties[prop].type === 'array'
+      ) {
+        objectPathsToOctetSequence.push(
+          ...findOctetSequencePaths(objectProperties[prop], propPath)
+        );
+      }
+    });
+  }
+  return objectPathsToOctetSequence;
+}
+
+module.exports.findOctetSequencePaths = findOctetSequencePaths;
diff --git a/src/plugins/validation/2and3/semantic-validators/parameters-ibm.js b/src/plugins/validation/2and3/semantic-validators/parameters-ibm.js
@@ -181,7 +181,7 @@ function formatValid(obj, isOAS3) {
       return (
         !schema.format ||
         includes(
-          ['byte', 'binary', 'date', 'date-time', 'password'],
+          ['byte', 'date', 'date-time', 'password'],
           schema.format.toLowerCase()
         )
       );

diff --git a/src/plugins/validation/oas3/semantic-validators/operations.js b/src/plugins/validation/oas3/semantic-validators/operations.js
@@ -4,15 +4,21 @@
 // Assertation 2. Operations with non-form request bodies should set the `x-codegen-request-body-name`
 // annotation (for code generation purposes)
 
+// Assertation 3. Request bodies with application/json content should not use schema
+// type: string, format: binary.
+
 const pick = require('lodash/pick');
 const each = require('lodash/each');
 const { hasRefProperty } = require('../../../utils');
+const findOctetSequencePaths = require('../../../utils/findOctetSequencePaths')
+  .findOctetSequencePaths;
 
 module.exports.validate = function({ resolvedSpec, jsSpec }, config) {
   const result = {};
   result.error = [];
   result.warning = [];
 
+  const configSchemas = config.schemas;
   config = config.operations;
 
   const REQUEST_BODY_NAME = 'x-codegen-request-body-name';
@@ -82,6 +88,28 @@ module.exports.validate = function({ resolvedSpec, jsSpec }, config) {
               });
             }
           }
+
+          // Assertation 3
+          const binaryStringStatus = configSchemas.json_or_param_binary_string;
+          if (binaryStringStatus !== 'off') {
+            for (const mimeType of requestBodyMimeTypes) {
+              if (mimeType === 'application/json') {
+                const schemaPath = `paths.${pathName}.${opName}.requestBody.content.${mimeType}.schema`;
+                const octetSequencePaths = findOctetSequencePaths(
+                  requestBodyContent[mimeType].schema,
+                  schemaPath
+                );
+                const message =
+                  'JSON request/response bodies should not contain binary (type: string, format: binary) values.';
+                for (const p of octetSequencePaths) {
+                  result[binaryStringStatus].push({
+                    path: p,
+                    message
+                  });
+                }
+              }
+            }
+          }
         }
       }
     });

diff --git a/src/plugins/validation/oas3/semantic-validators/parameters.js b/src/plugins/validation/oas3/semantic-validators/parameters.js
@@ -7,13 +7,20 @@
 
 // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#parameterObject
 
+// Assertation 3:
+// A paramater should not use schema type: string, format: binary because there is now well-
+// defined way to encode an octet sequence in a URL.
+
 const { isParameterObject, walk } = require('../../../utils');
+const findOctetSequencePaths = require('../../../utils/findOctetSequencePaths')
+  .findOctetSequencePaths;
 
 module.exports.validate = function({ jsSpec }, config) {
   const result = {};
   result.error = [];
   result.warning = [];
 
+  const configSchemas = config.schemas;
   config = config.parameters;
 
   walk(jsSpec, [], function(obj, path) {
@@ -66,6 +73,38 @@ module.exports.validate = function({ jsSpec }, config) {
           });
         }
       }
+
+      const binaryStringStatus = configSchemas.json_or_param_binary_string;
+      if (binaryStringStatus !== 'off') {
+        const octetSequencePaths = [];
+        octetSequencePaths.push(
+          ...findOctetSequencePaths(obj.schema, `${path.join('.')}.schema`)
+        );
+        if (obj.content) {
+          Object.keys(obj.content).forEach(function(mimeType) {
+            if (mimeType === 'application/json') {
+              const paramContentPath = `${path.join(
+                '.'
+              )}.content.${mimeType}.schema`;
+              octetSequencePaths.push(
+                ...findOctetSequencePaths(
+                  obj.content[mimeType].schema,
+                  paramContentPath
+                )
+              );
+            }
+          });
+        }
+
+        for (const p of octetSequencePaths) {
+          const message =
+            'Parameters should not contain binary (type: string, format: binary) values.';
+          result[binaryStringStatus].push({
+            path: p.split('.'),
+            message
+          });
+        }
+      }
     }
   });
 

diff --git a/src/plugins/validation/oas3/semantic-validators/responses.js b/src/plugins/validation/oas3/semantic-validators/responses.js
@@ -12,30 +12,39 @@
 // Assertation 4:
 // A non-204 success response should define a response body
 
+// Assertation 3. Response bodies with application/json content should not use schema
+// type: string, format: binary.
+
 const { walk } = require('../../../utils');
+const findOctetSequencePaths = require('../../../utils/findOctetSequencePaths')
+  .findOctetSequencePaths;
 
 module.exports.validate = function({ resolvedSpec }, config) {
   const result = {};
   result.error = [];
   result.warning = [];
 
+  const configSchemas = config.schemas;
   config = config.responses;
 
   walk(resolvedSpec, [], function(obj, path) {
     const contentsOfResponsesObject =
       path[0] === 'paths' && path[path.length - 1] === 'responses';
 
     if (contentsOfResponsesObject) {
-      if (obj['204'] && obj['204'].content) {
-        result.error.push({
-          path: path.concat(['204', 'content']),
-          message: `A 204 response MUST NOT include a message-body.`
-        });
+      const [statusCodes, successCodes] = getResponseCodes(obj);
+
+      const binaryStringStatus = configSchemas.json_or_param_binary_string;
+      if (binaryStringStatus !== 'off') {
+        validateNoBinaryStringsInResponse(
+          obj,
+          result,
+          path,
+          binaryStringStatus
+        );
       }
-      const responseCodes = Object.keys(obj).filter(code =>
-        isResponseCode(code)
-      );
-      if (!responseCodes.length) {
+
+      if (!statusCodes.length) {
         const message =
           'Each `responses` object MUST have at least one response code.';
         const checkStatus = config.no_response_codes;
@@ -45,36 +54,33 @@ module.exports.validate = function({ resolvedSpec }, config) {
             message
           });
         }
+      } else if (!successCodes.length) {
+        const message =
+          'Each `responses` object SHOULD have at least one code for a successful response.';
+        const checkStatus = config.no_success_response_codes;
+        if (checkStatus !== 'off') {
+          result[checkStatus].push({
+            path,
+            message
+          });
+        }
       } else {
-        const successCodes = responseCodes.filter(
-          code => code.slice(0, 1) === '2'
-        );
-        if (!successCodes.length) {
-          const message =
-            'Each `responses` object SHOULD have at least one code for a successful response.';
-          const checkStatus = config.no_success_response_codes;
-          if (checkStatus !== 'off') {
-            result[checkStatus].push({
-              path,
-              message
-            });
-          }
-        } else {
-          const checkStatus = config.no_response_body;
-          // if response body rule is on, loops through success codes and issues warning (by default)
-          // for non-204 success responses without a response body
-          if (checkStatus !== 'off') {
-            for (const successCode of successCodes) {
-              if (successCode != '204' && !obj[successCode].content) {
-                result[checkStatus].push({
-                  path: path.concat([successCode]),
-                  message:
-                    `A ` +
-                    successCode +
-                    ` response should include a response body. Use 204 for responses without content.`
-                });
-              }
+        // validate success codes
+        for (const successCode of successCodes) {
+          if (successCode !== '204' && !obj[successCode].content) {
+            const checkStatus = config.no_response_body;
+            if (checkStatus !== 'off') {
+              const message = `A ${successCode} response should include a response body. Use 204 for responses without content.`;
+              result[checkStatus].push({
+                path: path.concat([successCode]),
+                message
+              });
             }
+          } else if (successCode === '204' && obj[successCode].content) {
+            result.error.push({
+              path: path.concat(['204', 'content']),
+              message: `A 204 response MUST NOT include a message-body.`
+            });
           }
         }
       }
@@ -84,7 +90,47 @@ module.exports.validate = function({ resolvedSpec }, config) {
   return { errors: result.error, warnings: result.warning };
 };
 
-function isResponseCode(code) {
+function getResponseCodes(responseObj) {
+  const statusCodes = Object.keys(responseObj).filter(code =>
+    isStatusCode(code)
+  );
+  const successCodes = statusCodes.filter(code => code.slice(0, 1) === '2');
+  return [statusCodes, successCodes];
+}
+
+function isStatusCode(code) {
   const allowedFirstDigits = ['1', '2', '3', '4', '5'];
   return code.length === 3 && allowedFirstDigits.includes(code.slice(0, 1));
 }
+
+function validateNoBinaryStringsInResponse(
+  responseObj,
+  result,
+  path,
+  binaryStringStatus
+) {
+  Object.keys(responseObj).forEach(function(responseCode) {
+    const responseBodyContent = responseObj[responseCode].content;
+    if (responseBodyContent) {
+      Object.keys(responseBodyContent).forEach(function(mimeType) {
+        if (mimeType === 'application/json') {
+          const schemaPath = `${path.join(
+            '.'
+          )}.${responseCode}.content.${mimeType}.schema`;
+          const octetSequencePaths = findOctetSequencePaths(
+            responseBodyContent[mimeType].schema,
+            schemaPath
+          );
+          const message =
+            'JSON request/response bodies should not contain binary (type: string, format: binary) values.';
+          for (const p of octetSequencePaths) {
+            result[binaryStringStatus].push({
+              path: p.split('.'),
+              message
+            });
+          }
+        }
+      });
+    }
+  });
+}
diff --git a/test/plugins/validation/2and3/parameters-ibm.js b/test/plugins/validation/2and3/parameters-ibm.js
@@ -594,7 +594,7 @@ describe('validation plugin - semantic - parameters-ibm', () => {
                 in: 'query',
                 schema: {
                   type: 'string',
-                  format: 'binary'
+                  format: 'byte'
                 }
               }
             ]