#35909: Mutation of FormMixin.initial can cause a FormView with a formset to
crash
-------------------------------------+-------------------------------------
Reporter: David Sanders | Type: Bug
Status: new | Component: Generic
| views
Version: 5.1 | Severity: Normal
Keywords: FormView FormSet | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
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 get_context_data(self, **kwargs):
self.initial["foo"] = 2
return super().get_context_data(**kwargs)
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35909>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/01070193260de580-5ede34f1-2e6a-4d1d-ad44-6f66c78d7f14-000000%40eu-central-1.amazonses.com.
