From 0b338e5cac5ca469348ab989f26ee75db5228060 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 12 Aug 2012 20:41:17 -0400 Subject: [PATCH] - [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. --- CHANGES | 7 ++++ lib/sqlalchemy/orm/attributes.py | 56 ++++++++++++++++++--------- test/orm/test_attributes.py | 66 ++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index da604a8f31..f725df5f2c 100644 --- 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 6be981ac20..0fe77501ee 100644 --- 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 4702cc231b..472b5f8e10 100644 --- 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 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 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 -- 2.47.2