Opened 8 years ago

Closed 7 years ago

#28171 closed Cleanup/optimization (fixed)

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

Reported by: Vlastimil Zíma Owned by: Herbert Fortes
Component: Forms Version: dev
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 (9)

comment:1 by Tim Graham, 8 years ago

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 by Naveen Yadav, 8 years ago

Owner: changed from nobody to Naveen Yadav
Status: newassigned

comment:3 by Naveen Yadav, 8 years ago

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 8 years ago by Naveen Yadav (previous) (diff)

comment:4 by Tim Graham, 8 years ago

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 by Naveen Yadav, 7 years ago

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 by Vlastimil Zíma, 7 years ago

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

comment:7 by Naveen Yadav, 7 years ago

Owner: Naveen Yadav removed
Status: assignednew

comment:8 by Herbert Fortes, 7 years ago

Owner: set to Herbert Fortes
Status: newassigned

Hi,

I created a Pull Request to close this ticket:

https://github.com/django/django/pull/9691

Regards,
Herbert

comment:9 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In d368784:

Fixed #28171 -- Added an exception if Form's empty_permitted and use_required_attribute arguments conflict.

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