Skip to content

Fix StackOverflowError on schema cycles that carry no $ref - #928

Open
andrew-from-toronto wants to merge 2 commits into
OpenAPITools:masterfrom
andrew-from-toronto:fix/stackoverflow-allof-wrapped-ref-cycle
Open

Fix StackOverflowError on schema cycles that carry no $ref#928
andrew-from-toronto wants to merge 2 commits into
OpenAPITools:masterfrom
andrew-from-toronto:fix/stackoverflow-allof-wrapped-ref-cycle

Conversation

@andrew-from-toronto

@andrew-from-toronto andrew-from-toronto commented Sep 11, 2026

Copy link
Copy Markdown

Comparing a spec against itself overflows the stack when schemas form a reference cycle that no $ref survives.

resolveComposedSchema inlines an allOf target's properties into the wrapping schema and then clears the allOf. Two wrappers in a cycle therefore end up holding each other's properties with neither a $ref nor an allOf left on either one, and computeDeferredDiff only consults its recursion guard when both sides carry a bare $ref, so nothing stops the descent.

Guarding on the identity of the schema pair being compared covers that case, and also the simpler ones where a cycle passes through an allOf-wrapped reference on one or both sides. The guard is scoped to the current path, so sibling occurrences of the same schema are still diffed.

Changes:

  • track the (left, right) schema pair in RecursiveSchemaSet, entering before computeDiffForReal and leaving after it
  • cut the recursion when a pair is re-entered
  • add regression tests and fixtures for a wrapped cycle, a changed schema inside a wrapped cycle, and a mixed wrapped/bare cycle

./mvnw clean verify passes (core 273 tests, maven 13 tests, 0 failures). Also checked against a ~540KB generated spec that reproduced the original overflow: it now compares clean, and a type change made deep inside the cycle is still reported.

Claude assisted with this change.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java Outdated
Comment thread core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java Outdated
@andrew-from-toronto andrew-from-toronto changed the title Fix StackOverflowError on reference cycles wrapped in allOf Fix StackOverflowError on schema cycles that carry no $ref Sep 11, 2026
@andrew-from-toronto

Copy link
Copy Markdown
Author

Both review points were valid, and chasing the second one turned up a deeper cause. I have reworked the fix rather than patching the original approach.

On the false negative: confirmed. refSet.put(...) seeded the same set resolveComposedSchema uses as its visited list, so a wrapper's first occurrence was never expanded and its contents went undiffed. I reproduced it with a fixture that changes a property type inside the cycle; it was reported as unchanged.

On the mixed wrapper/bare pair: also confirmed. Reverting just the reference normalization makes that fixture overflow. It had passed earlier only because the first issue was masking it, gutting the wrapper and ending the recursion by accident.

What the first approach missed entirely: resolveComposedSchema inlines the target's properties into the wrapper and then clears the allOf. Once two wrappers in a cycle have been expanded, they hold each other's properties with neither a $ref nor an allOf remaining, so no reference-keyed guard can see them. The original patch only survived my real-world spec because it never expanded wrappers at all, which is precisely the bug you found.

The recursion is now guarded on the identity of the compared schema pair, scoped to the current path. That covers the flattened case and makes the wrapped and mixed cases fall out for free, so getWrappedSchemaRef is gone and the diff is smaller than before.

Verified: ./mvnw clean verify green (core 273, maven 13). Disabling the guard makes all three new fixtures overflow. On the real spec that prompted this, identical inputs now compare clean, and a type change deep inside the cycle is still reported, so the guard is not silently suppressing changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant