Opened 7 months ago

Last modified 5 weeks ago

#28171 new Cleanup/optimization

Raise an exception if Form's empty_permitted and use_required_attribute arguments conflict

Reported by: Vlastimil Zíma Owned by:
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It is possible to easily create a form which does not provide expected behavior. If a form is created with empty_permitted=True, it still renders a required attribute into its inputs, although it clearly shouldn't.

from django import forms

class FooForm(forms.Form):
     foo = forms.CharField()

unicode(FooForm(empty_permitted=True))
>>> u'<tr><th><label for="id_foo">Foo:</label></th><td><input id="id_foo" name="foo" type="text" required /></td></tr>'

I suggest to disable use_required_attribute if empty_permitted is enabled and/or raise exception if both attributes are set up contradictory. Even though empty_permitted is not a documented attribute, incorrect usage should be limited if possible.

Change History (7)

comment:1 Changed 7 months ago by Tim Graham

Summary: empty_permitted=True contradicts use_required_attribute=TrueRaise an exception if Form's empty_permitted and use_required_attribute arguments conflict
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Raising an exception if the arguments are in conflict seems like the best approach to me.

comment:2 Changed 7 months ago by Naveen Yadav

Owner: changed from nobody to Naveen Yadav
Status: newassigned

comment:3 Changed 7 months ago by Naveen Yadav

Hi,

working on my first ticket on django !!

i am raising the ValidationError from [https://github.com/django/django/blob/master/django/forms/forms.py#L418 for concerned issue.

for that couple of test cases are failing because of following :

creating a formset on certain https://github.com/django/django/blob/master/django/forms/formsets.py#L170 set empty_permitted to True.
Not sure should we allow error in this case also.

Any pointers will be appreciated.

Thanks.

Last edited 7 months ago by Naveen Yadav (previous) (diff)

comment:4 Changed 7 months ago by Tim Graham

I expect a ValueError to be raised in BaseForm.__init__() if the values of empty_permitted and use_required_attribute are incompatible. This is a programming error, not a validation error that the user can do anything about.

comment:5 Changed 7 months ago by Naveen Yadav

Thanks for suggestion Tim.

I tried raising ValueError which causes lot of tests to fail because of this linehttps://github.com/django/django/blob/master/django/forms/formsets.py#L172 all related to formset. I guess when forms are created through formset both values empty_permitted and use_required_attribute are same.

Should i place check if form(s) is created via formset then error should not be raised ?

comment:6 Changed 7 months ago by Vlastimil Zíma

If formset create form with empty_permitted, it has to set use_required_attribute to False as well.

comment:7 Changed 5 weeks ago by Naveen Yadav

Owner: Naveen Yadav deleted
Status: assignednew
Note: See TracTickets for help on using tickets.
Back to Top