Opened 11 years ago

Closed 4 years ago

Last modified 4 years ago

#20347 closed New feature (fixed)

Add an absolute_max parameter to formset_factory

Reported by: Carsten Fuchs Owned by: David Smith
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The documentation at https://docs.djangoproject.com/en/1.5/topics/forms/formsets/#limiting-the-maximum-number-of-forms seems to indicate (if I understood it correctly) that the purpose of the max_num parameter is to prevent that someone sends a manipulated, excessively large value in the hidden form field that states the number of (extra) forms that are submitted, whereas it is not (directly) related to the number of forms that are actually POSTed, or initialized via parameter initials.

However, following the example at that page, with MyInitials being a list of e.g. 1500 initial values and request.POST containing more than 1500 formsets:

>>> ArticleFormSet = formset_factory(ArticleForm, extra=0)
>>> formset1 = ArticleFormSet(initial=MyInitials)
>>> formset2 = ArticleFormSet(request.POST)

Now, accessing formset1.forms[1000] throws an IndexError exception.

The max_num is at its default value of 1000, but in the above context, it is not expected that formset1 or formset2 is reduced to max_num forms -- I'd have expected each to have the full number of forms as initialized.

Related thread at django-users: http://thread.gmane.org/gmane.comp.python.django.user/152946

Change History (14)

comment:1 by Tim Graham, 11 years ago

Component: FormsDocumentation
Easy pickings: set
Summary: Creating a formset with more than max_num formsNote that initializing a formset will be limited by max_num
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

I believe the code is working as intended, accepting as a documentation clarification.

comment:2 by anonymous, 11 years ago

Clarifying the documentation would certainly help, but doesn't this mean that we have to use max_num=len(MyInitials) as a "permanent" work-around in user code?

ArticleFormSet = formset_factory(ArticleForm, max_num=len(MyInitials), extra=0)

comment:3 by Carsten Fuchs, 11 years ago

(Sorry, the above comment was by me, but I realized too late that I wasn't logged in when submitting.)

comment:4 by Carl Meyer, 11 years ago

Component: DocumentationForms
Type: Cleanup/optimizationBug

Yes, this is actually a bug, and it is already fixed in master as part of the fix for #20084. absolute_max, which unlike max_num is a hard limit on the number of forms instantiated, in Django 1.5.1 is set to max(1000, max_num), which means that if max_num >= 1000, then absolute_max == max_num, meaning max_num essentially becomes a hard limit (which it is not intended or documented to be).

Even with the fix for #20084, where absolute_max is set to max_num + 1000 instead, it would still be possible for someone to hit this issue if they instantiate a formset with over 1000 more initial forms than max_num. Given that it seems people are using formsets with higher numbers of forms than anticipated, I think we should probably go further than #20084 did and fix this fully, by documenting absolute_max and making it explicitly configurable in formset_factory.

The other question is whether this deserves a backport to 1.5.X. I think it probably does, given that it could be a data-loss situation.

comment:5 by ethurgood, 11 years ago

Owner: changed from nobody to ethurgood
Status: newassigned

comment:6 by Tim Graham, 11 years ago

Easy pickings: unset
Has patch: set
Patch needs improvement: set
Summary: Note that initializing a formset will be limited by max_numAdd an absolute_max parameter to formset_factory
Type: BugNew feature

Pull request which has some minor comments from Carl.

comment:7 by David Smith, 4 years ago

Owner: changed from ethurgood to David Smith
Patch needs improvement: unset

comment:8 by Mariusz Felisiak, 4 years ago

Version: 1.5master

comment:9 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:10 by David Smith, 4 years ago

Patch needs improvement: unset

comment:11 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In b5aa9cb2:

Refs #20347 -- Added test for formset_factory()'s absolute_max default.

Co-authored-by: ethurgood <ethurgood@…>

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 433dd73:

Fixed #20347 -- Allowed customizing the maximum number of instantiated forms in formsets.

Co-authored-by: ethurgood <ethurgood@…>

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 3254991:

Refs #20347 -- Allowed customizing the maximum number of instantiated forms in generic_inlineformset_factory().

Follow up to 433dd737f94b09043f64b873b0ac067b3f97364b.

Note: See TracTickets for help on using tickets.
Back to Top