Fix StackOverflowError on schema cycles that carry no $ref

core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java

@@ -332,8 +332,21 @@ protected DeferredChanged<ChangedSchema> 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 {
+    }
+
+    // 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);
     }
   }
 

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<String> leftKeys = new HashSet<>();
   HashSet<String> rightKeys = new HashSet<>();
+  HashSet<SchemaPair> schemaPath = new HashSet<>();
 
   public HashSet<String> 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);
+    }
+  }
 }

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;
@@ -10,6 +11,9 @@ 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";
+  private final String OPENAPI_DOC5 = "recursive_allof_model_2.yaml";
+  private final String OPENAPI_DOC6 = "recursive_allof_model_3.yaml";
 
   @Test
   public void testDiffSame() {
@@ -25,4 +29,19 @@ public void testDiffDifferentCyclic() {
   public void testDiffDifferent() {
     assertOpenApiBackwardIncompatible(OPENAPI_DOC1, OPENAPI_DOC2);
   }
+
+  @Test
+  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);
+  }
 }

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

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

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