fix(mergeAllOf): prevent infinite loop on mutually recursive allOf schemas - #39
Open
vwong wants to merge 2 commits into
Open
fix(mergeAllOf): prevent infinite loop on mutually recursive allOf schemas#39vwong wants to merge 2 commits into
vwong wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a schema contains mutually recursive
allOfreferences (e.g.A = { allOf: [{ $ref: '#/B' }] },B = { allOf: [{ $ref: '#/A' }] }),mergeAllOfenters an infinite loop and hangs the process.The root cause is in the
do-whileloop inmergeAllOf:resolveAllOf(from@stoplight/json-schema-merge-allof, called withdeep: false) returns a new object on every invocation. Becauseseenis aWeakMapkeyed by object identity, the cache never hits for these new intermediate objects. The loop runs forever, consuming 100% CPU without ever overflowing the call stack.This surfaces as an
InternalError: too much recursion(Firefox) orRangeError: Maximum call stack size exceeded(Chrome) in consumers like@stoplight/elementswhen rendering OpenAPI specs that contain circular schema references.Fix
Track which
$refs have been resolved across do-while iterations using aSet<string>created once permergeAllOfcall. Each iteration records the$refs it resolves; subsequent iterations check against that accumulated set. If a$reffrom a prior iteration appears again, the chain is circular and we throwResolvingError— which the walker already catches and handles gracefully by falling back to the raw unmerged fragment.Key properties of this approach:
$refappearing multiple times within a singleallOf(resolved in the same iteration) is allowed — only cross-iteration repeats are blocked.A → allOf → B → allOf → C → (no allOf)resolves#/Bthen#/Cin successive iterations with no repeated refs.storeWeakMap (which trackedschemaRefsper(resolveRef, fragment)pair) in favour of the per-call set.Test
src/__tests__/mergeAllOf.spec.tscallsmergeAllOfdirectly with a mutually recursiveA ↔ Bschema and assertstoThrow(ResolvingError). Without the fix the test hangs; with it the test completes in ~3 ms.