Opened 6 years ago
Closed 5 years ago
#30261 closed Bug (fixed)
Calling a form method _html_output modifies the self._errors dict for NON_FIELD_ERRORS if there are hidden field with errors
Reported by: | Benjamin Mampaey | Owned by: | Hasan Ramezani |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | form errors |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Each time the _html_output method of a form is called, it appends the errors of the hidden field errors to the NON_FIELD_ERRORS (all) entry.
This happen for example when the form methods as_p() as_table() as_ul() are called multiple time, or any other method that themselves call one of them.
For example, a test form with an hidden input field that add errors during the clean call.
Python 3.6.5 (default, Apr 25 2018, 14:26:36) Type 'copyright', 'credits' or 'license' for more information IPython 6.4.0 -- An enhanced Interactive Python. Type '?' for help. In [1]: import django In [2]: django.__version__ Out[2]: '2.1.7' In [3]: from django import forms ...: In [4]: class TestForm(forms.Form): ...: hidden_input = forms.CharField(widget=forms.HiddenInput) ...: ...: def clean(self): ...: self.add_error(None, 'Form error') ...: self.add_error('hidden_input', 'Hidden input error') ...: In [5]: test_form = TestForm({}) In [6]: test_form.errors Out[6]: {'hidden_input': ['This field is required.', 'Hidden input error'], '__all__': ['Form error']} In [7]: print(test_form.as_table()) <tr><td colspan="2"><ul class="errorlist nonfield"><li>Form error</li><li>(Hidden field hidden_input) This field is required.</li><li>(Hidden field hidden_input) Hidden input error</li></ul><input type="hidden" name="hidden_input" id="id_hidden_input"></td></tr> In [8]: test_form.errors Out[8]: {'hidden_input': ['This field is required.', 'Hidden input error'], '__all__': ['Form error', '(Hidden field hidden_input) This field is required.', '(Hidden field hidden_input) Hidden input error']} In [9]: print(test_form.as_table()) <tr><td colspan="2"><ul class="errorlist nonfield"><li>Form error</li><li>(Hidden field hidden_input) This field is required.</li><li>(Hidden field hidden_input) Hidden input error</li><li>(Hidden field hidden_input) This field is required.</li><li>(Hidden field hidden_input) Hidden input error</li></ul><input type="hidden" name="hidden_input" id="id_hidden_input"></td></tr> In [10]: test_form.errors Out[10]: {'hidden_input': ['This field is required.', 'Hidden input error'], '__all__': ['Form error', '(Hidden field hidden_input) This field is required.', '(Hidden field hidden_input) Hidden input error', '(Hidden field hidden_input) This field is required.', '(Hidden field hidden_input) Hidden input error']} In [11]: test_form.non_field_errors() Out[11]: ['Form error', '(Hidden field hidden_input) This field is required.', '(Hidden field hidden_input) Hidden input error', '(Hidden field hidden_input) This field is required.', '(Hidden field hidden_input) Hidden input error']
This bug affects probably also version 2.2.
A simple fix would be to use a copy of the error list before adding the hidden field errors in the file django/forms/forms.py:
--- forms.py 2019-03-17 18:59:04.000000000 +0100 +++ forms_fixed.py 2019-03-17 19:00:08.000000000 +0100 @@ -194,7 +194,7 @@ def _html_output(self, normal_row, error_row, row_ender, help_text_html, errors_on_separate_row): "Output HTML. Used by as_table(), as_ul(), as_p()." - top_errors = self.non_field_errors() # Errors that should be displayed above all fields. + top_errors = self.non_field_errors().copy() # Errors that should be displayed above all fields. output, hidden_fields = [], [] for name, field in self.fields.items():
Change History (5)
comment:1 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Version: | 2.1 → master |
comment:2 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 5 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
comment:4 by , 5 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
I didn't reproduce but the report and the suggested patch make sense, accepting on that basis.
Are you interested in submitting a Github pull request incorporating your patch and a regression test?