Opened 2 months ago

Closed 2 months ago

#35735 closed Bug (fixed)

For python 3.9+ class property may not be accessible by Django's template system

Reported by: Fabian Braun Owned by: Fabian Braun
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: Fabian Braun Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Fabian Braun)

Before python 3.9 class properties were always available through the template system. If you had a class

class MyClass(list):
    in_template = True
    do_not_call_in_templates = True  # prevent instantiation

    @classmethod
    def render_all_objects(cls):
        ...

you could access the class property in the template through (if it was contained in the context) {{ MyClass.in_template }} or {{ MyClass. render_all_objects }}.

The template system first gets the class MyClass (and does not instantiate it) or gets it as a result of a callable get_my_class. Then it checks if the class is subscriptable (i.e. tries MyClass["in_template"]), will fail and then will get the in_template property.

As of python 3.9 some classes actually are subscriptable and trying to get the item will not fail: Typing shortcuts introduced syntax like list[int]. These hide class properties or methods from the template system.

Here's a test (that might go into tests/template_tests/syntax_tests/tests_basic.py) which passes on Python 3.9 and fails on Python 3.10+:

    @setup({"basic-syntax19b": "{{ klass.in_template }}"})
    def test_access_class_property(self):
        class MyClass(list):
            in_template = True
            do_not_call_in_templates = True  # prevent instantiation

        output = self.engine.render_to_string("basic-syntax19b", {"klass": MyClass})
        self.assertEqual(output, "True")

I'd be happy to propose a fix that will not call a classes' __class_getitem__ method.

Thanks to Ben Stähli and Serhii Tereshchenko for figuring out this issue.

References:

Change History (13)

comment:1 by Fabian Braun, 2 months ago

Description: modified (diff)
Owner: set to Fabian Braun
Status: newassigned

comment:2 by Fabian Braun, 2 months ago

Description: modified (diff)
Summary: For python 3.10+ class property may not be accessible by Django's template systemFor python 3.9+ class property may not be accessible by Django's template system
Type: UncategorizedBug

comment:3 by Fabian Braun, 2 months ago

Description: modified (diff)
Has patch: set

comment:4 by Fabian Braun, 2 months ago

Description: modified (diff)

comment:5 by Natalia Bidart, 2 months ago

Hi Fabian, thank you for taking the time to create this report!

(Before your last edit) I have tried to reproduce the issue described and I wasn't able to, I then analyzed your test and noticed that in the test (but not in the ticket description) MyClass is a child of list. For that case, and for children of dict or set, the test indeed fail; but for children of object, str, int, the test do not fail.

So on one hand, the issues seems less generic than presented in the title and description. On the other hand, I'm not sure what you mean with:

As of python 3.9 some classes actually are subscriptable

My first thought is that your MyClass is subscriptable because it's a child of list... so I'm having a hard time understanding how Django is at fault here. Could you please elaborate?

Also for this sentence:

I'd be happy to propose a fix that will not call a classes' __class_getitem__ method.

I have grepped all the Django source code and nothing other than a few classes in the ORM implement __class_getitem__, so what do you mean exactly?

$ grep -nR __class_getitem__
django/db/models/fields/related.py:999:    def __class_getitem__(cls, *args, **kwargs):
django/db/models/manager.py:39:    def __class_getitem__(cls, *args, **kwargs):
django/db/models/query.py:435:    def __class_getitem__(cls, *args, **kwargs):

I'm closing as needsinfo but please reopen when you can provide further clarifications. Thanks again!

comment:6 by Natalia Bidart, 2 months ago

Resolution: needsinfo
Status: assignedclosed
Version: 5.0dev

comment:7 by Fabian Braun, 2 months ago

Description: modified (diff)

comment:8 by Fabian Braun, 2 months ago

Resolution: needsinfo
Status: closednew

Hi Natalia!

Thanks for taking time to look into this.

Sorry, indeed the example needs the list parent class. Sorry, I missed that on the ticket. The fix has three tests that work with python 3.8 and not with python 3.9+. I wanted the test code to be as short as possible and that took me more iterations than anticipated.

While these tests subclass list, any class that implements some sort of type hinting using __class_get_item__ will have its properties or methods shadowed. list is just a built-in example. Having said this, you will have to assume that projects subclass Django classes, shadowing for example a model's manager:

{% for obj in MyModel.objects.all %}
    <li>{{ obj }}</li>
{% endfor %}

will not work, if the custom MyModel for some reason implements __class_get_item__ - a thing one might do to annotate, for example, what model a generic foreign key might refer to, or what sort of data is stored in a JSON field, or ....

At the time the template system was designed, classes were never subscriptable. Python has changed, and that's not Django's fault. But I believe it is time that Django changes with python. The fix I propose avoids the issue because it does not run MyClass["property_name"] in the first place. This restores the original template variable resolution design and order of how template references were resolved before python 3.9.

With type hinting getting more and more popular, I expect this issue to become more and more important. That's what I mean with "class properties may not be accessible". Sorry, if it felt like I was overstating the issue.

I guess, already now, the issue might be important: The latest version of django-modeltranslation adds `__class_getitem__` to all admin classes. This has unforeseen side effects on all admin classes of all projects using django-modeltranslation. (And to prevent the discussion if django-modeltranslation should fix this: (a) they've worked their way around it and (b) it is not a specific issue for django-modeltranslation but for all classes that make their way into Django's template system.)

I hope I could clarify your questions. Please keep asking if there is need for more information. Since I am still convinced this should be fixed, hence I reopen the ticket.

comment:9 by Carlton Gibson, 2 months ago

This seems correct to me. (The test cases aren’t quite as clear as the explanation here… I’m not sure I’d see quickly — at all 😅 — the connection to __class_getitem__ without an explicit example, even if only for documentation’s sake.)

comment:10 by Fabian Braun, 2 months ago

Python (since 3.9) resolves MyClass["something"] this way (taken from the docs linked - adapted for a class):

def subscribe(cls, x):
    """Return the result of the expression 'cls[x] if cls is a class'"""

    metaclass = type(cls)

    # If the metaclass of cls defines __getitem__, call metaclass.__getitem__(cls, x)
    if hasattr(metaclass, '__getitem__'):  # This is also true for python pre-3.9
        return metaclass.__getitem__(cls, x)

    # New in Python 3.9:
    # Else, if obj is a class and defines __class_getitem__, call cls.__class_getitem__(x)
    elif hasattr(cls, '__class_getitem__'):  
        # Instead of TypeError the __class_getitem__ class method returns a GenericAlias object
        return cls.__class_getitem__(x)  

    # Else, raise an exception - this will let Django's template system try a property next
    else:
        raise TypeError(
            f"'{cls.__name__}' object is not subscriptable"
        )
Last edited 2 months ago by Fabian Braun (previous) (diff)

comment:11 by Sarah Boyce, 2 months ago

Triage Stage: UnreviewedAccepted

comment:12 by Sarah Boyce, 2 months ago

Triage Stage: AcceptedReady for checkin

comment:13 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

Resolution: fixed
Status: newclosed

In d2c97981:

Fixed #35735 -- Enabled template access to methods and properties of classes with class_get_item.

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