Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing issue #309 - References outside components and paths are not resolved #645

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
13 changes: 2 additions & 11 deletions lib/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ const _ = require('lodash'),
parse = require('./parse.js'),
{ ParseError } = require('./common/ParseError'),
Utils = require('./utils'),
crypto = require('crypto');
crypto = require('crypto'),
{ isCircularReference } = require('./utils');

let path = require('path'),
pathBrowserify = require('path-browserify'),
Expand Down Expand Up @@ -481,16 +482,6 @@ function findReferenceByMainKeyInTraceFromContext(documentContext, mainKeyInTrac
return relatedRef;
}

/**
* Verifies if a node has same content as one of the parents so it is a circular ref
* @param {function} traverseContext - The context of the traverse function
* @param {object} contentFromTrace - The resolved content of the node to deref
* @returns {boolean} whether is circular reference or not.
*/
function isCircularReference(traverseContext, contentFromTrace) {
return traverseContext.parents.find((parent) => { return parent.node === contentFromTrace; }) !== undefined;
}

/**
* Modifies content of a node if it is circular reference.
*
Expand Down
70 changes: 69 additions & 1 deletion lib/deref.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const _ = require('lodash'),
mergeAllOf = require('json-schema-merge-allof'),
{ typesMap } = require('./common/schemaUtilsCommon'),
{ isLocalRef } = require('./jsonPointer'),
PARAMETER_SOURCE = {
REQUEST: 'REQUEST',
RESPONSE: 'RESPONSE'
Expand Down Expand Up @@ -29,7 +30,9 @@ const _ = require('lodash'),
],
DEFAULT_SCHEMA_UTILS = require('./30XUtils/schemaUtils30X'),
traverseUtility = require('traverse'),
PROPERTIES_TO_ASSIGN_ON_CASCADE = ['type', 'nullable'];
PROPERTIES_TO_ASSIGN_ON_CASCADE = ['type', 'nullable'],
CIRCULAR_REF_KEY = '$circularRef',
{ isCircularReference } = require('./utils');

/**
* @param {*} currentNode - the object from which you're trying to find references
Expand Down Expand Up @@ -201,6 +204,9 @@ module.exports = {
return this.resolveAllOf(schema.allOf, parameterSourceOption, components, schemaResolutionCache, resolveFor,
resolveTo, stack, _.cloneDeep(seenRef), stackLimit);
}
if (schema.hasOwnProperty(CIRCULAR_REF_KEY)) {
return schema;
}
if (schema.$ref && _.isFunction(schema.$ref.split)) {
let refKey = schema.$ref,
outerProperties = concreteUtils.getOuterPropsIfIsSupported(schema);
Expand Down Expand Up @@ -388,5 +394,67 @@ module.exports = {
}

return schema;
},

/**
* Take a $ref and a spec and return the referenced value
* @param {object} spec the parsed spec object
* @param {string} reference the $ref value to dereference
* @returns {object} the dereferenced $ref value
*/
dereferenceElement: function (spec, reference) {
LuisTejedaS marked this conversation as resolved.
Show resolved Hide resolved
let splitRef = reference.split('/'),
resolvedContent;

splitRef = splitRef.slice(1).map((elem) => {
// https://swagger.io/docs/specification/using-ref#escape
// since / is the default delimiter, slashes are escaped with ~1
return decodeURIComponent(
erfemega marked this conversation as resolved.
Show resolved Hide resolved
elem
.replace(/~1/g, '/')
.replace(/~0/g, '~')
);
});

resolvedContent = this._getEscaped(spec, splitRef);
return resolvedContent;
},

/**
* Dereferences the values referenced from out of components/schemas element
* @param {object} spec The parsed specification
* @param {object} constraintRegexp The regexp to match against the $ref element's value
* @returns {object} The specification with the values referenced from other place than components/schemas
*/
dereferenceByConstraint: function(spec, constraintRegexp) {
let dereferencedSpec = _.cloneDeep(spec),
that = this,
seenContents = {};

traverseUtility(dereferencedSpec).forEach(function (property) {
if (
typeof property === 'object' &&
_.size(property) === 1 &&
property.hasOwnProperty('$ref') &&
isLocalRef(property, '$ref') &&
(
property.$ref.match(constraintRegexp)
)
) {
const contentFromRef = seenContents[property.$ref];
if (isCircularReference(this, contentFromRef)) {
this.update({ [CIRCULAR_REF_KEY]: '<Circular reference to ' + property.$ref + ' detected>' });
}
else {
const dereferencedContent = that.dereferenceElement(
dereferencedSpec,
property.$ref
);
seenContents[property.$ref] = dereferencedContent;
this.update(dereferencedContent);
}
}
});
return dereferencedSpec;
}
};
3 changes: 2 additions & 1 deletion lib/schemaUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,8 @@ module.exports = {
*/
generateTrieFromPaths: function (spec, options, fromWebhooks = false) {
options = _.merge({}, defaultOptions, options);
// We only dereference the elements(if exist) that references to components/pathItems
spec = deref.dereferenceByConstraint(spec, /#\/components\/pathItems/);
erfemega marked this conversation as resolved.
Show resolved Hide resolved
let concreteUtils = getConcreteSchemaUtils({ type: 'json', data: spec }),
specComponentsAndUtils = {
concreteUtils
Expand Down Expand Up @@ -595,7 +597,6 @@ module.exports = {
return methods;
};
Object.assign(specComponentsAndUtils, concreteUtils.getRequiredData(spec));

for (path in paths) {
if (paths.hasOwnProperty(path)) {
currentPathObject = paths[path];
Expand Down
8 changes: 6 additions & 2 deletions lib/schemapack.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
'use strict';

const deref = require('./deref.js'),
{ getNegativeRegexp } = require('./utils.js');

// This is the default collection name if one can't be inferred from the OpenAPI spec
const COLLECTION_NAME = 'Imported from OpenAPI 3.0',
{ getConcreteSchemaUtils } = require('./common/versionUtils.js'),
Expand Down Expand Up @@ -272,8 +275,9 @@ class SchemaPack {
if (error) {
return callback(error);
}

this.openapi = newOpenapi;
// Here we resolve only the elements to reference to aou of components.
// All the dereferences to components are resolved in their own process
this.openapi = deref.dereferenceByConstraint(newOpenapi, getNegativeRegexp('#/components/'));
erfemega marked this conversation as resolved.
Show resolved Hide resolved
// this cannot be attempted before validation
specComponentsAndUtils = { concreteUtils };
Object.assign(specComponentsAndUtils, concreteUtils.getRequiredData(this.openapi));
Expand Down
21 changes: 21 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,26 @@ module.exports = {
res.push(segment);
}
return res.join('/');
},

/**
* Generate a regexp to match with strings that does not contain the provided string
* @param {string} negatedWord The string we want to appear in the string
* @returns {object} The regexp that matches with strings that does not contain the provided string
*/
getNegativeRegexp: function (negatedWord) {
const regexpValue = `^(?!.*?${negatedWord}).*$`,
regexp = new RegExp(regexpValue);
return regexp;
},

/**
* Verifies if a node has same content as one of the parents in traverseUtil context so it is a circular ref
* @param {function} traverseContext - The context of the traverse function
* @param {object} contentFromTrace - The resolved content of the node to deref
* @returns {boolean} whether is circular reference or not.
*/
isCircularReference: function (traverseContext, contentFromTrace) {
return traverseContext.parents.find((parent) => { return parent.node === contentFromTrace; }) !== undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erfemega Is this a deep comparison?

}
};
Loading