Commit f9ab5437 authored by Andrew Gorcester's avatar Andrew Gorcester Committed by Carl Meyer
Browse files

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.
parent aaec4f2b

django/contrib/contenttypes/generic.py

+3 −2
Original line number Diff line number Diff line
@@ -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

django/forms/formsets.py

+20 −10
Original line number Diff line number Diff line
@@ -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):

django/forms/models.py

+5 −3
Original line number Diff line number Diff line
@@ -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

docs/ref/contrib/contenttypes.txt

+1 −1
Original line number Diff line number Diff line
@@ -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`.

docs/ref/forms/formsets.txt

0 → 100644
+16 −0
Original line number Diff line number Diff line 
====================

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.