#35531: BaseModelFormSet.clean() recursively triggers is_valid() which may cause
problems when is_valid() was overriden
--------------------------------------------+------------------------
Reporter: Christophe Henry | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
--------------------------------------------+------------------------
The current implementation of
[https://github.com/django/django/blob/main/django/forms/models.py#L800-L801
`BaseModelFormSet.clean()`] calls
[https://github.com/django/django/blob/main/django/forms/models.py#L800-L801
`BaseModelFormSet.validate_unique()`] which itself calls
[https://github.com/django/django/blob/main/django/forms/formsets.py#L286
`self.is_valid()`] though
[https://github.com/django/django/blob/main/django/forms/models.py#L807
`self.deleted_forms`].
This recursive call of `is_valid()` may mess up the validation if
`is_valid()` was overriden and `self.data` is altered during the process.
For instance:
```
{{{#!python
from django.forms import BaseModelFormSet
from django.forms.formsets import TOTAL_FORM_COUNT
class ExampleFormSet(BaseModelFormSet):
def __init__(self, extra_data_provider, **kwargs):
self.extra_data_provider = extra_data_provider
super().__init__(**kwargs)
def is_valid(self):
if not self.is_bound:
return False
next_idx =
self.data[self.management_form.add_prefix(TOTAL_FORM_COUNT)]
old_data = self.data
self.data = self.data.copy()
self.data[self.management_form.add_prefix(TOTAL_FORM_COUNT)] =
f"{next_idx+1}"
self.management_form.full_clean()
# Retreiving form data from somewhere else (this is an example)
next_form_data = self.extra_data_provider.retreive()
self.data.update(next_form_data)
self.forms.append(
self._construct_form(next_idx,
**self.get_form_kwargs(next_idx))
)
result = super().is_valid()
self._validate_empty_form = False
if not result:
# Reverting our changes
self.data = old_data
self.management_form.full_clean()
self.forms.pop()
return result
}}}
Let's assume `super().is_valid()` is `False` because `self.clean()`
failed. Then `self.forms.append()` will be called twice, but the cleaning
in the `if not result` block ` will perform only once.
In my opinion, this is a bug that produces unexpected behavior and is
difficult to debug.
This could be solved
[https://github.com/django/django/blob/main/django/forms/formsets.py#L286
by not calling `self.is_valid()` in `BaseFormSet.deleted_forms`] and
instead perform only a partial validation here. I enclosed a patch to this
ticket to show my solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/35531>
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 on the web visit
https://groups.google.com/d/msgid/django-updates/010701902bb530ec-1df8d4e6-2036-4144-9a40-021f09fa351d-000000%40eu-central-1.amazonses.com.
