Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#12049 closed (fixed)

New LazyObject-wrapped User breaks queries in template tags

Reported by: Christian Hammond Owned by: nobody
Component: contrib.auth Version: dev
Severity: Keywords:
Cc: chipx86@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I spent a couple days tracking this down since the last fix (ticket #12037) for the new lazy-wrapped User. Basically, if you have a template tag that grabs the User from the context and performs a complex query with two or more Q()'s chained together, it will fail. The failure case on Python 2.4 is fatal, while on 2.5/2.6 it seems to not fail but does log internally caught exception spew.

Basically, when a deepcopy() is performed on the node list, it sees a function() for the wrapped User and just fails.

Imagine you have a template tag that takes in the context and starts to build a query, like so:

query = Q(user=context['user']) & Q(someflag=True)

The process of using a Q() on a wrapped User and chaining with another Q() will break. On Python 2.4, we see:

  File "/usr/lib/python2.4/site-packages/django/db/models/query_utils.py", line 162, in __and__
    return self._combine(other, self.AND)
  File "/usr/lib/python2.4/site-packages/django/db/models/query_utils.py", line 154, in _combine
    obj = deepcopy(self)
  File "/usr/lib/python2.4/copy.py", line 185, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python2.4/site-packages/django/utils/tree.py", line 61, in __deepcopy__
    obj.children = deepcopy(self.children, memodict)
  File "/usr/lib/python2.4/copy.py", line 174, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python2.4/copy.py", line 241, in _deepcopy_list
    y.append(deepcopy(a, memo))
  File "/usr/lib/python2.4/copy.py", line 174, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python2.4/copy.py", line 248, in _deepcopy_tuple
    y.append(deepcopy(a, memo))
  File "/usr/lib/python2.4/copy.py", line 204, in deepcopy
    y = _reconstruct(x, rv, 1, memo)
  File "/usr/lib/python2.4/copy.py", line 351, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/lib/python2.4/copy.py", line 174, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python2.4/copy.py", line 268, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python2.4/copy.py", line 204, in deepcopy
    y = _reconstruct(x, rv, 1, memo)
  File "/usr/lib/python2.4/copy.py", line 336, in _reconstruct
    y = callable(*args)
  File "/usr/lib/python2.4/copy_reg.py", line 92, in __newobj__
    return cls.__new__(cls, *args)
TypeError: function() takes at least 2 arguments (0 given)

On Python 2.5 and 2.6, the call succeeds (or at least doesn't throw a fatal exception, but I don't know if there are internal side effects here), but does log the following:

Exception RuntimeError: 'maximum recursion depth exceeded while calling a Python object' in <type 'exceptions.AttributeError'> ignored

I'm attaching a diff that simply amends the test_user_attrs testcase to simulate this. After the other checks, it pulls out the User and then builds a dummy query.

Attachments (2)

user-query-test.diff (1.5 KB ) - added by Christian Hammond 14 years ago.
Test case for building a query with a wrapped User.
lazyobject-deepcopy.diff (2.2 KB ) - added by Christian Hammond 14 years ago.
Implements deepcopy for LazyObject

Download all attachments as: .zip

Change History (8)

by Christian Hammond, 14 years ago

Attachment: user-query-test.diff added

Test case for building a query with a wrapped User.

comment:1 by Christian Hammond, 14 years ago

Cc: chipx86@… added

comment:2 by Luke Plant, 14 years ago

Thanks for the test, that's really useful. Hopefully I look at fixing this on Monday. If you want to do it yourself, I imagine the fix will be to implement __deepcopy__ on SimpleLazyObject. I think the implementation is straightforward - if self._wrapped is None, then you just assign the same _setupfunc function to the copy (no need to make a copy of the function). If self._wrapped is not None, you copy the wrapped object itself.

In this context, I don't think we need a deep copy of the wrapped object, because it is just going to be thrown away and not modified, but to obey the __deepcopy__ contract, you might want to make it a deep copy. Alternatively, you could make a ContextLazyObject which subclasses SimpleLazyObject, and has a __deepcopy__ method that does shallow copies, which should be safe if ContextLazyObject is only used for this purpose. Perhaps. I haven't thought about it properly.

by Christian Hammond, 14 years ago

Attachment: lazyobject-deepcopy.diff added

Implements deepcopy for LazyObject

comment:3 by Christian Hammond, 14 years ago

The attached patch implements __deepcopy__ for LazyObject. I like the idea of providing the implementation there rather than in a subclass as it seems that this functionality should exist higher up, but if you think of a case where we really should limit this to a subclass, we can change that.

I kept the testcase in the same place even though really the fix isn't part of the auth framework. I couldn't find anything that, as far as I could tell, tested LazyObject specifically, and since the User wrapper is a real-world case of breakage (which could break again if User was wrapped in something else), I thought it'd be nice to continue testing this case. However, I'll leave this up to you to decide.

Ran the unit tests with Python 2.4, 2.5 and 2.6. All tests succeeded.

comment:4 by Luke Plant, 14 years ago

Great work, cheers! Sorry for the lost hours debugging, it's one of those use cases which is just really difficult to anticipate. I'll commit the patch soon.

comment:5 by Luke Plant, 14 years ago

Resolution: fixed
Status: newclosed

(In [11634]) Fixed #12049 - LazyObject-wrapped User breaks queries in template tags

Thanks to chipx86 for the report and patch.

comment:6 by Luke Plant, 14 years ago

(In [11635]) [1.1.X] Fixed #12049 - LazyObject-wrapped User breaks queries in template tags

Thanks to chipx86 for the report and patch.

Backport of r11634 from trunk

Note: See TracTickets for help on using tickets.
Back to Top