From e80d66d14fd14c757fab3d971fb3680820e72a30 Mon Sep 17 00:00:00 2001 From: Voon Wong Date: Thu, 27 Aug 2026 11:41:14 +1000 Subject: [PATCH 1/2] fix: circular reference consuming all available memory --- .../combiners/allof-oneof-circular.json | 42 ++++++++++ src/__tests__/__snapshots__/tree.spec.ts.snap | 84 +++++++++++++++++++ src/mergers/mergeAllOf.ts | 5 ++ 3 files changed, 131 insertions(+) create mode 100644 src/__tests__/__fixtures__/combiners/allof-oneof-circular.json diff --git a/src/__tests__/__fixtures__/combiners/allof-oneof-circular.json b/src/__tests__/__fixtures__/combiners/allof-oneof-circular.json new file mode 100644 index 0000000..6ffc759 --- /dev/null +++ b/src/__tests__/__fixtures__/combiners/allof-oneof-circular.json @@ -0,0 +1,42 @@ +{ + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": { + "value": { + "$ref": "#/definitions/Parent" + } + }, + "definitions": { + "Parent": { + "type": "object", + "oneOf": [ + { "$ref": "#/definitions/ChildA" }, + { "$ref": "#/definitions/ChildB" } + ] + }, + "ChildA": { + "type": "object", + "allOf": [ + { "$ref": "#/definitions/Parent" }, + { + "type": "object", + "properties": { + "a": { "type": "string" } + } + } + ] + }, + "ChildB": { + "type": "object", + "allOf": [ + { "$ref": "#/definitions/Parent" }, + { + "type": "object", + "properties": { + "b": { "type": "string" } + } + } + ] + } + } +} diff --git a/src/__tests__/__snapshots__/tree.spec.ts.snap b/src/__tests__/__snapshots__/tree.spec.ts.snap index 28d8a9e..69cfca7 100644 --- a/src/__tests__/__snapshots__/tree.spec.ts.snap +++ b/src/__tests__/__snapshots__/tree.spec.ts.snap @@ -1092,6 +1092,90 @@ exports[`SchemaTree output should generate valid tree for combiners/allOfs/with- " `; +exports[`SchemaTree output should generate valid tree for combiners/allof-oneof-circular.json 1`] = ` +"└─ # + ├─ types + │ └─ 0: object + ├─ primaryType: object + └─ children + └─ 0 + └─ #/properties/value + ├─ combiners + │ └─ 0: oneOf + └─ children + ├─ 0 + │ └─ #/properties/value/oneOf/0 + │ ├─ types + │ │ └─ 0: object + │ ├─ primaryType: object + │ ├─ combiners + │ │ └─ 0: oneOf + │ └─ children + │ ├─ 0 + │ │ └─ #/properties/value/oneOf/0/oneOf/0 + │ │ ├─ types + │ │ │ └─ 0: object + │ │ ├─ primaryType: object + │ │ ├─ combiners + │ │ │ └─ 0: oneOf + │ │ └─ children + │ │ ├─ 0 + │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/0 + │ │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0 + │ │ ├─ 1 + │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1 + │ │ │ ├─ types + │ │ │ │ └─ 0: object + │ │ │ ├─ primaryType: object + │ │ │ ├─ combiners + │ │ │ │ └─ 0: oneOf + │ │ │ └─ children + │ │ │ ├─ 0 + │ │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1/oneOf/0 + │ │ │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0 + │ │ │ ├─ 1 + │ │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1/oneOf/1 + │ │ │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0/oneOf/1 + │ │ │ └─ 2 + │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1/properties/b + │ │ │ ├─ types + │ │ │ │ └─ 0: string + │ │ │ └─ primaryType: string + │ │ └─ 2 + │ │ └─ #/properties/value/oneOf/0/oneOf/0/properties/a + │ │ ├─ types + │ │ │ └─ 0: string + │ │ └─ primaryType: string + │ ├─ 1 + │ │ └─ #/properties/value/oneOf/0/oneOf/1 + │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0/oneOf/1 + │ └─ 2 + │ └─ #/properties/value/oneOf/0/properties/a + │ ├─ types + │ │ └─ 0: string + │ └─ primaryType: string + └─ 1 + └─ #/properties/value/oneOf/1 + ├─ types + │ └─ 0: object + ├─ primaryType: object + ├─ combiners + │ └─ 0: oneOf + └─ children + ├─ 0 + │ └─ #/properties/value/oneOf/1/oneOf/0 + │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0 + ├─ 1 + │ └─ #/properties/value/oneOf/1/oneOf/1 + │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0/oneOf/1 + └─ 2 + └─ #/properties/value/oneOf/1/properties/b + ├─ types + │ └─ 0: string + └─ primaryType: string +" +`; + exports[`SchemaTree output should generate valid tree for combiners/oneof-with-array-type.json 1`] = ` "└─ # ├─ combiners diff --git a/src/mergers/mergeAllOf.ts b/src/mergers/mergeAllOf.ts index 24d89dd..23f741d 100644 --- a/src/mergers/mergeAllOf.ts +++ b/src/mergers/mergeAllOf.ts @@ -19,6 +19,11 @@ function _mergeAllOf( return cached; } + // Mark as in-flight before resolving so that any re-entrant call for the + // same fragment (via a child→parent→child allOf/oneOf cycle) returns the + // unmerged fragment rather than recursing infinitely. + seen.set(fragment, fragment); + const merged = resolveAllOf(fragment, { deep: false, resolvers: resolveAllOf.stoplightResolvers, From 76ac0b96cb44e4862b898858b1796712e106abbf Mon Sep 17 00:00:00 2001 From: Voon Wong Date: Thu, 27 Aug 2026 13:51:58 +1000 Subject: [PATCH 2/2] chore: 2nd attempt at fixing the problem --- .../combiners/allof-oneof-circular.json | 42 ---------- src/__tests__/__snapshots__/tree.spec.ts.snap | 84 ------------------- src/__tests__/mergeAllOf.spec.ts | 23 +++++ src/mergers/mergeAllOf.ts | 54 +++++------- 4 files changed, 44 insertions(+), 159 deletions(-) delete mode 100644 src/__tests__/__fixtures__/combiners/allof-oneof-circular.json create mode 100644 src/__tests__/mergeAllOf.spec.ts diff --git a/src/__tests__/__fixtures__/combiners/allof-oneof-circular.json b/src/__tests__/__fixtures__/combiners/allof-oneof-circular.json deleted file mode 100644 index 6ffc759..0000000 --- a/src/__tests__/__fixtures__/combiners/allof-oneof-circular.json +++ /dev/null @@ -1,42 +0,0 @@ -{ - "$schema": "http://json-schema.org/schema#", - "type": "object", - "properties": { - "value": { - "$ref": "#/definitions/Parent" - } - }, - "definitions": { - "Parent": { - "type": "object", - "oneOf": [ - { "$ref": "#/definitions/ChildA" }, - { "$ref": "#/definitions/ChildB" } - ] - }, - "ChildA": { - "type": "object", - "allOf": [ - { "$ref": "#/definitions/Parent" }, - { - "type": "object", - "properties": { - "a": { "type": "string" } - } - } - ] - }, - "ChildB": { - "type": "object", - "allOf": [ - { "$ref": "#/definitions/Parent" }, - { - "type": "object", - "properties": { - "b": { "type": "string" } - } - } - ] - } - } -} diff --git a/src/__tests__/__snapshots__/tree.spec.ts.snap b/src/__tests__/__snapshots__/tree.spec.ts.snap index 69cfca7..28d8a9e 100644 --- a/src/__tests__/__snapshots__/tree.spec.ts.snap +++ b/src/__tests__/__snapshots__/tree.spec.ts.snap @@ -1092,90 +1092,6 @@ exports[`SchemaTree output should generate valid tree for combiners/allOfs/with- " `; -exports[`SchemaTree output should generate valid tree for combiners/allof-oneof-circular.json 1`] = ` -"└─ # - ├─ types - │ └─ 0: object - ├─ primaryType: object - └─ children - └─ 0 - └─ #/properties/value - ├─ combiners - │ └─ 0: oneOf - └─ children - ├─ 0 - │ └─ #/properties/value/oneOf/0 - │ ├─ types - │ │ └─ 0: object - │ ├─ primaryType: object - │ ├─ combiners - │ │ └─ 0: oneOf - │ └─ children - │ ├─ 0 - │ │ └─ #/properties/value/oneOf/0/oneOf/0 - │ │ ├─ types - │ │ │ └─ 0: object - │ │ ├─ primaryType: object - │ │ ├─ combiners - │ │ │ └─ 0: oneOf - │ │ └─ children - │ │ ├─ 0 - │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/0 - │ │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0 - │ │ ├─ 1 - │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1 - │ │ │ ├─ types - │ │ │ │ └─ 0: object - │ │ │ ├─ primaryType: object - │ │ │ ├─ combiners - │ │ │ │ └─ 0: oneOf - │ │ │ └─ children - │ │ │ ├─ 0 - │ │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1/oneOf/0 - │ │ │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0 - │ │ │ ├─ 1 - │ │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1/oneOf/1 - │ │ │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0/oneOf/1 - │ │ │ └─ 2 - │ │ │ └─ #/properties/value/oneOf/0/oneOf/0/oneOf/1/properties/b - │ │ │ ├─ types - │ │ │ │ └─ 0: string - │ │ │ └─ primaryType: string - │ │ └─ 2 - │ │ └─ #/properties/value/oneOf/0/oneOf/0/properties/a - │ │ ├─ types - │ │ │ └─ 0: string - │ │ └─ primaryType: string - │ ├─ 1 - │ │ └─ #/properties/value/oneOf/0/oneOf/1 - │ │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0/oneOf/1 - │ └─ 2 - │ └─ #/properties/value/oneOf/0/properties/a - │ ├─ types - │ │ └─ 0: string - │ └─ primaryType: string - └─ 1 - └─ #/properties/value/oneOf/1 - ├─ types - │ └─ 0: object - ├─ primaryType: object - ├─ combiners - │ └─ 0: oneOf - └─ children - ├─ 0 - │ └─ #/properties/value/oneOf/1/oneOf/0 - │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0 - ├─ 1 - │ └─ #/properties/value/oneOf/1/oneOf/1 - │ └─ mirrors: #/properties/value/oneOf/0/oneOf/0/oneOf/1 - └─ 2 - └─ #/properties/value/oneOf/1/properties/b - ├─ types - │ └─ 0: string - └─ primaryType: string -" -`; - exports[`SchemaTree output should generate valid tree for combiners/oneof-with-array-type.json 1`] = ` "└─ # ├─ combiners diff --git a/src/__tests__/mergeAllOf.spec.ts b/src/__tests__/mergeAllOf.spec.ts new file mode 100644 index 0000000..1aae0fe --- /dev/null +++ b/src/__tests__/mergeAllOf.spec.ts @@ -0,0 +1,23 @@ +import { ResolvingError } from '../errors'; +import { mergeAllOf } from '../mergers/mergeAllOf'; +import type { SchemaFragment } from '../types'; +import type { WalkingOptions } from '../walker/types'; + +describe('mergeAllOf', () => { + // Regression: mutually recursive allOf (A -> B -> A) caused _mergeAllOf to + // call itself with the same resolved fragment before seen-cache was populated, + // producing a RangeError (call stack overflow) in the caller. + it('throws ResolvingError (not a stack overflow) for mutually recursive allOf schemas', () => { + const schemaA: SchemaFragment = { allOf: [{ $ref: '#/B' }] }; + const schemaB: SchemaFragment = { allOf: [{ $ref: '#/A' }] }; + const refs: Record = { '#/A': schemaA, '#/B': schemaB }; + + const walkingOptions: WalkingOptions = { + mergeAllOf: true, + resolveRef: (_path, $ref) => refs[$ref] ?? {}, + maxRefDepth: null, + }; + + expect(() => mergeAllOf({ allOf: [{ $ref: '#/A' }] }, [], walkingOptions, new WeakMap())).toThrow(ResolvingError); + }); +}); diff --git a/src/mergers/mergeAllOf.ts b/src/mergers/mergeAllOf.ts index 23f741d..89ac061 100644 --- a/src/mergers/mergeAllOf.ts +++ b/src/mergers/mergeAllOf.ts @@ -6,23 +6,21 @@ import type { WalkerRefResolver, WalkingOptions } from '../walker/types'; const resolveAllOf = require('@stoplight/json-schema-merge-allof'); -const store = new WeakMap>(); - function _mergeAllOf( fragment: SchemaFragment, path: string[], resolveRef: WalkerRefResolver | null, seen: WeakMap, + resolvedInPriorIterations: Set | null, ): SchemaFragment { const cached = seen.get(fragment); if (cached !== void 0) { return cached; } - // Mark as in-flight before resolving so that any re-entrant call for the - // same fragment (via a child→parent→child allOf/oneOf cycle) returns the - // unmerged fragment rather than recursing infinitely. - seen.set(fragment, fragment); + // Track $refs resolved in THIS iteration so we can add them to + // resolvedInPriorIterations after the call completes. + const refsThisIteration = resolvedInPriorIterations !== null ? new Set() : null; const merged = resolveAllOf(fragment, { deep: false, @@ -38,36 +36,26 @@ function _mergeAllOf( throw new ResolvingError('Circular reference detected'); } - const allRefs = store.get(resolveRef)!; - let schemaRefs = allRefs.get(fragment); - - if (schemaRefs === void 0) { - schemaRefs = [$ref]; - allRefs.set(fragment, schemaRefs); - } else if (schemaRefs.includes($ref)) { - const resolved = resolveRef(null, $ref); - return 'allOf' in resolved ? _mergeAllOf(resolved, path, resolveRef, seen) : resolved; - } else { - schemaRefs.push($ref); - } - - const resolved = resolveRef(null, $ref); - - if (Array.isArray(resolved.allOf)) { - for (const member of resolved.allOf) { - const index = schemaRefs.indexOf(member.$ref); - if (typeof member.$ref === 'string' && index !== -1 && index !== schemaRefs.lastIndexOf(member.$ref)) { - throw new ResolvingError('Circular reference detected'); - } - } + // A $ref seen in a prior do-while iteration means the chain is + // circular (e.g. A→B→A). Throw instead of looping forever. + if (resolvedInPriorIterations?.has($ref) === true) { + throw new ResolvingError('Circular reference detected'); } - return resolved; + refsThisIteration?.add($ref); + return resolveRef(null, $ref); }, } : null), }); + // Promote this iteration's refs so the next iteration can detect cycles. + if (resolvedInPriorIterations !== null && refsThisIteration !== null) { + for (const ref of refsThisIteration) { + resolvedInPriorIterations.add(ref); + } + } + seen.set(fragment, merged); return merged; } @@ -78,13 +66,13 @@ export function mergeAllOf( walkingOptions: WalkingOptions, seen: WeakMap, ) { - if (walkingOptions.resolveRef !== null && !store.has(walkingOptions.resolveRef)) { - store.set(walkingOptions.resolveRef, new WeakMap()); - } + // One set shared across all do-while iterations; grows monotonically so any + // $ref seen in iteration N will be detected as circular in iteration N+1. + const resolvedInPriorIterations = walkingOptions.resolveRef !== null ? new Set() : null; let merged = fragment; do { - merged = _mergeAllOf(merged, path, walkingOptions.resolveRef, seen); + merged = _mergeAllOf(merged, path, walkingOptions.resolveRef, seen, resolvedInPriorIterations); } while ('allOf' in merged); return merged;