NewForms labels are not conditionally escaped

[Paul Hummer]

Consider the following code:

required = '<span class="required">*</span>'required = '<span class="required">*</span>'
class FooForm(forms.Form):
    email = forms.EmailField(label='%sEmail Address' % required)
    username = forms.CharField(label='%sUsername' % required)
    password = forms.CharField(label='%sPassword' % required, widget=forms.PasswordInput)
    password2 = forms.CharField(label='%sPassword (Again)' % required,
      widget=forms.PasswordInput)
    firstname = forms.CharField(label='First Name')
    lastname = forms.CharField(label='Last Name')

The labels are currently being escaped. Considering that the labels are usually developer/designer created instead of user created, it's probably safe to assume that most times, they are safe from XSS attacks.

[Paul Hummer]

Patch to fix issue 6283

[Chris Beaven]

The conditional_escape-ing is good, but it shouldn't be marked safe by default. People (like you) can do that in your models if you need it.

[Chris Beaven]

in your form definitions I mean :)

[Paul Hummer]

Wow, I guess I haven't been getting email notifications. Came back because the bug still manifests itself, and it's my fault! Submitted a new patch so that fields can now be passed a safe parameter so as not to escape the label if set to True.

For instance:

name = forms.CharField(label='<span class="foo">Name</span>', safe=True)

[Chris Beaven]

I don't like the new patch at all.

Like I said earlier, all we need is the conditional_escape bit. Then in your code you'd just do:

name = forms.CharField(label=mark_safe('<span class="foo">Name</span>'))

[Paul Hummer]

While I personally don't agree at all with the method for which this is marked safe (adding an interface to the framework instead of pulling in part of the framework to work with code seems messy to me), all I really care about is getting a fix into Django, instead of trying to get it fixed. Obviously, having markup in there is not ideal, but it's still a bug regardless.

However, I've fixed the patch to require mark_safe() in the label

[Chris Beaven]

We decided to push it back to design decision after a chat on irc - but I didn't realise during the discussion that it already escaped it (the design decision was whether it should be escaped in the first place).

Latest patch is now good, but could still do with some tests.

[Julien Phalip]

Technically a new feature.

[bedmondmark]

Sorry, I resolved this ticket as anonymous without a comment - I'm a noob :)

The changes from this patch are already in the trunk, and tests can be found in forms.py (tests_escaping() method).