Skip to content

Commit

Permalink
fix: adjust path location regular expressions to handle edge cases
Browse files Browse the repository at this point in the history
A couple of the regular expressions we use for determining the location
of a given OpenAPI artifact based on the path had minor bugs that would
prevent expected operations for edge cases. These are now resolved.
Also, they are now packaged into utilities for easy sharing between rules
and increased readability within the rule function code itself.

Signed-off-by: Dustin Popp <[email protected]>
  • Loading branch information
dpopp07 committed Dec 16, 2024
1 parent 42752ca commit f1ef03e
Show file tree
Hide file tree
Showing 7 changed files with 807 additions and 55 deletions.
2 changes: 1 addition & 1 deletion .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"files": "package-lock.json|^.secrets.baseline$",
"lines": null
},
"generated_at": "2024-11-27T14:56:49Z",
"generated_at": "2024-12-16T19:27:38Z",
"plugins_used": [
{
"name": "AWSKeyDetector"
Expand Down
33 changes: 9 additions & 24 deletions packages/ruleset/src/functions/binary-schemas.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const { isBinarySchema } = require('@ibm-cloud/openapi-ruleset-utilities');

const {
isJsonMimeType,
pathMatchesRegexp,
isParamSchema,
isParamContentSchema,
isRequestBodySchema,
isResponseSchema,
LoggerFactory,
} = require('../utils');

Expand Down Expand Up @@ -49,20 +52,10 @@ function binarySchemaCheck(schema, path) {

// 1. Is it the schema for a parameter within a path item or operation?
// a. ...parameters[n].schema
const isParamSchema = pathMatchesRegexp(
path,
/^paths,.*,parameters,\d+,schema$/
);

// b. ...parameters[n].content.*.schema
const isParamContentSchema = pathMatchesRegexp(
path,
/^paths,.*,parameters,\d+,content,.*schema$/
);

if (
isParamSchema ||
(isParamContentSchema && isJsonMimeType(path[path.length - 2]))
isParamSchema(path) ||
(isParamContentSchema(path) && isJsonMimeType(path[path.length - 2]))
) {
logger.debug(`${ruleId}: it's a parameter schema!`);
return [
Expand All @@ -75,11 +68,7 @@ function binarySchemaCheck(schema, path) {
}

// 2. Is it the schema for a JSON requestBody?
const isRequestBodySchema = pathMatchesRegexp(
path,
/^paths,.*,requestBody,content,.*,schema$/
);
if (isRequestBodySchema && isJsonMimeType(path[path.length - 2])) {
if (isRequestBodySchema(path) && isJsonMimeType(path[path.length - 2])) {
logger.debug(`${ruleId}: it's a requestBody schema!`);
return [
{
Expand All @@ -91,11 +80,7 @@ function binarySchemaCheck(schema, path) {
}

// 3. Is it the schema for a JSON response?
const isResponseSchema = pathMatchesRegexp(
path,
/^paths,.*,responses,[^,]*,content,.*,schema$/
);
if (isResponseSchema && isJsonMimeType(path[path.length - 2])) {
if (isResponseSchema(path) && isJsonMimeType(path[path.length - 2])) {
logger.debug(`${ruleId}: it's a response schema!`);
return [
{
Expand Down
26 changes: 13 additions & 13 deletions packages/ruleset/src/functions/property-description-exists.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const {
schemaHasConstraint,
validateSubschemas,
} = require('@ibm-cloud/openapi-ruleset-utilities');
const { LoggerFactory, pathMatchesRegexp } = require('../utils');
const { LoggerFactory, isSchemaProperty } = require('../utils');

let ruleId;
let logger;
Expand All @@ -23,20 +23,20 @@ module.exports = function (schema, _opts, context) {

function propertyDescriptionExists(schema, path) {
// If "schema" is a schema property, then check for a description.
const isSchemaProperty = pathMatchesRegexp(path, /^.*,properties,[^,]*$/);
if (isSchemaProperty) {
if (isSchemaProperty(path)) {
logger.debug(
`${ruleId}: checking schema property at location: ${path.join('.')}`
);
}
if (isSchemaProperty && !schemaHasDescription(schema)) {
logger.debug(`${ruleId}: no description found!`);
return [
{
message: 'Schema properties should have a non-empty description',
path,
},
];

if (!schemaHasDescription(schema)) {
logger.debug(`${ruleId}: no description found!`);
return [
{
message: 'Schema properties should have a non-empty description',
path,
},
];
}
}

return [];
Expand Down
20 changes: 3 additions & 17 deletions packages/ruleset/src/functions/schema-description-exists.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const {
validateSubschemas,
schemaHasConstraint,
} = require('@ibm-cloud/openapi-ruleset-utilities');
const { LoggerFactory, pathMatchesRegexp } = require('../utils');
const { LoggerFactory, isPrimarySchema } = require('../utils');

let ruleId;
let logger;
Expand All @@ -22,23 +22,9 @@ module.exports = function (schema, _opts, context) {
};

function schemaDescriptionExists(schema, path) {
//
// Check to see if "path" represents a primary schema (i.e. not a schema property).
// Note: the regexp used below uses a "lookbehind assertion"
// (i.e. the "(?<!,parameters,\d+)" part) to match paths that end with the "schema" part,
// but not paths where "schema" is preceded by "parameters" and "<digits>".
// So a primary schema is one with a path like:
// ["paths", "/v1/drinks", "requestBody", "content", "application/json", "schema"]
// but not one with a path like these:
// ["paths", "/v1/drinks", "parameters", "0", "schema"]
// ["paths", "/v1/drinks", "requestBody", "content", "application/json", "schema", "properties", "prop1"]
//
const isPrimarySchema = pathMatchesRegexp(
path,
/^.*(?<!,parameters,\d+),schema$/
);
// If "schema" is a primary schema, then check for a description.
if (isPrimarySchema) {
if (isPrimarySchema(path)) {
logger.debug(`${ruleId}: checking schema at location: ${path.join('.')}`);

if (!schemaHasDescription(schema)) {
Expand Down
1 change: 1 addition & 0 deletions packages/ruleset/src/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ module.exports = {
pathMatchesRegexp: require('./path-matches-regexp'),
...require('./mimetype-utils'),
...require('./pagination-utils'),
...require('./path-location-utils'),
...require('./schema-finding-utils'),
};
119 changes: 119 additions & 0 deletions packages/ruleset/src/utils/path-location-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/**
* Copyright 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const pathMatchesRegexp = require('./path-matches-regexp');

/**
* Returns true if the path points to a schema object within
* a media type object defined on a parameter's content field.
*
* @param {array} path the path to the openapi artifact
* @returns true if path is to a parameter content schema object
*/
function isParamContentSchema(path) {
// ...parameters[n].content.*.schema
return pathMatchesRegexp(
path,
/^paths,.+,parameters,\d+,content,[^,]+,schema$/
);
}

/**
* Returns true if the path points to a schema object defined
* directly on a parameter.
*
* @param {array} path the path to the openapi artifact
* @returns true if path is to a parameter schema object
*/
function isParamSchema(path) {
// Schema for a parameter within a path item or operation:
// ...parameters[n].schema
return pathMatchesRegexp(path, /^paths,.+,parameters,\d+,schema$/);
}

/**
* Returns true if the path points to a schema object that
* is a "primary" schema, meaning it is not the schema of a
* parameter or a property of another schema. This ultimately
* translates to "content" schemas (for request bodies,
* response bodies, and parameters).
*
* So a primary schema is one with a path like:
* - ["paths", "/v1/drinks", "requestBody", "content", "application/json", "schema"]
*
* but not one with a path like these:
* - ["paths", "/v1/drinks", "parameters", "0", "schema"]
* - ["paths", "/v1/drinks", "requestBody", "content", "application/json", "schema", "properties", "prop1"]
*
* @param {array} path the path to the openapi artifact
* @returns true if path is to a primary, non-property schema
*/
function isPrimarySchema(path) {
// Note: the regexp used below uses a "lookbehind assertion"
// (i.e. the "(?<!properties)" part) to ensure we don't match any
// schemas in "properties" that may have names like "content" or "schema".
return pathMatchesRegexp(path, /^.*(?<!properties),content,[^,]+,schema$/);
}

/**
* Returns true if the path points to a schema object within
* a media type object defined on a request body.
*
* @param {array} path the path to the openapi artifact
* @returns true if path is to a request body content schema object
*/
function isRequestBodySchema(path) {
return pathMatchesRegexp(path, /^paths,.+,requestBody,content,[^,]+,schema$/);
}

/**
* Returns true if the path points to a schema object within
* a media type object defined on a response body.
*
* @param {array} path the path to the openapi artifact
* @returns true if path is to a response body content schema object
*/
function isResponseSchema(path) {
return pathMatchesRegexp(
path,
/^paths,([^,]+,){2}responses,[^,]+,content,[^,]+,schema$/
);
}

/**
* Returns true if the path points to a schema object that is
* explictly defined as a property of another object schema.
*
* @param {array} path the path to the openapi artifact
* @returns true if path is to a schema property schema object
*/
function isSchemaProperty(path) {
const matches = pathMatchesRegexp(path, /^.+,properties,[^,]+$/);

// Handle the rare edge case where a property is named "properties"
// and we end up in its properties field, which should return false.
// This scenario is rather difficult to handle with a regular expression.
if (matches && path.at(-1) === 'properties') {
// Count the amount of times the segment 'properties' occurs at the end
// of the path. Look for an even number.
const count = path
.slice()
.reverse()
.findIndex(p => p !== 'properties');

return count % 2 === 0;
}

return matches;
}

module.exports = {
isParamContentSchema,
isParamSchema,
isPrimarySchema,
isRequestBodySchema,
isResponseSchema,
isSchemaProperty,
};
Loading

0 comments on commit f1ef03e

Please sign in to comment.