Django Widget protocol not implemented properly in WidgetMixin
ercpe opened this issue · 2 comments
Today we hit a nasty bug with dal.ModelSelect2 fields where an autocomplete field would receive forwards from a different class which uses the same base class.
We traced the error back to the WidgetMixin
where we found that it does not implement __deepcopy__
properly and thus would copy a reference to the forward
list between instances of the widget.
Background: When Django creates a Form
instance from a form class, the fields and widgets are copied instead of instantiated. This means that every field/widget must implement __deepcopy__
properly to create clones of any references it holds.
Example to reproduce:
class MyBaseForm(forms.Form):
choice_field = forms.ModelChoiceField(User.objects.all(),
widget=autocomplete.ModelSelect2(url='/somewhere'))
class Form1(MyBaseForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields['choice_field'].widget.forward.append(forward.Const(True, 'filter_a'))
class Form2(MyBaseForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields['choice_field'].widget.forward.append(forward.Const(True, 'filter_b'))
form1 = Form1()
print([x.__dict__ for x in form1.fields['choice_field'].widget.forward]) # prints only filter_a
form2 = Form2()
print([x.__dict__ for x in form2.fields['choice_field'].widget.forward]) # prints filter_a and filter_b
In this example, the reference to the forward
list is copied once when the Form1
instance is created and again when the Form2
instance is created. Thus, the widget.forward.append
call in Form2
adds the forward to the very same list and now affects every instance of the form (in the same python process).
To fix the issue:
Implement __deepcopy__
in WidgetMixin
and copy()
the forward
, e.g.
def __deepcopy__(self, memo):
clone = super().__deepcopy__(memo)
clone.forward = self.forward.copy()
return clone
See django.forms.widgets.Widget.__deepcopy__
and django.forms.widgets.ChoiceWidget.__deepcopy__
for an example in Django itself where mutable types are copied when the Widget is cloned.
Also, the forward
argument to the WidgetMixin.__init__
should also be copied as it has the same issue.
Interesting, let's see what a pull request looks like with this