Skip to content

Commit

Permalink
feat: add validator warning for binary string in "application/json" o…
Browse files Browse the repository at this point in the history
…r parameter

- added a hasOctetSequence module 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 "binary_string" as to .defaultsForValidator as a warning in the shared.schemas section
- added "binary_string" configuration option to the README.md rules and defaults sections in the schemas section
  • Loading branch information
Barrett Schonefeld committed Feb 14, 2020
1 parent 7a05a41 commit 37d6184
Show file tree
Hide file tree
Showing 11 changed files with 1,378 additions and 51 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
| binary_string | Flag parameters or application/json request and response bodies with schema type: string, format: binary. | shared |

##### security_definitions
| Rule | Description | Spec |
Expand Down Expand Up @@ -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 |
| binary_string | warning |

###### walker
| Rule | Default |
Expand Down
3 changes: 2 additions & 1 deletion src/.defaultsForValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
62 changes: 62 additions & 0 deletions src/plugins/utils/hasOctetSequence.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// 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 schema object from a resolvedSpec and returns a list of
// paths to octet sequences (empty list if none found).

const hasOctetSequence = (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') {
arrayOctetSequences(resolvedSchema, pathsToOctetSequence, path);
} else if (resolvedSchema.type === 'object') {
objectOctetSequences(resolvedSchema, pathsToOctetSequence, path);
}

return pathsToOctetSequence;
};

function arrayOctetSequences(resolvedSchema, pathsToOctetSequence, path) {
let arrayItems = resolvedSchema.items;
let arrayPath = `${path}.items`;
while (arrayItems !== undefined) {
if (arrayItems.type === 'string' && arrayItems.format === 'binary') {
pathsToOctetSequence.push(arrayPath);
} else if (arrayItems.type === 'object') {
pathsToOctetSequence.push(...hasOctetSequence(arrayItems, arrayPath));
}
arrayItems = arrayItems.items;
arrayPath += '.items';
}
}

function objectOctetSequences(resolvedSchema, pathsToOctetSequence, path) {
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'
) {
pathsToOctetSequence.push(propPath);
} else if (
objectProperties[prop].type === 'object' ||
objectProperties[prop].type === 'array'
) {
pathsToOctetSequence.push(
...hasOctetSequence(objectProperties[prop], propPath)
);
}
});
}
}

module.exports.octetSequences = hasOctetSequence;
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
);
Expand Down
30 changes: 29 additions & 1 deletion src/plugins/validation/oas3/semantic-validators/operations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 octetSequences = require('../../../utils/hasOctetSequence')
.octetSequences;

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';
Expand Down Expand Up @@ -78,10 +84,32 @@ module.exports.validate = function({ resolvedSpec, jsSpec }, config) {
'Operations with non-form request bodies should set a name with the x-codegen-request-body-name annotation.';
result[checkStatus].push({
path: `paths.${pathName}.${opName}`,
message
message: message
});
}
}

// Assertation 3
const binaryStringStatus = configSchemas.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 = octetSequences(
requestBodyContent[mimeType].schema,
schemaPath
);
const message =
'No well-defined way to represent bare octets (type: string, format: binary) as JSON.';
for (const p of octetSequencePaths) {
result[binaryStringStatus].push({
path: p,
message: message
});
}
}
}
}
}
}
});
Expand Down
46 changes: 42 additions & 4 deletions src/plugins/validation/oas3/semantic-validators/parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 octetSequences = require('../../../utils/hasOctetSequence')
.octetSequences;

module.exports.validate = function({ jsSpec }, config) {
const result = {};
result.error = [];
result.warning = [];

const configSchemas = config.schemas;
config = config.parameters;

walk(jsSpec, [], function(obj, path) {
Expand All @@ -28,7 +35,7 @@ module.exports.validate = function({ jsSpec }, config) {
const checkStatus = config.no_in_property;
if (checkStatus !== 'off') {
result[checkStatus].push({
path,
path: path,
message: 'Parameters MUST have an `in` property.'
});
}
Expand All @@ -50,7 +57,7 @@ module.exports.validate = function({ jsSpec }, config) {
const checkStatus = config.missing_schema_or_content;
if (checkStatus !== 'off') {
result[checkStatus].push({
path,
path: path,
message:
'Parameters MUST have their data described by either `schema` or `content`.'
});
Expand All @@ -60,14 +67,45 @@ module.exports.validate = function({ jsSpec }, config) {
const checkStatus = config.has_schema_and_content;
if (checkStatus !== 'off') {
result[checkStatus].push({
path,
path: path,
message:
'Parameters MUST NOT have both a `schema` and `content` property.'
});
}
}

const binaryStringStatus = configSchemas.binary_string;
if (binaryStringStatus !== 'off') {
const octetSequencePaths = [];
octetSequencePaths.push(
...octetSequences(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(
...octetSequences(
obj.content[mimeType].schema,
paramContentPath
)
);
}
});
}

for (const p of octetSequencePaths) {
const message =
'No well-defined way to represent bare octets (type: string, format: binary) as JSON.';
result[binaryStringStatus].push({
path: p.split('.'),
message: message
});
}
}
}
});

return { errors: result.error, warnings: result.warning };
};

0 comments on commit 37d6184

Please sign in to comment.