Add protection to some classes to prevent state leaking between requests?

Django Internals
21

I think I see the source of @KariWassholm’s confusion.

In the post I used the example:

class BookAdmin(admin.ModelAdmin):
    ...
    is_special_popup = True

    def changelist_view(self, request, extra_context=None):
        ...
        self.is_special_popup = True
        ...

This declares the attribute at the class-level, then overwrites it at the instance-level. In Python, those are actually two distinct attributes - BookAdmin.is_special_popup and self.is_special_popup. But the accessor self.is_special_popup will read the class-level BookAdmin.is_special_popup if the instance-level variable is not defined.

I copied this when adapting the original code example, but it’s not necessary for the inter-request pollution to occur. I am now updating the post to show is_special_popup only being created as an instance-level variable, created in __init__():

class BookAdmin(admin.ModelAdmin):
    fields = [
        "title",
        "special",
    ]
    
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.is_special_popup = False

@KariWassholm you are correct that inter-request pollution is possible by setting class-level variables, like:

class MyView(View):
    def dispatch(self, request, *args, **kwargs):
        if request.GET.get("special-popup", None):
            MyView.is_special_popup = True
        return super().dispatch(self, request, *args, **kwargs)

I don’t think there’s anything we can do about that, but hopefully it is more evident how that can share between requests, because we expect classes to exist only once.

But using self prevents this issue:

class MyView(View):
    def dispatch(self, request, *args, **kwargs):
        if request.GET.get("special-popup", None):
            self.is_special_popup = True
        return super().dispatch(self, request, *args, **kwargs)

Classes and their attributes are not isolated per thread - all threads will see changes.

1 Like
22

You mean all threads in one process? I am correct?

23

Aye, we fixed our issue by using the __init__ method.

We also wondered if this would be possible to be caught with a custom linter rule, e.g. in Semgrep or a similar tool.

24

Yes

Yeah, that could be possible. It would be hard in a traditional AST-based linter, like Semgrep or Flake8, because there is no type information, making it impossible to accurately infer whether a class inherits from View. But pylint has type information, and so does Ruff, which should allow extensions at some point.