BaseFormSet.is_valid() produces ValidationError when there is no management form

[anonymous]

I was torn between reporting this as a bug or a feature request, but then I thought if I make it a feature request it will most likely break a lot of Django apps and hence I guess that means it's more of a bug...

Anyway so the line in question is django/forms/formsets.py:292 (in Django version 1.6.1):

for i in range(0, self.total_form_count()):

...where the self.total_form_count() executes this line django/forms/formsets.py:106 (in Django version 1.6.1):

return min(self.management_form.cleaned_data[TOTAL_FORM_COUNT], self.absolute_max)

..which then raises this exception django/forms/formsets.py:87 (in Django version 1.6.1):

raise ValidationError(
                    _('ManagementForm data is missing or has been tampered with'),
                    code='missing_management_form',
                )

That stack trace occurs if/when a user submits a formset after stripping out the management form hidden fields.

I have been using Django for a few years now and have never come across an exception being raised by a form/formset is_valid() call before. So my point is that I believe this exception should never be allowed to leave the BaseFormSet.is_valid() call, because it is an oddball behaviour compared to the rest of the is_valid() implementations.

I.e. I believe there should be a check in BaseFormSet.is_valid() which checks for the presence of a valid management form (first) and returns False if it is not present, as opposed to raising an exception.

Yes I could wrap the is_valid() call in a try/catch, but I believe this is an unnecessary hack caused by a bad design deviation of the implementation of the BaseFormSet.is_valid() method.

I didn't bother creating a patch and test cases, because I have a feeling this will get rejected or something like that, but I just thought I should bring this up, as I can't find mention of it anywhere and it seems important to me.

[anonymous]

So it turns out this oddball behaviour is necessary because the formset cannot be rendered after the lack of management form has been detected, hence you should perform special behaviour for these cases of management form tampering (e.g. redirecting to the same page to make the user start over).

Sorry for wasting anyone's time on this.

[Patryk Zawadzki]

I would like to reopen this and propose changing the formset implementation to never let a ValidationError exception escape .is_valid(). The rationale: management form tampering indicates a problem on the user side and it's not something the site's code can or should recover from. Have the formset become invalid and otherwise behave as if it did not contain any forms.

[Patryk Zawadzki]

Pull request: ​https://github.com/django/django/pull/3634

[Tim Graham]

Actually, I may have been premature in accepting this. It seems this will make debugging quite a bit tricker (when you forget to include a management form, the form will be silently invalid, correct?). At the least, there should be some error message (see also #13060).

Also the current behavior is ​documented so we have to consider backwards compatibility here.

[Patryk Zawadzki]

I agree that adding "management form is invalid" to non-form-specific errors is a nice improvement. Can you provide good wording for that?

As for documentation, it's all damned lies as it used to raise in the constructor like the docs show but now it raises in is_valid instead :)

[Tim Graham]

I think we could use the same wording as the existing exception.

[Patryk Zawadzki]

I've added a non-form error to the formset for the case where management form is not valid. Formset's default string representations do not show non-form errors but they do render individual field errors for the management form which should be enough.

[Tim Graham]

Comments for improvement on PR.

[Jon Dufresne]

Thanks for the original work Patryk Zawadzki. I've continued it in PR ​https://github.com/django/django/pull/13607

[GitHub <noreply@…>]

In 859cd7c:

Fixed #22276 -- Fixed crash when formset management form is invalid.

Co-authored-by: Patryk Zawadzki <patrys@…>