fix: prevent RecursionError when a dict contains itself (#592)

BaseDict._get_dict_or_value() recursively unwraps nested benedict
values with no cycle detection, so assigning a benedict into its own
nested descendant a second time recurses forever and crashes with
RecursionError. Track ids of mappings currently being unwrapped and
raise a clear ValueError as soon as a self-reference is detected,
instead of recursing into unrelated, equally unprotected traversal
code until the interpreter stack overflows.

benedict/dicts/base/base_dict.py

@@ -22,15 +22,41 @@ class BaseDict(dict[_K, _V]):
     _dict: dict[_K, _V] | None
     _frozen: bool
 
+    # ids of mappings currently being unwrapped by `_get_dict_or_value`,
+    # anywhere on the current call stack (not just in directly nested calls
+    # of this method: unwrapping a value can re-enter this method through
+    # __init__/__setitem__/_cast). Used to detect self-referential
+    # (cyclic) structures and abort unwrapping them instead of recursing
+    # forever. fix #592
+    _unwrapping_ids: set[int] = set()
+
     @classmethod
     def _get_dict_or_value(cls, value: Any) -> Any:
         value = value.dict() if isinstance(value, cls) else value
         if isinstance(value, MutableMapping):
-            for key in value.keys():
+            value_id = id(value)
-                key_val = value[key]
+            if value_id in cls._unwrapping_ids:
-                if isinstance(key_val, cls):
+                # value is already being unwrapped higher up the call stack,
-                    key_val = cls._get_dict_or_value(value[key])
+                # ie. it (directly or indirectly) contains itself. Raise
-                    value[key] = key_val
+                # immediately instead of unwrapping it again, to unwind the
+                # whole unwrap/cast chain before the still-cyclic structure
+                # can reach unrelated code with no cycle protection of its
+                # own (eg. keypath separator validation), which would also
+                # recurse infinitely but with an uncontrolled RecursionError.
+                raise ValueError(
+                    "Cannot assign a dict that contains itself "
+                    "(directly or indirectly): self-referential "
+                    "(cyclic) structures are not supported."
+                )
+            cls._unwrapping_ids.add(value_id)
+            try:
+                for key in value.keys():
+                    key_val = value[key]
+                    if isinstance(key_val, cls):
+                        key_val = cls._get_dict_or_value(value[key])
+                        value[key] = key_val
+            finally:
+                cls._unwrapping_ids.discard(value_id)
         return value
 
     def __new__(cls, *args: Any, **kwargs: Any) -> Self:

tests/github/test_issue_0592.py

@@ -0,0 +1,52 @@
+import unittest
+
+from benedict import benedict
+
+
+class github_issue_0592_test_case(unittest.TestCase):
+    """
+    This class describes a github issue 0592 test case.
+    https://github.com/fabiocaccamo/python-benedict/issues/592
+
+    To run this specific test:
+    - Run python -m unittest tests.github.test_issue_0592
+    """
+
+    def test_assigning_dict_to_its_own_nested_descendant_does_not_raise(self) -> None:
+        # this specific case (assigning the dict into an empty nested
+        # descendant) is not self-referential yet at assignment time,
+        # so it must keep working exactly as before this fix.
+        d = benedict(keyattr_enabled=True, keyattr_dynamic=True)
+        d.a.b.c = d
+        self.assertIs(dict.__getitem__(d, "a")["b"]["c"], d)
+
+    def test_assigning_dict_to_its_own_nested_descendant_twice_does_not_recurse(
+        self,
+    ) -> None:
+        # once `d` contains itself (from the first assignment), any further
+        # operation that needs to unwrap `d` again (eg. navigating into it
+        # to perform a second self-nesting assignment) must not raise
+        # RecursionError (it used to crash the Python/Jupyter process).
+        d = benedict(keyattr_enabled=True, keyattr_dynamic=True)
+        d.a.b.c = d
+        with self.assertRaises(ValueError):
+            d.a.b.d.e = d
+
+    def test_self_assignment_at_new_key_still_works(self) -> None:
+        # assigning a dict to itself under a brand new key must keep
+        # behaving exactly as before this fix (unaffected by it, verified
+        # against unpatched behavior: it does not raise, and the resulting
+        # structure is still a benedict, even though it stays cyclic).
+        d = benedict({"a": {"b": 1}})
+        d["self"] = d
+        self.assertIsInstance(d["self"], benedict)
+        self.assertEqual(d["self"]["a"], {"b": 1})
+
+    def test_normal_nested_dict_assignment_is_unaffected(self) -> None:
+        # sanity check: normal (non self-referential) nested benedict
+        # values must still be unwrapped/copied as before this fix.
+        inner = benedict({"x": 1})
+        outer = benedict()
+        outer["inner"] = inner
+        self.assertEqual(outer, {"inner": {"x": 1}})
+        self.assertIsNot(outer["inner"], inner)