From e16e23f33994af45b648db857b615886d16fd7b5 Mon Sep 17 00:00:00 2001 From: Richard Weyer Date: Tue, 26 May 2026 12:58:21 +0200 Subject: [PATCH] fix(resolver): handle recursive external JSON Schema refs --- src/resolver/specmap/lib/refs.js | 37 ++++++-- .../strategies/openapi-2--3-0.js | 84 ++++++++++++++++++- 2 files changed, 111 insertions(+), 10 deletions(-) diff --git a/src/resolver/specmap/lib/refs.js b/src/resolver/specmap/lib/refs.js index 0f9498066..8c67ab558 100644 --- a/src/resolver/specmap/lib/refs.js +++ b/src/resolver/specmap/lib/refs.js @@ -131,7 +131,8 @@ const plugin = { return undefined; } - const { baseDoc } = specmap.getContext(fullPath); + const context = specmap.getContext(fullPath); + const { baseDoc } = context; if (typeof ref !== 'string') { return new JSONRefError('$ref: must be a string (JSON-Ref)', { $ref: ref, @@ -156,10 +157,15 @@ const plugin = { }); } + const fullyQualifiedRef = fullyQualifyRef(basePath, pointer); + const refStack = context.refStack || []; let promOrVal; let tokens; - if (pointerAlreadyInPath(pointer, basePath, parent, specmap)) { + if ( + refStack.indexOf(fullyQualifiedRef) > -1 || + pointerAlreadyInPath(pointer, basePath, parent, specmap) + ) { // Cyclic reference! // if `useCircularStructures` is not set, just leave the reference // unresolved, but absolutify it so that we don't leave an invalid $ref @@ -212,15 +218,17 @@ const plugin = { const absolutifiedRef = absolutifyPointer(ref, basePath); const patch = lib.replace(parent, promOrVal, { $$ref: absolutifiedRef }); - if (basePath && basePath !== baseDoc) { - return [patch, lib.context(parent, { baseDoc: basePath })]; + const resolvedContext = { refStack: refStack.concat(fullyQualifiedRef) }; + if (basePath || baseDoc) { + resolvedContext.baseDoc = basePath; } + const contextPatch = lib.context(parent, resolvedContext); try { // prevents circular values from being constructed, unless we specifically // want that to happen if (!patchValueAlreadyInPath(specmap.state, patch) || specmapInstance.useCircularStructures) { - return patch; + return [patch, contextPatch]; } } catch (e) { // if we're catching here, path traversal failed, so we should @@ -452,6 +460,10 @@ function arrayToJsonPointer(arr) { return `/${arr.map(escapeJsonPointerToken).join('/')}`; } +function fullyQualifyRef(basePath, pointer) { + return `${basePath || ''}#${pointer}`; +} + const pointerBoundaryChar = (c) => !c || c === '/' || c === '#'; function pointerIsAParent(pointer, parentPointer) { @@ -486,7 +498,7 @@ function pointerAlreadyInPath(pointer, basePath, parent, specmap) { } const parentPointer = arrayToJsonPointer(parent); - const fullyQualifiedPointer = `${basePath || ''}#${pointer}`; + const fullyQualifiedPointer = fullyQualifyRef(basePath, pointer); // dirty hack to strip `allof/[index]` from the path, in order to avoid cases // where we get false negatives because: @@ -498,7 +510,7 @@ function pointerAlreadyInPath(pointer, basePath, parent, specmap) { // solution: always throw the allOf constructs out of paths we store // TODO: solve this with a global register, or by writing more metadata in // either allOf or refs plugin - const safeParentPointer = parentPointer.replace(/allOf\/\d+\/?/g, ''); + const safeParentPointer = normalizeAllOfPath(parentPointer); // Case 1: direct cycle, e.g. a.b.c.$ref: '/a.b' // Detect by checking that the parent path doesn't start with pointer. @@ -517,10 +529,12 @@ function pointerAlreadyInPath(pointer, basePath, parent, specmap) { let currPath = ''; const hasIndirectCycle = parent.some((token) => { currPath = `${currPath}/${escapeJsonPointerToken(token)}`; + const safeCurrPath = normalizeAllOfPath(currPath); return ( - refs[currPath] && - refs[currPath].some( + refs[safeCurrPath] && + refs[safeCurrPath].some( (ref) => + ref === fullyQualifiedPointer || pointerIsAParent(ref, fullyQualifiedPointer) || pointerIsAParent(fullyQualifiedPointer, ref) ) @@ -538,6 +552,11 @@ function pointerAlreadyInPath(pointer, basePath, parent, specmap) { return undefined; } +function normalizeAllOfPath(pointer) { + const normalized = pointer.replace(/allOf\/\d+\/?/g, ''); + return normalized.length > 1 && normalized.endsWith('/') ? normalized.slice(0, -1) : normalized; +} + /** * Checks if the value of this patch ends up pointing to an ancestor along the path. */ diff --git a/test/subtree-resolver/strategies/openapi-2--3-0.js b/test/subtree-resolver/strategies/openapi-2--3-0.js index e27b79e27..fc7bdc9f9 100644 --- a/test/subtree-resolver/strategies/openapi-2--3-0.js +++ b/test/subtree-resolver/strategies/openapi-2--3-0.js @@ -1,6 +1,9 @@ import * as undici from 'undici'; import resolve from '../../../src/subtree-resolver/index.js'; +import { plugins } from '../../../src/resolver/specmap/index.js'; + +const { refs } = plugins; describe('subtree $ref resolver', () => { let mockAgent; @@ -19,7 +22,7 @@ describe('subtree $ref resolver', () => { }); beforeEach(() => { - // refs.clearCache() + refs.clearCache(); }); test('should resolve a subtree of an object, and return the targeted subtree', async () => { @@ -97,6 +100,85 @@ describe('subtree $ref resolver', () => { }); }); + test('should terminate recursive external JSON Schema refs while resolving a schema subtree', async () => { + const mockPool = mockAgent.get('http://mock.swagger.test'); + mockPool.intercept({ path: '/gdd/object.json' }).reply( + 200, + { + type: 'object', + properties: { + entries: { + type: 'array', + items: { + $ref: 'http://mock.swagger.test/gdd/basic-types.json', + }, + }, + }, + }, + { headers: { 'Content-Type': 'application/json' } } + ); + mockPool.intercept({ path: '/gdd/basic-types.json' }).reply( + 200, + { + oneOf: [ + { + type: 'array', + items: { + $ref: 'http://mock.swagger.test/gdd/object.json', + }, + }, + { + type: 'object', + properties: { + nested: { + $ref: 'http://mock.swagger.test/gdd/object.json', + }, + }, + }, + ], + }, + { headers: { 'Content-Type': 'application/json' } } + ); + + const input = { + openapi: '3.0.3', + info: { + title: 'Recursive external JSON Schema refs', + version: '1.0.0', + }, + paths: {}, + components: { + schemas: { + RenderTargetSchema: { + type: 'object', + properties: { + gdd: { + $ref: '#/components/schemas/ShallowGDDObjectSchema', + }, + }, + }, + ShallowGDDObjectSchema: { + allOf: [ + { + $ref: 'http://mock.swagger.test/gdd/object.json', + }, + ], + }, + }, + }, + }; + + const res = await resolve(input, ['components', 'schemas', 'RenderTargetSchema']); + + expect(res.errors).toEqual([]); + expect(res.spec.properties.gdd.properties.entries.items.oneOf[0].items.$ref).toEqual( + 'http://mock.swagger.test/gdd/object.json' + ); + expect( + res.spec.properties.gdd.properties.entries.items.oneOf[1].properties.nested.$ref + ).toEqual('http://mock.swagger.test/gdd/object.json'); + }); + test('should return null when the path is invalid', async () => { const input = { a: {