Code

Opened 14 months ago

Last modified 10 months ago

#20347 assigned New feature

Add an absolute_max parameter to formset_factory

Reported by: CarstenF Owned by: ethurgood
Component: Forms Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
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

Attachments (0)

Change History (6)

comment:1 Changed 13 months ago by timo

  • Component changed from Forms to Documentation
  • Easy pickings set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Creating a formset with more than max_num forms to Note that initializing a formset will be limited by max_num
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

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

comment:2 Changed 13 months ago by anonymous

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 Changed 13 months ago by CarstenF

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

comment:4 Changed 13 months ago by carljm

  • Component changed from Documentation to Forms
  • Type changed from Cleanup/optimization to Bug

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 Changed 10 months ago by ethurgood

  • Owner changed from nobody to ethurgood
  • Status changed from new to assigned

comment:6 Changed 10 months ago by timo

  • Easy pickings unset
  • Has patch set
  • Patch needs improvement set
  • Summary changed from Note that initializing a formset will be limited by max_num to Add an absolute_max parameter to formset_factory
  • Type changed from Bug to New feature

Pull request which has some minor comments from Carl.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from ethurgood to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.