Opened 34 hours ago

Last modified 33 hours ago

#35909 closed Bug

Mutation of FormMixin.initial can cause a FormView with a formset to crash — at Version 1

Reported by: David Sanders Owned by:
Component: Generic views Version: 5.1
Severity: Normal Keywords: FormView FormSet
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by David Sanders)

Hi Djangonauts, long time no see… have been super busy, hope you're all well!

Here's an interesting problem with some backstory for which I never knew Django behaved like this.

So some of the old timers may know that FormMixin.initial is a mutable class attribute, and there have been past bug reports concerning this, the main one from 13 years ago is #16138

The fix for this was to call .copy() during get_initial().

However, I just spent a couple of days tracking down and fixing a 500 which was caused by this 😅 The direct cause for this was a junior dev mutating initial directly, not realising that because it is a class attribute then it's a long-living mutation.

Example

Given this simple example model setup:

class Foo(Model):
    foo = IntegerField()

class Bar(Model):
    foo = IntegerField()

and these 2 views:

class FooCreateView(CreateView):
    model = Foo
    fields = ["foo"]

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.initial["foo"] = 2

class BarCreateView(CreateView):
    model = Bar
    fields = ["foo"]

If you first visit FooCreateView, then visit BarCreateView; then this will have the unintended side-effect of initialising BarCreateView with 2.

This alone doesn't cause a 500 but it is a source of potential bug that may go unnoticed!

Furthermore if you create a FormView with a model formset, this will cause it to 500!

BarFormSet = modelformset_factory(Bar, fields=["foo"])

class LotsOfBarCreateView(FormView):
    template_name = "bars.html"
    form_class = BarFormSet

The unintended initialisation is incompatible with the way initialisation works with formsets - formsets expect a list, not a dictionary - see the traceback below

Potential Solution?

If we simply move the initialisation of initial to an __init__() the problem goes away. Furthermore I ran the full test suite with this change and nothing seems to break.

Claude seems to the only active contributor that was part of the original ticket - Would you (or anyone else) know why the solution wasn't to initialise from __init__() to begin with? I did see mention of it in a duplicate ticket #16701. The original ticket appears to be from prior GitHub times so there's no PR discussion to review.

index ebd071cf00..bff9ac68ce 100644
--- a/django/views/generic/edit.py
+++ b/django/views/generic/edit.py
@@ -13,11 +13,15 @@ from django.views.generic.detail import (
 class FormMixin(ContextMixin):
     """Provide a way to show and handle a form in a request."""

-    initial = {}
+    initial = None
     form_class = None
     success_url = None
     prefix = None

+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.initial = {}
+
     def get_initial(self):
         """Return the initial data to use for forms on this view."""
         return self.initial.copy()

Traceback

Here's the traceback:

Traceback (most recent call last):
  File "/path/to/test-project/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/core/handlers/base.py", line 220, in _get_response
    response = response.render()
               ^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/response.py", line 114, in render
    self.content = self.rendered_content
                   ^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/response.py", line 92, in rendered_content
    return template.render(context, self._request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/backends/django.py", line 107, in render
    return self.template.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 171, in render
    return self._render(context)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 163, in _render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 1016, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 1016, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 977, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 1081, in render
    return render_value_in_context(output, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 1058, in render_value_in_context
    value = str(value)
            ^^^^^^^^^^
  File "/path/to/test-project/django/forms/utils.py", line 55, in render
    return mark_safe(renderer.render(template, context))
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/forms/renderers.py", line 29, in render
    return template.render(context, request=request).strip()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/backends/django.py", line 107, in render
    return self.template.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 171, in render
    return self._render(context)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 163, in _render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 1016, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 1016, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/base.py", line 977, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/template/defaulttags.py", line 199, in render
    len_values = len(values)
                 ^^^^^^^^^^^
  File "/path/to/test-project/django/forms/formsets.py", line 121, in __len__
    return len(self.forms)
               ^^^^^^^^^^
  File "/path/to/test-project/django/utils/functional.py", line 47, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
                                         ^^^^^^^^^^^^^^^^^^^
  File "/path/to/test-project/django/forms/formsets.py", line 205, in forms
    return [
           ^
  File "/path/to/test-project/django/forms/formsets.py", line 206, in <listcomp>
    self._construct_form(i, **self.get_form_kwargs(i))
  File "/path/to/test-project/django/forms/models.py", line 740, in _construct_form
    kwargs["initial"] = self.initial_extra[i - self.initial_form_count()]
                        ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 0

Change History (1)

comment:1 by David Sanders, 34 hours ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top