Code

Opened 3 years ago

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

Download all attachments as: .zip

Change History (5)

Changed 3 years ago by dtrebbien

Patch

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 3 years ago by dtrebbien

Patch with regression test

comment:2 Changed 3 years ago by mlavin

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 3 years ago by kmtracey

  • Resolution set to wontfix
  • Status changed from new to closed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.