fix(resolver): handle recursive external JSON Schema refs

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 || '<specmap-base>'}#${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 || '<specmap-base>'}#${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.
  */

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: {