Mutation of FormMixin.initial can cause a FormView with a formset to crash

[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

[Sarah Boyce]

Hi David, long time no see!

So it looks like we don't have tests covering setting the initial data but I would expect folks to either set the initial attribute or override get_initial(), I also feel like that's in the docs although there isn't an explicit example: ​https://docs.djangoproject.com/en/5.1/ref/class-based-views/mixins-editing/#formmixin

So in short, I think what they have done is not how it should be used and so I wouldn't call this a bug and would close as invalid

I would be open to adding more test coverage (and maybe doc updates to make the above clearer)
Something like (with better naming)...

tests/generic_views/test_edit.py

@@ -14 +14 @@
 class FormMixinTests(SimpleTestCase):
     request_factory = RequestFactory()
 
-    def test_initial_data(self):
+    def test_initial_data_independent(self):
         """Test instance independence of initial data dict (see #16138)"""
         initial_1 = FormMixin().get_initial()
         initial_1["foo"] = "bar"
         initial_2 = FormMixin().get_initial()
         self.assertNotEqual(initial_1, initial_2)
 
+    def test_set_initial_data(self):
+        class InitialAttribute(FormMixin):
+            initial = {"foo": "bar"}
+
+        class GetInitial(FormMixin):
+            def get_initial(self):
+                initial = super().get_initial()
+                initial["foo"] = "bar"
+                return initial
+
+        self.assertEqual(InitialAttribute().get_initial(), {"foo": "bar"})
+        self.assertEqual(GetInitial().get_initial(), {"foo": "bar"})
+
     def test_get_prefix(self):
         """Test prefix can be set (see #18872)"""
         test_string = "test"