Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#21733 closed Bug (invalid)

@python_2_unicode_compatible causes infinite recursion when module imported more than once

Reported by: ntucker Owned by: nobody
Component: Utilities Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Sometimes a module that uses @python_2_unicode_compatible is imported multiple times. Possibly because a different pathname is used to import it. When this occurs, it makes unicode be the str function that was originally assigned, which calls unicode. So anytime unicode is called on the object, there is infinite recursion.

A simple check whether the str function has already been assigned will prevent setting unicode to the provided recursive function. This will also guard against other cases of multiple @python_2_unicode_compatible calls.

class _STR(object):
    def __call__(self, obj):
        return self.__unicode__().encode('utf-8')
_str = _STR()
def python_2_unicode_compatible(klass):
    """
A decorator that defines __unicode__ and __str__ methods under Python 2.
Under Python 3 it does nothing.

To support Python 2 and 3 with a single code base, define a __str__ method
returning text and apply this decorator to the class.
"""
    if six.PY2:
        if '__str__' not in klass.__dict__:
            raise ValueError("@python_2_unicode_compatible cannot be applied "
                             "to %s because it doesn't define __str__()." %
                             klass.__name__)
        # this was already applied once
        if klass.__str__ == _str:
            return klass
        klass.__unicode__ = klass.__str__
        klass.__str__ = _str
    return klass

I will provide patch if this is accepted. Above is example code.

Attachments (2)

recursionpatch.diff (2.5 KB ) - added by ntucker 11 years ago.
Patch
21733.patch (2.4 KB ) - added by Aymeric Augustin 11 years ago.

Download all attachments as: .zip

Change History (10)

by ntucker, 11 years ago

Attachment: recursionpatch.diff added

Patch

comment:1 by ntucker, 11 years ago

Has patch: set

comment:3 by Aymeric Augustin, 11 years ago

Easy pickings: unset
Keywords: python2.7.3 removed

tl;dr

Don't import modules twice.

Django doesn't require putting a directory and a subdirectory on PYTHONPATH since 1.4 and will most likely actively prevent it (i.e. refuse to start) in future versions.


explanation

I assume the problem only happens on Django model classes. The root cause isn't obvious and doesn't lie in python_2_unicode_compatible.

The second import should create a new class and apply the proper transformations to its methods. Unfortunately, the devious ModelBase.__new__ metaclass gets in the way and returns the existing class, to which the decorator has already been applied. So it gets applied a second time and that breaks.

As part of the app-loading refactor, I plan to forbid importing models twice to remove that, err, interesting behavior. This is tracked in #21711 and will make it impossible to trigger this issue.


That said, mistakes such as applying the decorator to a subclass that doesn't override __str__ also trigger an infinite recursion. I'd like to raise a specific exception when the decorator is applied twice to the same methods (ie. twice to the same class, or to a class and a subclass when no subclass has overridden __str__ in the mean time).

The appropriate way to achive this is to set an attribute on the __unicode__ method when it has been created by the decorator, and to test for that attribute and raise an exception if it's set.

comment:4 by ntucker, 11 years ago

Hmm, I'm getting multiple imports because I do from .models import Whatever in the application, then at other places from application.models import whatever.

I also put my models in their own modules, and package them up so init.py has from .whatever import *

I don't see any way of avoiding these different import styles

As far as setting an attribute on str; I tried this and it would not allow it saying

AttributeError: 'instancemethod' object has no attribute '__secrettest'

when I try to

setattr(klass.__str__, '__secrettest', True)

comment:5 by Aymeric Augustin, 11 years ago

Regardless of your import style, double imports can happen only if you have a directory and a subdirectory on PYTHONPATH.

Read this explanation by a Python core dev if you want to learn more on this topic: http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html#the-double-import-trap

comment:6 by Aymeric Augustin, 11 years ago

Resolution: invalid
Status: newclosed

I had written a patch, but then I realized that the current code correctly handles the subclass case.

The patch is only useful when metaclass magic messes things up. I'm attaching it for the record, but I now believe that #21711 is the only action to take in response to this bug.

by Aymeric Augustin, 11 years ago

Attachment: 21733.patch added

comment:7 by ntucker, 11 years ago

Thanks for the explanation. For some reason all the other places I found did not explain module importing properly. :)

comment:8 by Daniel Hawkins, 10 years ago

Thanks for tracking down the cause of this! I wanted to stop the double imports from happening and be able to use relative imports where possible, so I did some more digging and found this SO post:
http://stackoverflow.com/questions/23527435/python-path-not-consistent-in-django-double-module-imports

I still had an __init__.py in my project root directory -- deleting that file stopped the double imports and thus stopped the infinite recursion for me.

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