Skip to content

Commit

Permalink
Fixed #20084 -- Provided option to validate formset max_num on server.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
andrewsg authored and carljm committed Mar 21, 2013
1 parent aaec4f2 commit f9ab543
Show file tree
Hide file tree
Showing 12 changed files with 210 additions and 47 deletions.
5 changes: 3 additions & 2 deletions django/contrib/contenttypes/generic.py
Expand Up @@ -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.
Expand All @@ -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
Expand Down
30 changes: 20 additions & 10 deletions django/forms/formsets.py
Expand Up @@ -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)

Expand Down Expand Up @@ -94,7 +97,11 @@ def management_form(self):
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
Expand All @@ -113,14 +120,13 @@ def initial_form_count(self):
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):
Expand Down Expand Up @@ -168,7 +174,6 @@ def empty_form(self):
self.add_fields(form, None)
return form

# Maybe this should just go away?
@property
def cleaned_data(self):
"""
Expand Down Expand Up @@ -294,8 +299,11 @@ def full_clean(self):
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)
Expand Down Expand Up @@ -367,16 +375,18 @@ def as_ul(self):
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):
Expand Down
8 changes: 5 additions & 3 deletions django/forms/models.py
Expand Up @@ -682,15 +682,16 @@ def pk_is_not_editable(pk):
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.
"""
form = modelform_factory(model, form=form, fields=fields, exclude=exclude,
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

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/ref/contrib/contenttypes.txt
Expand Up @@ -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`.
Expand Down
16 changes: 16 additions & 0 deletions 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.
1 change: 1 addition & 0 deletions docs/ref/forms/index.txt
Expand Up @@ -10,5 +10,6 @@ Detailed form API reference. For introductory material, see :doc:`/topics/forms/
api
fields
models
formsets
widgets
validation
15 changes: 8 additions & 7 deletions docs/ref/forms/models.txt
Expand Up @@ -25,25 +25,26 @@ 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.

Arguments ``model``, ``form``, ``fields``, ``exclude``,
``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
Expand All @@ -56,4 +57,4 @@ Model Form Functions

.. versionchanged:: 1.6

The widgets parameter was added.
The ``widgets`` and the ``validate_max`` parameters were added.
8 changes: 8 additions & 0 deletions docs/releases/1.6.txt
Expand Up @@ -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
=====================================

Expand Down
78 changes: 69 additions & 9 deletions docs/topics/forms/formsets.txt
Expand Up @@ -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)

Expand Down Expand Up @@ -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()
Expand All @@ -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
------------------

Expand Down Expand Up @@ -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``
~~~~~~~~~~~~~
Expand Down
7 changes: 4 additions & 3 deletions docs/topics/forms/modelforms.txt
Expand Up @@ -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
---------------------
Expand Down

0 comments on commit f9ab543

Please sign in to comment.