From 3e54b910b5104cfe5a94312cdf1d9cfa0bf9ee6e Mon Sep 17 00:00:00 2001 From: Andrew Hunt Date: Fri, 11 Sep 2026 11:06:49 -0400 Subject: [PATCH 1/2] Fix StackOverflowError on reference cycles wrapped in allOf --- .../openapidiff/core/compare/SchemaDiff.java | 33 ++++++++++++- .../openapidiff/core/RecursiveSchemaTest.java | 6 +++ .../resources/recursive_allof_model_1.yaml | 49 +++++++++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 core/src/test/resources/recursive_allof_model_1.yaml diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java index fadae7a1..41d1c388 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java @@ -303,6 +303,24 @@ private static String getSchemaRef(Schema schema) { return ofNullable(schema).map(Schema::get$ref).orElse(null); } + private static String getWrappedSchemaRef(Schema schema) { + if (!(schema instanceof ComposedSchema)) { + return null; + } + ComposedSchema composedSchema = (ComposedSchema) schema; + if (composedSchema.getAnyOf() != null && !composedSchema.getAnyOf().isEmpty()) { + return null; + } + if (composedSchema.getOneOf() != null && !composedSchema.getOneOf().isEmpty()) { + return null; + } + List allOf = composedSchema.getAllOf(); + if (allOf == null || allOf.size() != 1) { + return null; + } + return allOf.get(0).get$ref(); + } + public DeferredChanged diff(Schema left, Schema right, DiffContext context) { return this.diff(new RecursiveSchemaSet(), left, right, context); } @@ -332,9 +350,20 @@ protected DeferredChanged computeDeferredDiff( CacheKey key = new CacheKey(getSchemaRef(left), getSchemaRef(right), context); if (key.getLeft() != null && key.getRight() != null) { return openApiDiff.getDeferredSchemaCache().getOrAddSchema(refSet, key, left, right); - } else { - return computeDiffForReal(refSet, left, right, context); } + + // An allOf-wrapped reference has no $ref of its own, so a cycle built entirely from them never + // reaches the guard above and was followed until the stack overflowed. + CacheKey wrappedKey = + new CacheKey(getWrappedSchemaRef(left), getWrappedSchemaRef(right), context); + if (wrappedKey.getLeft() != null && wrappedKey.getRight() != null) { + if (refSet.contains(wrappedKey)) { + return DeferredChanged.empty(); + } + refSet.put(wrappedKey); + } + + return computeDiffForReal(refSet, left, right, context); } public DeferredChanged computeDiffForReal( diff --git a/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java b/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java index 7f4114c6..b2911332 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java @@ -10,6 +10,7 @@ public class RecursiveSchemaTest { private final String OPENAPI_DOC1 = "recursive_model_1.yaml"; private final String OPENAPI_DOC2 = "recursive_model_2.yaml"; private final String OPENAPI_DOC3 = "recursive_model_3.yaml"; + private final String OPENAPI_DOC4 = "recursive_allof_model_1.yaml"; @Test public void testDiffSame() { @@ -25,4 +26,9 @@ public void testDiffDifferentCyclic() { public void testDiffDifferent() { assertOpenApiBackwardIncompatible(OPENAPI_DOC1, OPENAPI_DOC2); } + + @Test + public void testDiffSameWithAllOfWrappedCycle() { + assertOpenApiAreEquals(OPENAPI_DOC4, OPENAPI_DOC4); + } } diff --git a/core/src/test/resources/recursive_allof_model_1.yaml b/core/src/test/resources/recursive_allof_model_1.yaml new file mode 100644 index 00000000..336a8a3e --- /dev/null +++ b/core/src/test/resources/recursive_allof_model_1.yaml @@ -0,0 +1,49 @@ +openapi: 3.0.1 +info: + title: allOf-wrapped recursive test + version: '1.0' +servers: + - url: 'http://localhost:8000/' +paths: + /ping: + get: + operationId: ping + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Entry' +components: + schemas: + Entry: + type: object + properties: + message: + type: string + first: + title: First + allOf: + - $ref: '#/components/schemas/First' + description: Only reachable through an allOf wrapper + First: + type: object + properties: + name: + type: string + second: + title: Second + allOf: + - $ref: '#/components/schemas/Second' + description: Only reachable through an allOf wrapper + Second: + type: object + properties: + name: + type: string + first: + title: First + allOf: + - $ref: '#/components/schemas/First' + description: Closes the cycle without passing through Entry From c47767810d6b2af45f6cb3f929956e6414f1c70a Mon Sep 17 00:00:00 2001 From: Andrew Hunt Date: Fri, 11 Sep 2026 11:40:00 -0400 Subject: [PATCH 2/2] Guard schema recursion on instance identity --- .../openapidiff/core/compare/SchemaDiff.java | 42 +++++----------- .../model/deferred/RecursiveSchemaSet.java | 36 ++++++++++++++ .../openapidiff/core/RecursiveSchemaTest.java | 13 +++++ .../resources/recursive_allof_model_2.yaml | 49 +++++++++++++++++++ .../resources/recursive_allof_model_3.yaml | 46 +++++++++++++++++ 5 files changed, 157 insertions(+), 29 deletions(-) create mode 100644 core/src/test/resources/recursive_allof_model_2.yaml create mode 100644 core/src/test/resources/recursive_allof_model_3.yaml diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java index 41d1c388..5ffcf774 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java @@ -303,24 +303,6 @@ private static String getSchemaRef(Schema schema) { return ofNullable(schema).map(Schema::get$ref).orElse(null); } - private static String getWrappedSchemaRef(Schema schema) { - if (!(schema instanceof ComposedSchema)) { - return null; - } - ComposedSchema composedSchema = (ComposedSchema) schema; - if (composedSchema.getAnyOf() != null && !composedSchema.getAnyOf().isEmpty()) { - return null; - } - if (composedSchema.getOneOf() != null && !composedSchema.getOneOf().isEmpty()) { - return null; - } - List allOf = composedSchema.getAllOf(); - if (allOf == null || allOf.size() != 1) { - return null; - } - return allOf.get(0).get$ref(); - } - public DeferredChanged diff(Schema left, Schema right, DiffContext context) { return this.diff(new RecursiveSchemaSet(), left, right, context); } @@ -352,18 +334,20 @@ protected DeferredChanged computeDeferredDiff( return openApiDiff.getDeferredSchemaCache().getOrAddSchema(refSet, key, left, right); } - // An allOf-wrapped reference has no $ref of its own, so a cycle built entirely from them never - // reaches the guard above and was followed until the stack overflowed. - CacheKey wrappedKey = - new CacheKey(getWrappedSchemaRef(left), getWrappedSchemaRef(right), context); - if (wrappedKey.getLeft() != null && wrappedKey.getRight() != null) { - if (refSet.contains(wrappedKey)) { - return DeferredChanged.empty(); - } - refSet.put(wrappedKey); + // A schema with no $ref of its own never reaches the guard above, so a cycle among such schemas + // was followed until the stack overflowed. Guarding on identity rather than on a reference also + // covers schemas whose reference resolveComposedSchema has already cleared. + if (left == null || right == null) { + return computeDiffForReal(refSet, left, right, context); + } + if (!refSet.enter(left, right)) { + return DeferredChanged.empty(); + } + try { + return computeDiffForReal(refSet, left, right, context); + } finally { + refSet.leave(left, right); } - - return computeDiffForReal(refSet, left, right, context); } public DeferredChanged computeDiffForReal( diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java index 890d2e6c..3fce7e8a 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java @@ -1,11 +1,13 @@ package org.openapitools.openapidiff.core.model.deferred; +import io.swagger.v3.oas.models.media.Schema; import java.util.HashSet; import org.openapitools.openapidiff.core.compare.CacheKey; public class RecursiveSchemaSet { HashSet leftKeys = new HashSet<>(); HashSet rightKeys = new HashSet<>(); + HashSet schemaPath = new HashSet<>(); public HashSet getLeftKeys() { return leftKeys; @@ -23,4 +25,38 @@ public void put(CacheKey key) { leftKeys.add(key.getLeft()); rightKeys.add(key.getRight()); } + + // Resolving an allOf copies the target's properties into the wrapper and then clears the allOf, + // so a cycle between two such wrappers ends up with no $ref left on either one and is invisible + // to the key-based guards above. Identity is all that still distinguishes them. + public boolean enter(Schema left, Schema right) { + return schemaPath.add(new SchemaPair(left, right)); + } + + public void leave(Schema left, Schema right) { + schemaPath.remove(new SchemaPair(left, right)); + } + + private static final class SchemaPair { + private final Schema left; + private final Schema right; + + private SchemaPair(Schema left, Schema right) { + this.left = left; + this.right = right; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof SchemaPair)) return false; + SchemaPair other = (SchemaPair) o; + return left == other.left && right == other.right; + } + + @Override + public int hashCode() { + return 31 * System.identityHashCode(left) + System.identityHashCode(right); + } + } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java b/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java index b2911332..5fa09f36 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java @@ -1,6 +1,7 @@ package org.openapitools.openapidiff.core; import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiAreEquals; +import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardCompatible; import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; import org.junit.jupiter.api.Test; @@ -11,6 +12,8 @@ public class RecursiveSchemaTest { private final String OPENAPI_DOC2 = "recursive_model_2.yaml"; private final String OPENAPI_DOC3 = "recursive_model_3.yaml"; private final String OPENAPI_DOC4 = "recursive_allof_model_1.yaml"; + private final String OPENAPI_DOC5 = "recursive_allof_model_2.yaml"; + private final String OPENAPI_DOC6 = "recursive_allof_model_3.yaml"; @Test public void testDiffSame() { @@ -31,4 +34,14 @@ public void testDiffDifferent() { public void testDiffSameWithAllOfWrappedCycle() { assertOpenApiAreEquals(OPENAPI_DOC4, OPENAPI_DOC4); } + + @Test + public void testDiffChangedInsideAllOfWrappedCycle() { + assertOpenApiBackwardIncompatible(OPENAPI_DOC4, OPENAPI_DOC5); + } + + @Test + public void testDiffAllOfWrappedAgainstBareCycle() { + assertOpenApiBackwardCompatible(OPENAPI_DOC4, OPENAPI_DOC6, true); + } } diff --git a/core/src/test/resources/recursive_allof_model_2.yaml b/core/src/test/resources/recursive_allof_model_2.yaml new file mode 100644 index 00000000..8c2c6425 --- /dev/null +++ b/core/src/test/resources/recursive_allof_model_2.yaml @@ -0,0 +1,49 @@ +openapi: 3.0.1 +info: + title: allOf-wrapped recursive test + version: '1.0' +servers: + - url: 'http://localhost:8000/' +paths: + /ping: + get: + operationId: ping + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Entry' +components: + schemas: + Entry: + type: object + properties: + message: + type: string + first: + title: First + allOf: + - $ref: '#/components/schemas/First' + description: Only reachable through an allOf wrapper + First: + type: object + properties: + name: + type: integer + second: + title: Second + allOf: + - $ref: '#/components/schemas/Second' + description: Only reachable through an allOf wrapper + Second: + type: object + properties: + name: + type: string + first: + title: First + allOf: + - $ref: '#/components/schemas/First' + description: Closes the cycle without passing through Entry diff --git a/core/src/test/resources/recursive_allof_model_3.yaml b/core/src/test/resources/recursive_allof_model_3.yaml new file mode 100644 index 00000000..274a9229 --- /dev/null +++ b/core/src/test/resources/recursive_allof_model_3.yaml @@ -0,0 +1,46 @@ +openapi: 3.0.1 +info: + title: allOf-wrapped recursive test + version: '1.0' +servers: + - url: 'http://localhost:8000/' +paths: + /ping: + get: + operationId: ping + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Entry' +components: + schemas: + Entry: + type: object + properties: + message: + type: string + first: + $ref: '#/components/schemas/First' + First: + type: object + properties: + name: + type: string + second: + title: Second + allOf: + - $ref: '#/components/schemas/Second' + description: Only reachable through an allOf wrapper + Second: + type: object + properties: + name: + type: string + first: + title: First + allOf: + - $ref: '#/components/schemas/First' + description: Closes the cycle without passing through Entry