schemagen: a cycle of bare $ref aliases crashes with an unhandled RecursionError instead of a GenError #74

Open
opened 2026-11-07 12:28:32 +00:00 by weaselbot · 0 comments
Member

Summary

contrib/schemagen/weaseljson_schemagen.py dies with an unhandled Python RecursionError (raw traceback, no error: diagnostic, exit code 1) when the schema contains a cycle of $defs entries whose bodies are nothing but $ref (an "alias cycle").

Cycles that pass through an object are broken by break_cycles (std::unique_ptr), and cycles that pass through an array are rejected with a clean GenError (fixed in #33, covered by test_direct_self_referential_array_rejected). Cycles made only of $ref-bodied definitions are neither detected nor rejected — build_def()/build_type() recurse until Python's recursion limit is hit.

Reproduction

schema.json:

{
  "$ref": "#/$defs/A",
  "$defs": {
    "A": { "$ref": "#/$defs/B" },
    "B": { "$ref": "#/$defs/A" }
  }
}
$ python3 contrib/schemagen/weaseljson_schemagen.py schema.json -o gen.h --namespace ns
Traceback (most recent call last):
  ...
  File ".../contrib/schemagen/weaseljson_schemagen.py", line 334, in build_type
    return self.build_def(self.ref_name(node["$ref"]))
  ...
  File ".../contrib/schemagen/weaseljson_schemagen.py", line 308, in build_def
    return self.build_type(node, defname, defname=defname)
  ...(the two frames above alternate until the limit is hit)...
RecursionError: maximum recursion depth exceeded
$ echo $?
1

Note that nothing is printed on stdout/stderr except the Python internals: the message does not name the definitions that form the cycle, and no other diagnostic is produced. The self-alias form ("$defs": {"A": {"$ref": "#/$defs/A"}}) and longer alias cycles (A -> B -> C -> A) behave the same. A plain (acyclic) alias chain, e.g. A: {"$ref": "#/$defs/B"}, B: {"$ref": "#/$defs/C"}, C: {object}, works fine, so only the cycle detection is missing.

Root cause

Builder.build_type() follows a $ref before the definition is registered as "in progress":

301    def build_def(self, defname):
302        """Build (or fetch cached) type for a named $defs entry."""
303        if defname in self._building:
304            return self._building[defname]
305        if defname not in self.defs:
306            raise GenError(f"$ref to unknown def: {defname}")
307        node = self.defs[defname]
308        return self.build_type(node, defname, defname=defname)
...
333        if "$ref" in node:
334            return self.build_def(self.ref_name(node["$ref"]))
335
336        # Non-object $defs entries must reuse the same type for every $ref.
337        if defname is not None and defname in self._building:
338            return self._building[defname]

self._building[defname] is only written by the structural branches (lines 360 enum, 374 array, 397 scalar, 419 object), so a definition whose body is only a $ref is never recorded as being built, and build_def() re-enters it forever. Registering the definition (or a "visiting" marker for the cycle) before line 334, and raising GenError when a $ref is re-entered, would fix it.

Expected behavior

A clear, actionable diagnostic like the other unsupported constructs, for example:

error: recursive $ref chain is not supported: 'A' -> 'B' -> 'A'

exit code 1, no traceback. This is what the tool promises for anything it cannot map to C++ (weaseljson_schemagen.py:8-9: "Only the subset of JSON Schema that maps naturally onto C++ structs is supported. Anything else is rejected at generation time (no JsonValue fallback).") and what #33 established for the analogous array-cycle case.

Impact

Any schema that ends up with an alias cycle — e.g. two definitions left aliasing each other after a rename:

"$defs": { "Node": { "$ref": "#/$defs/NodeImpl" }, "NodeImpl": { "$ref": "#/$defs/Node" } }

makes schemagen fail with an opaque, thousands-of-lines traceback made of alternating internal frames (build_type / build_def), instead of telling the user which $refs form the cycle. Because the recursion limit can be hit at a different point depending on the schema, the visible top-level error can also be a RecursionError from an unrelated frame. The same holds for definitions (both are collected into self.defs), and very deep but acyclic alias chains hit the same limit.

Why this is not a duplicate

  • #33 (closed) was about cycles through array definitions and was fixed by raising GenError("recursive array type is not supported") in the array branch of build_type. That check is never reached for a $ref-only definition, because build_type returns at line 334 first; there is no regression test for alias cycles.
  • #61 (open) is about Emitter.emit()'s nullable-root-object rename leaving a stale TObj.name, and its crash is a KeyError raised from break_cycles; it does not involve _building registration of $ref-bodied definitions and needs a nullable root object.
  • #60/#59/#62/#68/#69 are identifier/name collisions, #64 is std::vector<bool>, #66 is U+0000 in literals, #70 is dropped validation keywords, #73 is the integer parser's exponent arithmetic — none of them is about cycle detection in build_def.
## Summary `contrib/schemagen/weaseljson_schemagen.py` dies with an unhandled Python `RecursionError` (raw traceback, no `error:` diagnostic, exit code 1) when the schema contains a **cycle of `$defs` entries whose bodies are nothing but `$ref`** (an "alias cycle"). Cycles that pass through an *object* are broken by `break_cycles` (`std::unique_ptr`), and cycles that pass through an *array* are rejected with a clean `GenError` (fixed in #33, covered by `test_direct_self_referential_array_rejected`). Cycles made only of `$ref`-bodied definitions are neither detected nor rejected — `build_def()`/`build_type()` recurse until Python's recursion limit is hit. ## Reproduction `schema.json`: ```json { "$ref": "#/$defs/A", "$defs": { "A": { "$ref": "#/$defs/B" }, "B": { "$ref": "#/$defs/A" } } } ``` ```sh $ python3 contrib/schemagen/weaseljson_schemagen.py schema.json -o gen.h --namespace ns Traceback (most recent call last): ... File ".../contrib/schemagen/weaseljson_schemagen.py", line 334, in build_type return self.build_def(self.ref_name(node["$ref"])) ... File ".../contrib/schemagen/weaseljson_schemagen.py", line 308, in build_def return self.build_type(node, defname, defname=defname) ...(the two frames above alternate until the limit is hit)... RecursionError: maximum recursion depth exceeded $ echo $? 1 ``` Note that nothing is printed on stdout/stderr except the Python internals: the message does not name the definitions that form the cycle, and no other diagnostic is produced. The self-alias form (`"$defs": {"A": {"$ref": "#/$defs/A"}}`) and longer alias cycles (A -> B -> C -> A) behave the same. A plain (acyclic) alias chain, e.g. `A: {"$ref": "#/$defs/B"}`, `B: {"$ref": "#/$defs/C"}`, `C: {object}`, works fine, so only the cycle detection is missing. ## Root cause `Builder.build_type()` follows a `$ref` **before** the definition is registered as "in progress": ```python 301 def build_def(self, defname): 302 """Build (or fetch cached) type for a named $defs entry.""" 303 if defname in self._building: 304 return self._building[defname] 305 if defname not in self.defs: 306 raise GenError(f"$ref to unknown def: {defname}") 307 node = self.defs[defname] 308 return self.build_type(node, defname, defname=defname) ... 333 if "$ref" in node: 334 return self.build_def(self.ref_name(node["$ref"])) 335 336 # Non-object $defs entries must reuse the same type for every $ref. 337 if defname is not None and defname in self._building: 338 return self._building[defname] ``` `self._building[defname]` is only written by the *structural* branches (lines 360 enum, 374 array, 397 scalar, 419 object), so a definition whose body is only a `$ref` is never recorded as being built, and `build_def()` re-enters it forever. Registering the definition (or a "visiting" marker for the cycle) before line 334, and raising `GenError` when a `$ref` is re-entered, would fix it. ## Expected behavior A clear, actionable diagnostic like the other unsupported constructs, for example: ``` error: recursive $ref chain is not supported: 'A' -> 'B' -> 'A' ``` exit code 1, no traceback. This is what the tool promises for anything it cannot map to C++ (`weaseljson_schemagen.py:8-9`: *"Only the subset of JSON Schema that maps naturally onto C++ structs is supported. Anything else is rejected at generation time (no JsonValue fallback)."*) and what #33 established for the analogous array-cycle case. ## Impact Any schema that ends up with an alias cycle — e.g. two definitions left aliasing each other after a rename: ```json "$defs": { "Node": { "$ref": "#/$defs/NodeImpl" }, "NodeImpl": { "$ref": "#/$defs/Node" } } ``` makes schemagen fail with an opaque, thousands-of-lines traceback made of alternating internal frames (`build_type` / `build_def`), instead of telling the user which `$ref`s form the cycle. Because the recursion limit can be hit at a different point depending on the schema, the visible top-level error can also be a `RecursionError` from an unrelated frame. The same holds for `definitions` (both are collected into `self.defs`), and very deep but acyclic alias chains hit the same limit. ## Why this is not a duplicate * #33 (closed) was about cycles through **array** definitions and was fixed by raising `GenError("recursive array type is not supported")` in the array branch of `build_type`. That check is never reached for a `$ref`-only definition, because `build_type` returns at line 334 first; there is no regression test for alias cycles. * #61 (open) is about `Emitter.emit()`'s nullable-root-object rename leaving a stale `TObj.name`, and its crash is a `KeyError` raised from `break_cycles`; it does not involve `_building` registration of `$ref`-bodied definitions and needs a nullable root object. * #60/#59/#62/#68/#69 are identifier/name collisions, #64 is `std::vector<bool>`, #66 is U+0000 in literals, #70 is dropped validation keywords, #73 is the integer parser's exponent arithmetic — none of them is about cycle detection in `build_def`.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: weaselab/weaseljson#74