- [bug] Fixed bug whereby user error in related-object

assignment could cause recursion overflow if the
assignment triggered a backref of the same name
as a bi-directional attribute on the incorrect
class to the same target.  An informative
error is raised now.

diff --git a/CHANGES b/CHANGES
index da604a8f31eb9f61a37e7df9dd1b233e1977c58c..f725df5f2cd20f55ed1f2b6e20fec0e9155812ce 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -22,6 +22,13 @@ CHANGES
     is combined with uselist=False.  This is an
     exception raise in 0.8.
 
+  - [bug] Fixed bug whereby user error in related-object
+    assignment could cause recursion overflow if the
+    assignment triggered a backref of the same name
+    as a bi-directional attribute on the incorrect
+    class to the same target.  An informative
+    error is raised now.
+
 - sql
   - [bug] Fixed CTE bug whereby positional
     bound parameters present in the CTEs themselves

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 6be981ac2041be3d7c2f78de3253401465d00374..0fe77501ee6dcaca2aa3349bea8a4659598d75f0 100644 (file)
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -973,6 +973,14 @@ def backref_listeners(attribute, key, uselist):
 
     # use easily recognizable names for stack traces
 
+    parent_token = attribute.impl.parent_token
+
+    def _acceptable_key_err(child_state, initiator):
+        raise ValueError(
+            "Object %s not associated with attribute of "
+            "type %s" % (mapperutil.state_str(child_state),
+                    manager_of_class(initiator.class_)[initiator.key]))
+
     def emit_backref_from_scalar_set_event(state, child, oldchild, initiator):
         if oldchild is child:
             return child
@@ -991,35 +999,47 @@ def backref_listeners(attribute, key, uselist):
         if child is not None:
             child_state, child_dict = instance_state(child),\
                                         instance_dict(child)
-            child_state.manager[key].impl.append(
-                                            child_state,
-                                            child_dict,
-                                            state.obj(),
-                                            initiator,
-                                            passive=PASSIVE_NO_FETCH)
+            child_impl = child_state.manager[key].impl
+            if initiator.parent_token is not parent_token and \
+                initiator.parent_token is not child_impl.parent_token:
+                _acceptable_key_err(state, initiator)
+            child_impl.append(
+                                child_state,
+                                child_dict,
+                                state.obj(),
+                                initiator,
+                                passive=PASSIVE_NO_FETCH)
         return child
 
     def emit_backref_from_collection_append_event(state, child, initiator):
         child_state, child_dict = instance_state(child), \
                                     instance_dict(child)
-        child_state.manager[key].impl.append(
-                                            child_state,
-                                            child_dict,
-                                            state.obj(),
-                                            initiator,
-                                            passive=PASSIVE_NO_FETCH)
+        child_impl = child_state.manager[key].impl
+        if initiator.parent_token is not parent_token and \
+            initiator.parent_token is not child_impl.parent_token:
+            _acceptable_key_err(state, initiator)
+        child_impl.append(
+                                child_state,
+                                child_dict,
+                                state.obj(),
+                                initiator,
+                                passive=PASSIVE_NO_FETCH)
         return child
 
     def emit_backref_from_collection_remove_event(state, child, initiator):
         if child is not None:
             child_state, child_dict = instance_state(child),\
                                         instance_dict(child)
-            child_state.manager[key].impl.pop(
-                                            child_state,
-                                            child_dict,
-                                            state.obj(),
-                                            initiator,
-                                            passive=PASSIVE_NO_FETCH)
+            child_impl = child_state.manager[key].impl
+            # can't think of a path that would produce an initiator
+            # mismatch here, as it would require an existing collection
+            # mismatch.
+            child_impl.pop(
+                                child_state,
+                                child_dict,
+                                state.obj(),
+                                initiator,
+                                passive=PASSIVE_NO_FETCH)
 
     if uselist:
         event.listen(attribute, "append",

diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py
index 4702cc231b386fe01b5f8a3ef50b78385fc1aa94..472b5f8e10bc6132b03775fac8333a23864de126 100644 (file)
--- a/test/orm/test_attributes.py
+++ b/test/orm/test_attributes.py
@@ -1206,6 +1206,72 @@ class BackrefTest(fixtures.ORMTest):
         # and this condition changes.
         assert c1 in p1.children
 
+class CyclicBackrefAssertionTest(fixtures.TestBase):
+    """test that infinite recursion due to incorrect backref assignments
+    is blocked.
+
+    """
+    def test_scalar_set_type_assertion(self):
+        A, B, C = self._scalar_fixture()
+        c1 = C()
+        b1 = B()
+        assert_raises_message(
+            ValueError,
+            "Object <B at .*> not associated with attribute of type C.a",
+            setattr, c1, 'a', b1
+        )
+
+    def test_collection_append_type_assertion(self):
+        A, B, C = self._collection_fixture()
+        c1 = C()
+        b1 = B()
+        assert_raises_message(
+            ValueError,
+            "Object <B at .*> not associated with attribute of type C.a",
+            c1.a.append, b1
+        )
+
+    def _scalar_fixture(self):
+        class A(object):
+            pass
+        class B(object):
+            pass
+        class C(object):
+            pass
+        instrumentation.register_class(A)
+        instrumentation.register_class(B)
+        instrumentation.register_class(C)
+        attributes.register_attribute(C, 'a', backref='c', useobject=True)
+        attributes.register_attribute(C, 'b', backref='c', useobject=True)
+
+        attributes.register_attribute(A, 'c', backref='a', useobject=True,
+                        uselist=True)
+        attributes.register_attribute(B, 'c', backref='b', useobject=True,
+                        uselist=True)
+
+        return A, B, C
+
+    def _collection_fixture(self):
+        class A(object):
+            pass
+        class B(object):
+            pass
+        class C(object):
+            pass
+        instrumentation.register_class(A)
+        instrumentation.register_class(B)
+        instrumentation.register_class(C)
+
+        attributes.register_attribute(C, 'a', backref='c', useobject=True,
+                                                uselist=True)
+        attributes.register_attribute(C, 'b', backref='c', useobject=True,
+                                                uselist=True)
+
+        attributes.register_attribute(A, 'c', backref='a', useobject=True)
+        attributes.register_attribute(B, 'c', backref='b', useobject=True)
+
+        return A, B, C
+
 class PendingBackrefTest(fixtures.ORMTest):
     def setup(self):
         global Post, Blog, called, lazy_load