From 433dd737f94b09043f64b873b0ac067b3f97364b Mon Sep 17 00:00:00 2001 From: David Smith Date: Thu, 30 Apr 2020 08:34:53 +0100 Subject: [PATCH] Fixed #20347 -- Allowed customizing the maximum number of instantiated forms in formsets. Co-authored-by: ethurgood --- django/forms/formsets.py | 15 +++-- django/forms/models.py | 10 ++- docs/ref/forms/formsets.txt | 6 +- docs/ref/forms/models.txt | 16 +++-- docs/releases/3.2.txt | 5 +- docs/topics/forms/formsets.txt | 42 +++++++++++-- tests/forms_tests/tests/test_formsets.py | 49 +++++++++++++++ tests/model_formsets/tests.py | 78 ++++++++++++++++++++++++ 8 files changed, 202 insertions(+), 19 deletions(-) diff --git a/django/forms/formsets.py b/django/forms/formsets.py index 2b013dcafb..6f819bd696 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -433,16 +433,21 @@ class BaseFormSet: def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False, can_delete=False, max_num=None, validate_max=False, - min_num=None, validate_min=False): + min_num=None, validate_min=False, absolute_max=None): """Return a FormSet for the given form class.""" if min_num is None: min_num = DEFAULT_MIN_NUM if max_num is None: max_num = DEFAULT_MAX_NUM - # hard limit on forms instantiated, to prevent memory-exhaustion attacks - # 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 + # absolute_max is a hard limit on forms instantiated, to prevent + # memory-exhaustion attacks. Default to max_num + DEFAULT_MAX_NUM + # (which is 2 * DEFAULT_MAX_NUM if max_num is None in the first place). + if absolute_max is None: + absolute_max = max_num + DEFAULT_MAX_NUM + if max_num > absolute_max: + raise ValueError( + "'absolute_max' must be greater or equal to 'max_num'." + ) attrs = { 'form': form, 'extra': extra, diff --git a/django/forms/models.py b/django/forms/models.py index 0f6608a4bc..9f21156329 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -862,7 +862,8 @@ def modelformset_factory(model, form=ModelForm, formfield_callback=None, can_order=False, max_num=None, fields=None, exclude=None, widgets=None, validate_max=False, localized_fields=None, labels=None, help_texts=None, error_messages=None, - min_num=None, validate_min=False, field_classes=None): + min_num=None, validate_min=False, field_classes=None, + absolute_max=None): """Return a FormSet class for the given Django model class.""" meta = getattr(form, 'Meta', None) if (getattr(meta, 'fields', fields) is None and @@ -879,7 +880,8 @@ def modelformset_factory(model, form=ModelForm, formfield_callback=None, error_messages=error_messages, field_classes=field_classes) FormSet = formset_factory(form, formset, extra=extra, min_num=min_num, max_num=max_num, can_order=can_order, can_delete=can_delete, - validate_min=validate_min, validate_max=validate_max) + validate_min=validate_min, validate_max=validate_max, + absolute_max=absolute_max) FormSet.model = model return FormSet @@ -1048,7 +1050,8 @@ def inlineformset_factory(parent_model, model, form=ModelForm, can_delete=True, max_num=None, formfield_callback=None, widgets=None, validate_max=False, localized_fields=None, labels=None, help_texts=None, error_messages=None, - min_num=None, validate_min=False, field_classes=None): + min_num=None, validate_min=False, field_classes=None, + absolute_max=None): """ Return an ``InlineFormSet`` for the given kwargs. @@ -1078,6 +1081,7 @@ def inlineformset_factory(parent_model, model, form=ModelForm, 'help_texts': help_texts, 'error_messages': error_messages, 'field_classes': field_classes, + 'absolute_max': absolute_max, } FormSet = modelformset_factory(model, **kwargs) FormSet.fk = fk diff --git a/docs/ref/forms/formsets.txt b/docs/ref/forms/formsets.txt index 778ae20db3..e145eeb103 100644 --- a/docs/ref/forms/formsets.txt +++ b/docs/ref/forms/formsets.txt @@ -11,8 +11,12 @@ Formset API reference. For introductory material about formsets, see the ``formset_factory`` =================== -.. function:: formset_factory(form, formset=BaseFormSet, extra=1, can_order=False, can_delete=False, max_num=None, validate_max=False, min_num=None, validate_min=False) +.. function:: formset_factory(form, formset=BaseFormSet, extra=1, can_order=False, can_delete=False, max_num=None, validate_max=False, min_num=None, validate_min=False, absolute_max=None) Returns a ``FormSet`` class for the given ``form`` class. See :doc:`formsets ` for example usage. + + .. versionchanged:: 3.2 + + The ``absolute_max`` argument was added. diff --git a/docs/ref/forms/models.txt b/docs/ref/forms/models.txt index c5ddf99011..e18660b3c2 100644 --- a/docs/ref/forms/models.txt +++ b/docs/ref/forms/models.txt @@ -52,7 +52,7 @@ Model Form API reference. For introductory material about model forms, see the ``modelformset_factory`` ======================== -.. 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, localized_fields=None, labels=None, help_texts=None, error_messages=None, min_num=None, validate_min=False, field_classes=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, localized_fields=None, labels=None, help_texts=None, error_messages=None, min_num=None, validate_min=False, field_classes=None, absolute_max=None) Returns a ``FormSet`` class for the given ``model`` class. @@ -62,16 +62,20 @@ Model Form API reference. For introductory material about model forms, see the through to :func:`~django.forms.models.modelform_factory`. Arguments ``formset``, ``extra``, ``max_num``, ``can_order``, - ``can_delete`` and ``validate_max`` are passed through to - :func:`~django.forms.formsets.formset_factory`. See :doc:`formsets + ``can_delete``, ``validate_max``, and ``absolute_max`` are passed through + to :func:`~django.forms.formsets.formset_factory`. See :doc:`formsets ` for details. See :ref:`model-formsets` for example usage. + .. versionchanged:: 3.2 + + The ``absolute_max`` argument was added. + ``inlineformset_factory`` ========================= -.. 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, localized_fields=None, labels=None, help_texts=None, error_messages=None, min_num=None, validate_min=False, field_classes=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, localized_fields=None, labels=None, help_texts=None, error_messages=None, min_num=None, validate_min=False, field_classes=None, absolute_max=None) Returns an ``InlineFormSet`` using :func:`modelformset_factory` with defaults of ``formset=``:class:`~django.forms.models.BaseInlineFormSet`, @@ -81,3 +85,7 @@ Model Form API reference. For introductory material about model forms, see the the ``parent_model``, you must specify a ``fk_name``. See :ref:`inline-formsets` for example usage. + + .. versionchanged:: 3.2 + + The ``absolute_max`` argument was added. diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index 2ff09c69b0..03f485b224 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -136,7 +136,10 @@ File Uploads Forms ~~~~~ -* ... +* The new ``absolute_max`` argument for :func:`.formset_factory`, + :func:`.inlineformset_factory`, and :func:`.modelformset_factory` allows + customizing the maximum number of forms that can be instantiated when + supplying ``POST`` data. See :ref:`formsets-absolute-max` for more details. Generic Views ~~~~~~~~~~~~~ diff --git a/docs/topics/forms/formsets.txt b/docs/topics/forms/formsets.txt index 3b5775cabb..b3e696ed8c 100644 --- a/docs/topics/forms/formsets.txt +++ b/docs/topics/forms/formsets.txt @@ -126,6 +126,38 @@ 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`. +.. _formsets-absolute-max: + +Limiting the maximum number of instantiated forms +================================================= + +.. versionadded:: 3.2 + +The ``absolute_max`` parameter to :func:`.formset_factory` allows limiting the +number of forms that can be instantiated when supplying ``POST`` data. This +protects against memory exhaustion attacks using forged ``POST`` requests:: + + >>> from django.forms.formsets import formset_factory + >>> from myapp.forms import ArticleForm + >>> ArticleFormSet = formset_factory(ArticleForm, absolute_max=1500) + >>> data = { + ... 'form-TOTAL_FORMS': '1501', + ... 'form-INITIAL_FORMS': '0', + ... 'form-MAX_NUM_FORMS': '', + ... } + >>> formset = ArticleFormSet(data) + >>> len(formset.forms) + 1500 + >>> formset.is_valid() + False + >>> formset.non_form_errors() + ['Please submit 1000 or fewer forms.'] + +When ``absolute_max`` is None, it defaults to ``max_num + 1000``. (If +``max_num`` is ``None``, it defaults to ``2000``). + +If ``absolute_max`` is less than ``max_num``, a ``ValueError`` will be raised. + Formset validation ================== @@ -348,11 +380,11 @@ excessive. .. 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. + exceeds ``absolute_max``, then the form will fail to validate as if + ``validate_max`` were set, and additionally only the first ``absolute_max`` + forms will be validated. The remainder will be truncated entirely. This is + to protect against memory exhaustion attacks using forged POST requests. + See :ref:`formsets-absolute-max`. ``validate_min`` ---------------- diff --git a/tests/forms_tests/tests/test_formsets.py b/tests/forms_tests/tests/test_formsets.py index 117b50fa7c..e44b347802 100644 --- a/tests/forms_tests/tests/test_formsets.py +++ b/tests/forms_tests/tests/test_formsets.py @@ -892,6 +892,55 @@ class FormsFormsetTestCase(SimpleTestCase): ) self.assertEqual(formset.absolute_max, 2000) + def test_absolute_max(self): + data = { + 'form-TOTAL_FORMS': '2001', + 'form-INITIAL_FORMS': '0', + 'form-MAX_NUM_FORMS': '0', + } + AbsoluteMaxFavoriteDrinksFormSet = formset_factory( + FavoriteDrinkForm, + absolute_max=3000, + ) + formset = AbsoluteMaxFavoriteDrinksFormSet(data=data) + self.assertIs(formset.is_valid(), True) + self.assertEqual(len(formset.forms), 2001) + # absolute_max provides a hard limit. + data['form-TOTAL_FORMS'] = '3001' + formset = AbsoluteMaxFavoriteDrinksFormSet(data=data) + self.assertIs(formset.is_valid(), False) + self.assertEqual(len(formset.forms), 3000) + self.assertEqual( + formset.non_form_errors(), + ['Please submit 1000 or fewer forms.'], + ) + + def test_absolute_max_with_max_num(self): + data = { + 'form-TOTAL_FORMS': '1001', + 'form-INITIAL_FORMS': '0', + 'form-MAX_NUM_FORMS': '0', + } + LimitedFavoriteDrinksFormSet = formset_factory( + FavoriteDrinkForm, + max_num=30, + absolute_max=1000, + ) + formset = LimitedFavoriteDrinksFormSet(data=data) + self.assertIs(formset.is_valid(), False) + self.assertEqual(len(formset.forms), 1000) + self.assertEqual( + formset.non_form_errors(), + ['Please submit 30 or fewer forms.'], + ) + + def test_absolute_max_invalid(self): + msg = "'absolute_max' must be greater or equal to 'max_num'." + for max_num in [None, 31]: + with self.subTest(max_num=max_num): + with self.assertRaisesMessage(ValueError, msg): + formset_factory(FavoriteDrinkForm, max_num=max_num, absolute_max=30) + def test_more_initial_form_result_in_one(self): """ One form from initial and extra=3 with max_num=2 results in the one diff --git a/tests/model_formsets/tests.py b/tests/model_formsets/tests.py index 90e5cb1bd8..8c6d87e59c 100644 --- a/tests/model_formsets/tests.py +++ b/tests/model_formsets/tests.py @@ -1838,3 +1838,81 @@ class TestModelFormsetOverridesTroughFormMeta(TestCase): form = BookFormSet.form(data={'title': 'Foo ' * 30, 'author': author.id}) self.assertIs(Book._meta.get_field('title').__class__, models.CharField) self.assertIsInstance(form.fields['title'], forms.SlugField) + + def test_modelformset_factory_absolute_max(self): + AuthorFormSet = modelformset_factory(Author, fields='__all__', absolute_max=1500) + data = { + 'form-TOTAL_FORMS': '1501', + 'form-INITIAL_FORMS': '0', + 'form-MAX_NUM_FORMS': '0', + } + formset = AuthorFormSet(data=data) + self.assertIs(formset.is_valid(), False) + self.assertEqual(len(formset.forms), 1500) + self.assertEqual( + formset.non_form_errors(), + ['Please submit 1000 or fewer forms.'], + ) + + def test_modelformset_factory_absolute_max_with_max_num(self): + AuthorFormSet = modelformset_factory( + Author, + fields='__all__', + max_num=20, + absolute_max=100, + ) + data = { + 'form-TOTAL_FORMS': '101', + 'form-INITIAL_FORMS': '0', + 'form-MAX_NUM_FORMS': '0', + } + formset = AuthorFormSet(data=data) + self.assertIs(formset.is_valid(), False) + self.assertEqual(len(formset.forms), 100) + self.assertEqual( + formset.non_form_errors(), + ['Please submit 20 or fewer forms.'], + ) + + def test_inlineformset_factory_absolute_max(self): + author = Author.objects.create(name='Charles Baudelaire') + BookFormSet = inlineformset_factory( + Author, + Book, + fields='__all__', + absolute_max=1500, + ) + data = { + 'book_set-TOTAL_FORMS': '1501', + 'book_set-INITIAL_FORMS': '0', + 'book_set-MAX_NUM_FORMS': '0', + } + formset = BookFormSet(data, instance=author) + self.assertIs(formset.is_valid(), False) + self.assertEqual(len(formset.forms), 1500) + self.assertEqual( + formset.non_form_errors(), + ['Please submit 1000 or fewer forms.'], + ) + + def test_inlineformset_factory_absolute_max_with_max_num(self): + author = Author.objects.create(name='Charles Baudelaire') + BookFormSet = inlineformset_factory( + Author, + Book, + fields='__all__', + max_num=20, + absolute_max=100, + ) + data = { + 'book_set-TOTAL_FORMS': '101', + 'book_set-INITIAL_FORMS': '0', + 'book_set-MAX_NUM_FORMS': '0', + } + formset = BookFormSet(data, instance=author) + self.assertIs(formset.is_valid(), False) + self.assertEqual(len(formset.forms), 100) + self.assertEqual( + formset.non_form_errors(), + ['Please submit 20 or fewer forms.'], + )