Opened 13 years ago

Closed 13 years ago

#16684 closed Bug (wontfix)

BaseForm needs to escape the 'class' attribute value

Reported by: dtrebbien Owned by: nobody
Component: Forms Version: 1.3
Severity: Normal Keywords:
Cc: dtrebbien@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In the _html_output method of class django.forms.forms.BaseForm, this line:

    html_class_attr = ' class="%s"' % css_classes

needs to be changed to:

    html_class_attr = ' class="%s"' % conditional_escape(css_classes)

This is because CSS identifiers can contain escape characters:

For instance, the identifier "B&W?" may be written as "B\&W\?" or "B\26 W\3F".


Note that the attached patch is against trunk.

Attachments (2)

django_forms_forms.py.diff (645 bytes ) - added by dtrebbien 13 years ago.
Patch
16684.diff (1.8 KB ) - added by dtrebbien 13 years ago.
Patch with regression test

Download all attachments as: .zip

Change History (5)

by dtrebbien, 13 years ago

Attachment: django_forms_forms.py.diff added

Patch

comment:1 by Aymeric Augustin, 13 years ago

Needs tests: set
Triage Stage: UnreviewedAccepted

by dtrebbien, 13 years ago

Attachment: 16684.diff added

Patch with regression test

comment:2 by Mark Lavin, 13 years ago

conditional_escape is not appropriate for this issue and this test is not correct. Using the example from the CSS spec, conditional_escape would convert B&W to B&W not the correct B\&W . The test currently converts the class name \&required to \\&required which is not a valid class name.

To handle this there would need to be another escape function for handling css escaping rules. It seems as though it developers choose to use these characters in their css they can handle the escaping themselves. I don't see why this would need to be a part of Django.

comment:3 by Karen Tracey, 13 years ago

Resolution: wontfix
Status: newclosed

Given the different rules for escaping css classes than regular html escaping, and the fact that css class names are generally going to be under the control of developers and not pulled from user-supplied input, I don't see that escaping css class names is something Django should be doing.

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