Fixed #20084 -- Provided option to validate formset max_num on server.

This is provided as a new "validate_max" formset_factory option defaulting to
False, since the non-validating behavior of max_num is longstanding, and there
is certainly code relying on it. (In fact, even the Django admin relies on it
for the case where there are more existing inlines than the given max_num). It
may be that at some point we want to deprecate validate_max=False and
eventually remove the option, but this commit takes no steps in that direction.

This also fixes the DoS-prevention absolute_max enforcement so that it causes a
form validation error rather than an IndexError, and ensures that absolute_max
is always 1000 more than max_num, to prevent surprising changes in behavior
with max_num close to absolute_max.

Lastly, this commit fixes the previous inconsistency between a regular formset
and a model formset in the precedence of max_num and initial data. Previously
in a regular formset, if the provided initial data was longer than max_num, it
was truncated; in a model formset, all initial forms would be displayed
regardless of max_num. Now regular formsets are the same as model formsets; all
initial forms are displayed, even if more than max_num. (But if validate_max is
True, submitting these forms will result in a "too many forms" validation
error!) This combination of behaviors was chosen to keep the max_num validation
simple and consistent, and avoid silent data loss due to truncation of initial
data.

Thanks to Preston for discussion of the design choices.

2 files changed, 75 insertions, 12 deletions

diff --git a/tests/forms_tests/tests/formsets.py b/tests/forms_tests/tests/formsets.py
index 2bef0c5c33..4ac3c5ecf1 100644
--- a/tests/forms_tests/tests/formsets.py
+++ b/tests/forms_tests/tests/formsets.py
@@ -256,6 +256,27 @@ class FormsFormsetTestCase(TestCase):
         self.assertTrue(formset.is_valid())
         self.assertEqual([form.cleaned_data for form in formset.forms], [{'votes': 100, 'choice': 'Calexico'}, {}, {}])
 
+    def test_formset_validate_max_flag(self):
+        # If validate_max is set and max_num is less than TOTAL_FORMS in the
+        # data, then throw an exception. MAX_NUM_FORMS in the data is
+        # irrelevant here (it's output as a hint for the client but its
+        # value in the returned data is not checked)
+
+        data = {
+            'choices-TOTAL_FORMS': '2', # the number of forms rendered
+            'choices-INITIAL_FORMS': '0', # the number of forms with initial data
+            'choices-MAX_NUM_FORMS': '2', # max number of forms - should be ignored
+            'choices-0-choice': 'Zero',
+            'choices-0-votes': '0',
+            'choices-1-choice': 'One',
+            'choices-1-votes': '1',
+        }
+
+        ChoiceFormSet = formset_factory(Choice, extra=1, max_num=1, validate_max=True)
+        formset = ChoiceFormSet(data, auto_id=False, prefix='choices')
+        self.assertFalse(formset.is_valid())
+        self.assertEqual(formset.non_form_errors(), ['Please submit 1 or fewer forms.'])
+
     def test_second_form_partially_filled_2(self):
         # And once again, if we try to partially complete a form, validation will fail.
 
@@ -720,8 +741,20 @@ class FormsFormsetTestCase(TestCase):
 <tr><th><label for="id_form-1-name">Name:</label></th><td><input type="text" name="form-1-name" id="id_form-1-name" /></td></tr>""")
 
     def test_max_num_zero(self):
-        # If max_num is 0 then no form is rendered at all, even if extra and initial
-        # are specified.
+        # If max_num is 0 then no form is rendered at all, regardless of extra,
+        # unless initial data is present. (This changed in the patch for bug
+        # 20084 -- previously max_num=0 trumped initial data)
+
+        LimitedFavoriteDrinkFormSet = formset_factory(FavoriteDrinkForm, extra=1, max_num=0)
+        formset = LimitedFavoriteDrinkFormSet()
+        form_output = []
+
+        for form in formset.forms:
+            form_output.append(str(form))
+
+        self.assertEqual('\n'.join(form_output), "")
+
+        # test that initial trumps max_num
 
         initial = [
             {'name': 'Fernet and Coke'},
@@ -733,12 +766,13 @@ class FormsFormsetTestCase(TestCase):
 
         for form in formset.forms:
             form_output.append(str(form))
-
-        self.assertEqual('\n'.join(form_output), "")
+        self.assertEqual('\n'.join(form_output), """<tr><th><label for="id_form-0-name">Name:</label></th><td><input id="id_form-0-name" name="form-0-name" type="text" value="Fernet and Coke" /></td></tr>
+<tr><th><label for="id_form-1-name">Name:</label></th><td><input id="id_form-1-name" name="form-1-name" type="text" value="Bloody Mary" /></td></tr>""")
 
     def test_more_initial_than_max_num(self):
-        # More initial forms than max_num will result in only the first max_num of
-        # them to be displayed with no extra forms.
+        # More initial forms than max_num now results in all initial forms
+        # being displayed (but no extra forms).  This behavior was changed
+        # from max_num taking precedence in the patch for #20084
 
         initial = [
             {'name': 'Gin Tonic'},
@@ -751,9 +785,9 @@ class FormsFormsetTestCase(TestCase):
 
         for form in formset.forms:
             form_output.append(str(form))
-
-        self.assertHTMLEqual('\n'.join(form_output), """<tr><th><label for="id_form-0-name">Name:</label></th><td><input type="text" name="form-0-name" value="Gin Tonic" id="id_form-0-name" /></td></tr>
-<tr><th><label for="id_form-1-name">Name:</label></th><td><input type="text" name="form-1-name" value="Bloody Mary" id="id_form-1-name" /></td></tr>""")
+        self.assertHTMLEqual('\n'.join(form_output), """<tr><th><label for="id_form-0-name">Name:</label></th><td><input id="id_form-0-name" name="form-0-name" type="text" value="Gin Tonic" /></td></tr>
+<tr><th><label for="id_form-1-name">Name:</label></th><td><input id="id_form-1-name" name="form-1-name" type="text" value="Bloody Mary" /></td></tr>
+<tr><th><label for="id_form-2-name">Name:</label></th><td><input id="id_form-2-name" name="form-2-name" type="text" value="Jack and Coke" /></td></tr>""")
 
         # One form from initial and extra=3 with max_num=2 should result in the one
         # initial form and one extra.
@@ -883,8 +917,8 @@ class FormsFormsetTestCase(TestCase):
         # reduce the default limit of 1000 temporarily for testing
         _old_DEFAULT_MAX_NUM = formsets.DEFAULT_MAX_NUM
         try:
-            formsets.DEFAULT_MAX_NUM = 3
-            ChoiceFormSet = formset_factory(Choice)
+            formsets.DEFAULT_MAX_NUM = 2
+            ChoiceFormSet = formset_factory(Choice, max_num=1)
             # someone fiddles with the mgmt form data...
             formset = ChoiceFormSet(
                 {
@@ -904,6 +938,8 @@ class FormsFormsetTestCase(TestCase):
                 )
             # But we still only instantiate 3 forms
             self.assertEqual(len(formset.forms), 3)
+            # and the formset isn't valid
+            self.assertFalse(formset.is_valid())
         finally:
             formsets.DEFAULT_MAX_NUM = _old_DEFAULT_MAX_NUM
 
@@ -931,7 +967,7 @@ class FormsFormsetTestCase(TestCase):
                     },
                 prefix='choices',
                 )
-            # This time four forms are instantiated
+            # Four forms are instantiated and no exception is raised
             self.assertEqual(len(formset.forms), 4)
         finally:
             formsets.DEFAULT_MAX_NUM = _old_DEFAULT_MAX_NUM
diff --git a/tests/model_formsets/tests.py b/tests/model_formsets/tests.py
index c48f88ded8..8d0c017a61 100644
--- a/tests/model_formsets/tests.py
+++ b/tests/model_formsets/tests.py
@@ -899,6 +899,33 @@ class ModelFormsetTest(TestCase):
         self.assertFalse(formset.is_valid())
         self.assertEqual(formset.errors, [{'slug': ['Product with this Slug already exists.']}])
 
+    def test_modelformset_validate_max_flag(self):
+        # If validate_max is set and max_num is less than TOTAL_FORMS in the
+        # data, then throw an exception. MAX_NUM_FORMS in the data is
+        # irrelevant here (it's output as a hint for the client but its
+        # value in the returned data is not checked)
+
+        data = {
+            'form-TOTAL_FORMS': '2',
+            'form-INITIAL_FORMS': '0',
+            'form-MAX_NUM_FORMS': '2', # should be ignored
+            'form-0-price': '12.00',
+            'form-0-quantity': '1',
+            'form-1-price': '24.00',
+            'form-1-quantity': '2',
+        }
+
+        FormSet = modelformset_factory(Price, extra=1, max_num=1, validate_max=True)
+        formset = FormSet(data)
+        self.assertFalse(formset.is_valid())
+        self.assertEqual(formset.non_form_errors(), ['Please submit 1 or fewer forms.'])
+
+        # Now test the same thing without the validate_max flag to ensure
+        # default behavior is unchanged
+        FormSet = modelformset_factory(Price, extra=1, max_num=1)
+        formset = FormSet(data)
+        self.assertTrue(formset.is_valid())
+
     def test_unique_together_validation(self):
         FormSet = modelformset_factory(Price, extra=1)
         data = {