diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py index fdb05a626a9..aa232ab1d50 100644 --- a/django/contrib/contenttypes/generic.py +++ b/django/contrib/contenttypes/generic.py @@ -435,7 +435,7 @@ def generic_inlineformset_factory(model, form=ModelForm, fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, - formfield_callback=None): + formfield_callback=None, validate_max=False): """ Returns a ``GenericInlineFormSet`` for the given kwargs. @@ -457,7 +457,8 @@ def generic_inlineformset_factory(model, form=ModelForm, formfield_callback=formfield_callback, formset=formset, extra=extra, can_delete=can_delete, can_order=can_order, - fields=fields, exclude=exclude, max_num=max_num) + fields=fields, exclude=exclude, max_num=max_num, + validate_max=validate_max) FormSet.ct_field = ct_field FormSet.ct_fk_field = fk_field return FormSet diff --git a/django/forms/formsets.py b/django/forms/formsets.py index 81b75f27960..98ae3205feb 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -33,6 +33,9 @@ class ManagementForm(Form): def __init__(self, *args, **kwargs): self.base_fields[TOTAL_FORM_COUNT] = IntegerField(widget=HiddenInput) self.base_fields[INITIAL_FORM_COUNT] = IntegerField(widget=HiddenInput) + # MAX_NUM_FORM_COUNT is output with the rest of the management form, + # but only for the convenience of client-side code. The POST + # value of MAX_NUM_FORM_COUNT returned from the client is not checked. self.base_fields[MAX_NUM_FORM_COUNT] = IntegerField(required=False, widget=HiddenInput) super(ManagementForm, self).__init__(*args, **kwargs) @@ -94,7 +97,11 @@ class BaseFormSet(object): def total_form_count(self): """Returns the total number of forms in this FormSet.""" if self.is_bound: - return self.management_form.cleaned_data[TOTAL_FORM_COUNT] + # return absolute_max if it is lower than the actual total form + # count in the data; this is DoS protection to prevent clients + # from forcing the server to instantiate arbitrary numbers of + # forms + return min(self.management_form.cleaned_data[TOTAL_FORM_COUNT], self.absolute_max) else: initial_forms = self.initial_form_count() total_forms = initial_forms + self.extra @@ -113,14 +120,13 @@ class BaseFormSet(object): else: # Use the length of the inital data if it's there, 0 otherwise. initial_forms = self.initial and len(self.initial) or 0 - if initial_forms > self.max_num >= 0: - initial_forms = self.max_num return initial_forms def _construct_forms(self): # instantiate all the forms and put them in self.forms self.forms = [] - for i in xrange(min(self.total_form_count(), self.absolute_max)): + # DoS protection is included in total_form_count() + for i in xrange(self.total_form_count()): self.forms.append(self._construct_form(i)) def _construct_form(self, i, **kwargs): @@ -168,7 +174,6 @@ class BaseFormSet(object): self.add_fields(form, None) return form - # Maybe this should just go away? @property def cleaned_data(self): """ @@ -294,8 +299,11 @@ class BaseFormSet(object): for i in range(0, self.total_form_count()): form = self.forms[i] self._errors.append(form.errors) - # Give self.clean() a chance to do cross-form validation. try: + if (self.validate_max and self.total_form_count() > self.max_num) or \ + self.management_form.cleaned_data[TOTAL_FORM_COUNT] > self.absolute_max: + raise ValidationError(_("Please submit %s or fewer forms." % self.max_num)) + # Give self.clean() a chance to do cross-form validation. self.clean() except ValidationError as e: self._non_form_errors = self.error_class(e.messages) @@ -367,16 +375,18 @@ class BaseFormSet(object): return mark_safe('\n'.join([six.text_type(self.management_form), forms])) def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False, - can_delete=False, max_num=None): + can_delete=False, max_num=None, validate_max=False): """Return a FormSet for the given form class.""" if max_num is None: max_num = DEFAULT_MAX_NUM # hard limit on forms instantiated, to prevent memory-exhaustion attacks - # limit defaults to DEFAULT_MAX_NUM, but developer can increase it via max_num - absolute_max = max(DEFAULT_MAX_NUM, max_num) + # limit is simply max_num + DEFAULT_MAX_NUM (which is 2*DEFAULT_MAX_NUM + # if max_num is None in the first place) + absolute_max = max_num + DEFAULT_MAX_NUM attrs = {'form': form, 'extra': extra, 'can_order': can_order, 'can_delete': can_delete, - 'max_num': max_num, 'absolute_max': absolute_max} + 'max_num': max_num, 'absolute_max': absolute_max, + 'validate_max' : validate_max} return type(form.__name__ + str('FormSet'), (formset,), attrs) def all_valid(formsets): diff --git a/django/forms/models.py b/django/forms/models.py index 272f1ddee66..0672bafc478 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -682,7 +682,7 @@ class BaseModelFormSet(BaseFormSet): def modelformset_factory(model, form=ModelForm, formfield_callback=None, formset=BaseModelFormSet, extra=1, can_delete=False, can_order=False, max_num=None, fields=None, - exclude=None, widgets=None): + exclude=None, widgets=None, validate_max=False): """ Returns a FormSet class for the given Django model class. """ @@ -690,7 +690,8 @@ def modelformset_factory(model, form=ModelForm, formfield_callback=None, formfield_callback=formfield_callback, widgets=widgets) FormSet = formset_factory(form, formset, extra=extra, max_num=max_num, - can_order=can_order, can_delete=can_delete) + can_order=can_order, can_delete=can_delete, + validate_max=validate_max) FormSet.model = model return FormSet @@ -826,7 +827,7 @@ def inlineformset_factory(parent_model, model, form=ModelForm, formset=BaseInlineFormSet, fk_name=None, fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, - formfield_callback=None, widgets=None): + formfield_callback=None, widgets=None, validate_max=False): """ Returns an ``InlineFormSet`` for the given kwargs. @@ -848,6 +849,7 @@ def inlineformset_factory(parent_model, model, form=ModelForm, 'exclude': exclude, 'max_num': max_num, 'widgets': widgets, + 'validate_max': validate_max, } FormSet = modelformset_factory(model, **kwargs) FormSet.fk = fk diff --git a/docs/ref/contrib/contenttypes.txt b/docs/ref/contrib/contenttypes.txt index fb85653ce8c..388172c43eb 100644 --- a/docs/ref/contrib/contenttypes.txt +++ b/docs/ref/contrib/contenttypes.txt @@ -492,7 +492,7 @@ information. Subclasses of :class:`GenericInlineModelAdmin` with stacked and tabular layouts, respectively. -.. function:: generic_inlineformset_factory(model, form=ModelForm, formset=BaseGenericInlineFormSet, ct_field="content_type", fk_field="object_id", fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, formfield_callback=None) +.. function:: generic_inlineformset_factory(model, form=ModelForm, formset=BaseGenericInlineFormSet, ct_field="content_type", fk_field="object_id", fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, formfield_callback=None, validate_max=False) Returns a ``GenericInlineFormSet`` using :func:`~django.forms.models.modelformset_factory`. diff --git a/docs/ref/forms/formsets.txt b/docs/ref/forms/formsets.txt new file mode 100644 index 00000000000..0ab2590fcea --- /dev/null +++ b/docs/ref/forms/formsets.txt @@ -0,0 +1,16 @@ +==================== +Formset Functions +==================== + +.. module:: django.forms.formsets + :synopsis: Django's functions for building formsets. + +.. function:: formset_factory(form, formset=BaseFormSet, extra=1, can_order=False, can_delete=False, max_num=None, validate_max=False) + + Returns a ``FormSet`` class for the given ``form`` class. + + See :ref:`formsets` for example usage. + + .. versionchanged:: 1.6 + + The ``validate_max`` parameter was added. diff --git a/docs/ref/forms/index.txt b/docs/ref/forms/index.txt index 446fdb82ded..e6edc88ca17 100644 --- a/docs/ref/forms/index.txt +++ b/docs/ref/forms/index.txt @@ -10,5 +10,6 @@ Detailed form API reference. For introductory material, see :doc:`/topics/forms/ api fields models + formsets widgets validation diff --git a/docs/ref/forms/models.txt b/docs/ref/forms/models.txt index f3382d32c75..dd0a422fd08 100644 --- a/docs/ref/forms/models.txt +++ b/docs/ref/forms/models.txt @@ -25,7 +25,7 @@ Model Form Functions See :ref:`modelforms-factory` for example usage. -.. function:: modelformset_factory(model, form=ModelForm, formfield_callback=None, formset=BaseModelFormSet, extra=1, can_delete=False, can_order=False, max_num=None, fields=None, exclude=None, widgets=None) +.. function:: modelformset_factory(model, form=ModelForm, formfield_callback=None, formset=BaseModelFormSet, extra=1, can_delete=False, can_order=False, max_num=None, fields=None, exclude=None, widgets=None, validate_max=False) Returns a ``FormSet`` class for the given ``model`` class. @@ -33,17 +33,18 @@ Model Form Functions ``formfield_callback`` and ``widgets`` are all passed through to :func:`~django.forms.models.modelform_factory`. - Arguments ``formset``, ``extra``, ``max_num``, ``can_order``, and - ``can_delete`` are passed through to ``formset_factory``. See - :ref:`formsets` for details. + Arguments ``formset``, ``extra``, ``max_num``, ``can_order``, + ``can_delete`` and ``validate_max`` are passed through to + :func:`~django.forms.formsets.formset_factory`. See :ref:`formsets` for + details. See :ref:`model-formsets` for example usage. .. versionchanged:: 1.6 - The widgets parameter was added. + The ``widgets`` and the ``validate_max`` parameters were added. -.. function:: inlineformset_factory(parent_model, model, form=ModelForm, formset=BaseInlineFormSet, fk_name=None, fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, formfield_callback=None, widgets=None) +.. function:: inlineformset_factory(parent_model, model, form=ModelForm, formset=BaseInlineFormSet, fk_name=None, fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, formfield_callback=None, widgets=None, validate_max=False) Returns an ``InlineFormSet`` using :func:`modelformset_factory` with defaults of ``formset=BaseInlineFormSet``, ``can_delete=True``, and @@ -56,4 +57,4 @@ Model Form Functions .. versionchanged:: 1.6 - The widgets parameter was added. + The ``widgets`` and the ``validate_max`` parameters were added. diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index 132ce682321..16039851c28 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -167,6 +167,14 @@ Minor features field mixins to implement ``__init__()`` methods that will reliably be called. +* The ``validate_max`` parameter was added to ``BaseFormSet`` and + :func:`~django.forms.formsets.formset_factory`, and ``ModelForm`` and inline + versions of the same. The behavior of validation for formsets with + ``max_num`` was clarified. The previously undocumented behavior that + hardened formsets against memory exhaustion attacks was documented, + and the undocumented limit of the higher of 1000 or ``max_num`` forms + was changed so it is always 1000 more than ``max_num``. + Backwards incompatible changes in 1.6 ===================================== diff --git a/docs/topics/forms/formsets.txt b/docs/topics/forms/formsets.txt index 2534947dd34..f0a7668e0d2 100644 --- a/docs/topics/forms/formsets.txt +++ b/docs/topics/forms/formsets.txt @@ -32,8 +32,8 @@ would with a regular form:: As you can see it only displayed one empty form. The number of empty forms that is displayed is controlled by the ``extra`` parameter. By default, -``formset_factory`` defines one extra form; the following example will -display two blank forms:: +:func:`~django.forms.formsets.formset_factory` defines one extra form; the +following example will display two blank forms:: >>> ArticleFormSet = formset_factory(ArticleForm, extra=2) @@ -84,8 +84,9 @@ list of dictionaries as the initial data. Limiting the maximum number of forms ------------------------------------ -The ``max_num`` parameter to ``formset_factory`` gives you the ability to -limit the maximum number of empty forms the formset will display:: +The ``max_num`` parameter to :func:`~django.forms.formsets.formset_factory` +gives you the ability to limit the maximum number of empty forms the formset +will display:: >>> ArticleFormSet = formset_factory(ArticleForm, extra=2, max_num=1) >>> formset = ArticleFormSet() @@ -101,6 +102,20 @@ so long as the total number of forms does not exceed ``max_num``. A ``max_num`` value of ``None`` (the default) puts a high limit on the number of forms displayed (1000). In practice this is equivalent to no limit. +If the number of forms in the initial data exceeds ``max_num``, all initial +data forms will be displayed regardless. (No extra forms will be displayed.) + +By default, ``max_num`` only affects how many forms are displayed and does not +affect validation. If ``validate_max=True`` is passed to the +:func:`~django.forms.formsets.formset_factory`, then ``max_num`` will affect +validation. See :ref:`validate_max`. + +.. versionchanged:: 1.6 + The ``validate_max`` parameter was added to + :func:`~django.forms.formsets.formset_factory`. Also, the behavior of + ``FormSet`` was brought in line with that of ``ModelFormSet`` so that it + displays initial data regardless of ``max_num``. + Formset validation ------------------ @@ -248,14 +263,59 @@ The formset ``clean`` method is called after all the ``Form.clean`` methods have been called. The errors will be found using the ``non_form_errors()`` method on the formset. +.. _validate_max: + +Validating the number of forms in a formset +------------------------------------------- + +If ``validate_max=True`` is passed to +:func:`~django.forms.formsets.formset_factory`, validation will also check +that the number of forms in the data set is less than or equal to ``max_num``. + + >>> ArticleFormSet = formset_factory(ArticleForm, max_num=1, validate_max=True) + >>> data = { + ... 'form-TOTAL_FORMS': u'2', + ... 'form-INITIAL_FORMS': u'0', + ... 'form-MAX_NUM_FORMS': u'', + ... 'form-0-title': u'Test', + ... 'form-0-pub_date': u'1904-06-16', + ... 'form-1-title': u'Test 2', + ... 'form-1-pub_date': u'1912-06-23', + ... } + >>> formset = ArticleFormSet(data) + >>> formset.is_valid() + False + >>> formset.errors + [{}, {}] + >>> formset.non_form_errors() + [u'Please submit 1 or fewer forms.'] + +``validate_max=True`` validates against ``max_num`` strictly even if +``max_num`` was exceeded because the amount of initial data supplied was +excessive. + +Applications which need more customizable validation of the number of forms +should use custom formset validation. + +.. note:: + + Regardless of ``validate_max``, if the number of forms in a data set + exceeds ``max_num`` by more than 1000, then the form will fail to validate + as if ``validate_max`` were set, and additionally only the first 1000 + forms above ``max_num`` will be validated. The remainder will be + truncated entirely. This is to protect against memory exhaustion attacks + using forged POST requests. + +.. versionchanged:: 1.6 + The ``validate_max`` parameter was added to + :func:`~django.forms.formsets.formset_factory`. + Dealing with ordering and deletion of forms ------------------------------------------- -Common use cases with a formset is dealing with ordering and deletion of the -form instances. This has been dealt with for you. The ``formset_factory`` -provides two optional parameters ``can_order`` and ``can_delete`` that will do -the extra work of adding the extra fields and providing simpler ways of -getting to that data. +The :func:`~django.forms.formsets.formset_factory` provides two optional +parameters ``can_order`` and ``can_delete`` to help with ordering of forms in +formsets and deletion of forms from a formset. ``can_order`` ~~~~~~~~~~~~~ diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt index 62020e461e9..eaf2bbbaf2a 100644 --- a/docs/topics/forms/modelforms.txt +++ b/docs/topics/forms/modelforms.txt @@ -597,9 +597,10 @@ with the ``Author`` model. It works just like a regular formset:: .. note:: - :func:`~django.forms.models.modelformset_factory` uses ``formset_factory`` - to generate formsets. This means that a model formset is just an extension - of a basic formset that knows how to interact with a particular model. + :func:`~django.forms.models.modelformset_factory` uses + :func:`~django.forms.formsets.formset_factory` to generate formsets. This + means that a model formset is just an extension of a basic formset that + knows how to interact with a particular model. Changing the queryset --------------------- diff --git a/tests/forms_tests/tests/formsets.py b/tests/forms_tests/tests/formsets.py index 2bef0c5c337..4ac3c5ecf15 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): """) 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), """ +""") 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), """ -""") + self.assertHTMLEqual('\n'.join(form_output), """ + +""") # 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 c48f88ded8b..8d0c017a617 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 = {