Merge pull request 'schemagen: reject self-referential array $ref cycles instead of crashing' (#49) from weaselbot/weaseljson:weaselbot/issue-33 into main

Reviewed-on: weaselab/weaseljson#49

contrib/schemagen/test_schemagen.py

@@ -274,6 +274,85 @@ class SchemagenAdditionalPropertiesTest(unittest.TestCase):
         self.assertEqual(rc, 0, msg=stderr)
 
 
+class SchemagenCyclicArrayTest(unittest.TestCase):
+    """Regression tests for issue #33: cyclic array $ref targets."""
+
+    def run_schemagen(self, schema, args=None):
+        """Run schemagen on a schema dict. Returns (returncode, stdout, stderr)."""
+        with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as fp:
+            json.dump(schema, fp)
+            schema_path = fp.name
+        try:
+            cmd = [sys.executable, SCRIPT, schema_path]
+            if args:
+                cmd.extend(args)
+            result = subprocess.run(cmd, capture_output=True, text=True, check=False)
+            return result.returncode, result.stdout, result.stderr
+        finally:
+            os.unlink(schema_path)
+
+    def test_direct_self_referential_array_rejected(self):
+        schema = {
+            "type": "array",
+            "items": {"$ref": "#/$defs/Node"},
+            "$defs": {
+                "Node": {
+                    "type": "array",
+                    "items": {"$ref": "#/$defs/Node"},
+                }
+            },
+        }
+        rc, stdout, stderr = self.run_schemagen(schema)
+        self.assertNotEqual(rc, 0)
+        self.assertIn("recursive array type is not supported", stderr)
+        self.assertIn("Node", stderr)
+
+    def test_object_field_to_self_referential_array_rejected(self):
+        schema = {
+            "type": "object",
+            "properties": {"items": {"$ref": "#/$defs/Items"}},
+            "$defs": {
+                "Items": {
+                    "type": "array",
+                    "items": {"$ref": "#/$defs/Items"},
+                }
+            },
+        }
+        rc, stdout, stderr = self.run_schemagen(schema)
+        self.assertNotEqual(rc, 0)
+        self.assertIn("recursive array type is not supported", stderr)
+        self.assertIn("Items", stderr)
+
+    def test_chain_of_array_refs_rejected(self):
+        schema = {
+            "type": "object",
+            "properties": {"x": {"$ref": "#/$defs/A"}},
+            "$defs": {
+                "A": {"type": "array", "items": {"$ref": "#/$defs/B"}},
+                "B": {"type": "array", "items": {"$ref": "#/$defs/A"}},
+            },
+        }
+        rc, stdout, stderr = self.run_schemagen(schema)
+        self.assertNotEqual(rc, 0)
+        self.assertIn("recursive array type is not supported", stderr)
+
+    def test_non_recursive_array_refs_still_allowed(self):
+        schema = {
+            "type": "object",
+            "properties": {"roles": {"$ref": "#/$defs/Roles"}},
+            "$defs": {
+                "Role": {"enum": ["admin", "user"]},
+                "Roles": {
+                    "type": "array",
+                    "items": {"$ref": "#/$defs/Role"},
+                },
+            },
+        }
+        rc, stdout, stderr = self.run_schemagen(schema)
+        self.assertEqual(rc, 0, msg=stderr)
+        self.assertIn("std::optional<std::vector<Role>> roles;", stdout)
+
+
 class SchemagenIntegerBoundaryTest(unittest.TestCase):
     """Regression tests for issue #19: integer slot parsing near int64 boundaries."""
 

contrib/schemagen/weaseljson_schemagen.py

@@ -272,6 +272,24 @@ class Builder:
         """Names generated internally that must not collide with user types."""
         return name in ("Root", "RootScalar")
 
+    @staticmethod
+    def _array_reaches(target, ty):
+        """Return True if `target` can be reached from `ty` by following
+        TArr element types. This detects self-referential array cycles that
+        cannot be expressed as C++ structs."""
+        seen = set()
+        stack = [ty]
+        while stack:
+            cur = stack.pop()
+            if cur is target:
+                return True
+            if id(cur) in seen:
+                continue
+            seen.add(id(cur))
+            if isinstance(cur, TArr):
+                stack.append(cur.elem)
+        return False
+
     def ref_name(self, ref):
         if not ref.startswith("#/"):
             raise GenError(f"only local $ref supported, got: {ref}")
@@ -359,6 +377,12 @@ class Builder:
             )
             t.elem = elem
             t.elem_nullable = elem_nullable
+            # Self-referential array cycles (directly or through a chain of
+            # array definitions) cannot be represented as a C++ value type.
+            if self._array_reaches(t, elem):
+                raise GenError(
+                    "recursive array type is not supported: " f"{defname or hint!r}"
+                )
             return result
 
         scalar = {