fix: adjusting dereference caching so it doesn't send itself to infinite loops

Author: erunion

lib/dereference.ts

@@ -69,11 +69,8 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
     circular: false,
   };
 
-  if (options && options.timeoutMs) {
-    if (Date.now() - startTime > options.timeoutMs) {
-      throw new TimeoutError(options.timeoutMs);
-    }
-  }
+  checkDereferenceTimeout<S, O>(startTime, options);
+
   const derefOptions = (options.dereference || {}) as DereferenceOptions;
   const isExcludedPath = derefOptions.excludedPathMatcher || (() => false);
 
@@ -98,6 +95,8 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
         result.value = dereferenced.value;
       } else {
         for (const key of Object.keys(obj)) {
+          checkDereferenceTimeout<S, O>(startTime, options);
+
           const keyPath = Pointer.join(path, key);
           const keyPathFromRoot = Pointer.join(pathFromRoot, key);
 
@@ -214,7 +213,17 @@ function dereference$Ref<S extends object = JSONSchema, O extends ParserOptions<
   const $refPath = url.resolve(shouldResolveOnCwd ? url.cwd() : path, $ref.$ref);
 
   const cache = dereferencedCache.get($refPath);
-  if (cache && !cache.circular) {
+
+  if (cache) {
+    // If the object we found is circular we can immediately return it because it would have been
+    // cached with everything we need already and we don't need to re-process anything inside it.
+    //
+    // If the cached object however is _not_ circular and there are additional keys alongside our
+    // `$ref` pointer here we should merge them back in and return that.
+    if (cache.circular) {
+      return cache;
+    }
+
     const refKeys = Object.keys($ref);
     if (refKeys.length > 1) {
       const extraKeys = {};
@@ -294,6 +303,20 @@ function dereference$Ref<S extends object = JSONSchema, O extends ParserOptions<
   return dereferencedObject;
 }
 
+/**
+ * Check if we've run past our allowed timeout and throw an error if we have.
+ *
+ * @param startTime - The time when the dereferencing started.
+ * @param options
+ */
+function checkDereferenceTimeout<S extends object = JSONSchema, O extends ParserOptions<S> = ParserOptions<S>>(startTime: number, options: O): void {
+  if (options && options.timeoutMs) {
+    if (Date.now() - startTime > options.timeoutMs) {
+      throw new TimeoutError(options.timeoutMs);
+    }
+  }
+}
+
 /**
  * Called when a circular reference is found.
  * It sets the {@link $Refs#circular} flag, executes the options.dereference.onCircular callback,

test/specs/circular-extensive/circular-extensive.spec.ts

@@ -0,0 +1,105 @@
+import { describe, it } from "vitest";
+import $RefParser from "../../../lib/index.js";
+import helper from "../../utils/helper.js";
+import path from "../../utils/path.js";
+
+import { expect } from "vitest";
+
+describe("Schema with an extensive amount of circular $refs", () => {
+  it.only("should dereference successfully", async () => {
+    const circularRefs = new Set<string>();
+
+    const parser = new $RefParser<Record<string, any>>();
+    const schema = await parser.dereference(path.rel("test/specs/circular-extensive/schema.json"), {
+      dereference: {
+        onCircular: (ref: string) => circularRefs.add(ref),
+      },
+    });
+
+    // Ensure that a non-circular $ref was dereferenced.
+    expect(schema.components?.schemas?.ArrayOfMappedData).toStrictEqual({
+      type: 'array',
+      items: {
+        type: 'object',
+        properties: {
+          mappingTypeName: { type: 'string' },
+          sourceSystemValue: { type: 'string' },
+          mappedValueID: { type: 'string' },
+          mappedValue: { type: 'string' }
+        },
+        additionalProperties: false
+      }
+    });
+
+    // Ensure that a circular $ref **was** dereferenced.
+    expect(circularRefs).toHaveLength(23);
+    expect(schema.components?.schemas?.Customer?.properties?.customerNode).toStrictEqual({
+      "type": "array",
+      "items": {
+        type: 'object',
+        properties: {
+          customerNodeGuid: expect.any(Object),
+          customerGuid: expect.any(Object),
+          nodeId: expect.any(Object),
+          customerGu: expect.any(Object)
+        },
+        additionalProperties: false
+      },
+    });
+  });
+
+  it("should dereference successfully with `dereference.circular` is `ignore`", async () => {
+    const circularRefs = new Set<string>();
+
+    const parser = new $RefParser<Record<string, any>>();
+    const schema = await parser.dereference(path.rel("test/specs/circular-extensive/schema.json"), {
+      dereference: {
+        onCircular: (ref: string) => circularRefs.add(ref),
+        circular: 'ignore',
+      },
+    });
+
+    // Ensure that a non-circular $ref was dereferenced.
+    expect(schema.components?.schemas?.ArrayOfMappedData).toStrictEqual({
+      type: 'array',
+      items: {
+        type: 'object',
+        properties: {
+          mappingTypeName: { type: 'string' },
+          sourceSystemValue: { type: 'string' },
+          mappedValueID: { type: 'string' },
+          mappedValue: { type: 'string' }
+        },
+        additionalProperties: false
+      }
+    });
+
+    // Ensure that a circular $ref was **not** dereferenced.
+    expect(circularRefs).toHaveLength(23);
+    expect(schema.components?.schemas?.Customer?.properties?.customerNode).toStrictEqual({
+      "type": "array",
+      "items": {
+        "$ref": "#/components/schemas/CustomerNode",
+      },
+    });
+  });
+
+  it('should throw an error if "options.dereference.circular" is false', async () => {
+    const parser = new $RefParser();
+
+    try {
+      await parser.dereference(path.rel("test/specs/circular-extensive/schema.json"), {
+        dereference: { circular: false },
+      });
+
+      helper.shouldNotGetCalled();
+    } catch (err) {
+      expect(err).to.be.an.instanceOf(ReferenceError);
+      expect(err.message).to.contain("Circular $ref pointer found at ");
+      expect(err.message).to.contain("specs/circular-extensive/schema.json#/components/schemas/AssignmentExternalReference/properties/assignment/oneOf/0");
+
+      // $Refs.circular should be true
+      expect(parser.$refs.circular).to.equal(true);
+    }
+  });
+});