Opened 8 years ago

Closed 8 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


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) (645 bytes) - added by dtrebbien 8 years ago.
16684.diff (1.8 KB) - added by dtrebbien 8 years ago.
Patch with regression test

Download all attachments as: .zip

Change History (5)

Changed 8 years ago by dtrebbien

Attachment: added


comment:1 Changed 8 years ago by Aymeric Augustin

Needs tests: set
Triage Stage: UnreviewedAccepted

Changed 8 years ago by dtrebbien

Attachment: 16684.diff added

Patch with regression test

comment:2 Changed 8 years ago by Mark Lavin

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 Changed 8 years ago by Karen Tracey

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