Skip to content

Commit

Permalink
fix: properly handle circular regressions to prevent call stack excep…
Browse files Browse the repository at this point in the history
…tions (#253)

A regression was recently introduced to the validator in the circular references
utility that prevented such references from being caught and sanitized. This resulted
in "call stack exceeded" expections for certain patterns using circular references.

The regression came in the form of checking for only "plain" objects, rather than
all "objects" (which include arrays). This happened in a recent community PR and I
didn't realize the consequences when I reviewed and merged. I went back and double
checked every instance of this change in that PR and believe this is the only one
that is problematic. I fixed it and added a new test case that would've caught this
regression had it been there.
  • Loading branch information
dpopp07 authored Mar 1, 2021
1 parent 927530a commit 3696487
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/cli-validator/utils/circular-references-ibm.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// correct them,

const isPlainObject = require('lodash/isPlainObject');
const isObject = require('lodash/isObject');

// and return them as problems if applicable
const validate = function({ jsSpec, resolvedSpec }, config) {
Expand Down Expand Up @@ -29,7 +30,8 @@ const correctSpec = function(resolvedSpec) {
return null;
}

if (!isPlainObject(object)) {
// we need to catch arrays here, in addition to plain objects
if (!isObject(object)) {
return null;
}

Expand Down
29 changes: 29 additions & 0 deletions test/cli-validator/mockFiles/composite-pattern.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
openapi: '3.0.2'
info:
title: Repro "call stack" error
description: Reproduce the "Maximum call stack size exceeded" error with Composite-pattern schemas
version: '0.1.0'
paths:
/example:
post:
summary: Just an example path
description: Just an example path to make this a valid spec
operationId: postExample
components:
schemas:
Pair:
description: A pair of two elements or pairs or both
type: object
properties:
a:
oneOf:
- $ref: "#/components/schemas/Element"
- $ref: "#/components/schemas/Pair"
b:
oneOf:
- $ref: "#/components/schemas/Element"
- $ref: "#/components/schemas/Pair"
Element:
description: An elemental numeric value
type: number
example: 42
23 changes: 23 additions & 0 deletions test/cli-validator/tests/circular-refs-validator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,27 @@ describe('cli tool - test circular reference module', function() {
const realPath = circularRefsValidator.convert(testObject, resolvedPaths);
expect(realPath[0]).toEqual('definitions.something.foo');
});

it('should correctly validate a file using the composite pattern', async function() {
const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {});

const program = {};
program.args = ['./test/cli-validator/mockFiles/composite-pattern.yaml'];
program.default_mode = true;

const exitCode = await commandLineValidator(program);

const capturedText = getCapturedText(consoleSpy.mock.calls);
consoleSpy.mockRestore();

expect(exitCode).toEqual(0);

const allOutput = capturedText.join('');

expect(
allOutput.includes(
'Swagger object should not contain circular references.'
)
).toEqual(true);
});
});

0 comments on commit 3696487

Please sign in to comment.