From 3e95d89279adde7ec089f274144545430f847f9a Mon Sep 17 00:00:00 2001 From: Kyle Fuller Date: Thu, 15 Jul 2021 16:53:40 +0100 Subject: [PATCH] fix(oas2): prevent treating overlapping paths as a circular reference A check that a reference to `#/definitions/User` from within `#/definitions/UserList` would incorrectly been interpreted as a circular reference as a simplistic check that the current path starts with the current reference. We should ensure that the reference is either identical, or relative from another (such as with a trailing slash). --- packages/openapi2-parser/lib/json-schema.js | 12 +++-- .../openapi2-parser/test/json-schema-test.js | 51 ++++++++++++++++++- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/packages/openapi2-parser/lib/json-schema.js b/packages/openapi2-parser/lib/json-schema.js index 8e3c1797e..cfcf756e7 100644 --- a/packages/openapi2-parser/lib/json-schema.js +++ b/packages/openapi2-parser/lib/json-schema.js @@ -76,12 +76,12 @@ const pathHasCircularReference = (paths, path, reference) => { const currentPath = (path || []).join('/'); // Check for direct circular reference - if (currentPath.startsWith(reference)) { + if (currentPath === reference || currentPath.startsWith(`${reference}/`)) { return true; } // Check for indirect circular Reference - if ((paths || []).find(p => p.startsWith(reference))) { + if ((paths || []).find(p => p === reference || p.startsWith(`${reference}/`))) { return true; } @@ -393,5 +393,11 @@ const convertSchemaDefinitions = (definitions) => { }; module.exports = { - isExtension, parseReference, lookupReference, dereference, convertSchema, convertSchemaDefinitions, + isExtension, + parseReference, + lookupReference, + dereference, + convertSchema, + convertSchemaDefinitions, + pathHasCircularReference, }; diff --git a/packages/openapi2-parser/test/json-schema-test.js b/packages/openapi2-parser/test/json-schema-test.js index 8cc8b3c6c..8e460919d 100644 --- a/packages/openapi2-parser/test/json-schema-test.js +++ b/packages/openapi2-parser/test/json-schema-test.js @@ -1,5 +1,7 @@ const { expect } = require('chai'); -const { convertSchema, convertSchemaDefinitions, dereference } = require('../lib/json-schema'); +const { + convertSchema, convertSchemaDefinitions, dereference, pathHasCircularReference, +} = require('../lib/json-schema'); describe('Swagger Schema to JSON Schema', () => { it('returns compatible schema when given valid JSON Schema', () => { @@ -853,3 +855,50 @@ describe('Dereferencing', () => { }); }); }); + + +describe('#pathHasCircularReference', () => { + it('does not detect circular reference when reference is not circular', () => { + const reference = '#/definitions/User'; + const currentPath = ['#', 'definitions', 'ListUsers']; + + expect(pathHasCircularReference([], currentPath, reference)).to.be.false; + }); + + it('detects circular reference when current path is reference path', () => { + const reference = '#/definitions/User'; + const currentPath = ['#', 'definitions', 'User']; + + expect(pathHasCircularReference([], currentPath, reference)).to.be.true; + }); + + it('detects circular reference when current path contains reference path', () => { + const reference = '#/definitions/User'; + const currentPath = ['#', 'definitions', 'User', 'properties', 'parent']; + + expect(pathHasCircularReference([], currentPath, reference)).to.be.true; + }); + + it('detects incircular reference when current path is in the prior referenced tree', () => { + const reference = '#/definitions/User'; + const currentPath = ['#', 'definitions', 'ListUsers']; + const currentTree = ['#/definitions/User/properties/children']; + + expect(pathHasCircularReference(currentTree, currentPath, reference)).to.be.true; + }); + + it('does not detect circular reference when reference last component prefixes current path', () => { + const reference = '#/definitions/User'; + const currentPath = ['#', 'definitions', 'UserList']; + + expect(pathHasCircularReference([], currentPath, reference)).to.be.false; + }); + + it('does not detect incircular reference when reference last component prefixes path in prior reference tree', () => { + const reference = '#/definitions/User'; + const currentPath = ['#', 'definitions', 'UserDetail']; + const currentTree = ['#/definitions/UserList/items']; + + expect(pathHasCircularReference(currentTree, currentPath, reference)).to.be.false; + }); +});