Revisit strategy of escaping Django's form elements in non-Django forms

[Moritz Sichert]

Django uses django.utils.safestring for marking strings as escaped. This prevents already escaped text to be escaped again.
It also uses the __html__ magic method used by many other web frameworks.

However the information about a string being safe won't be carried on if an object gets converted to a string.
This mostly happens with forms, form fields an the Media class.
The django template backend "knows" about them so it doesn't escape them, however that's not the case with any other backend.

For example

  {{ my_form.my_field }}}

will be rendered as

  <input name=&34;my_field&34; type=&34;text&34; />

when using jinja2 backend.

In my opinion the best way to fix this is to add __html__ methods to the classes that should not be escaped.

[Moritz Sichert]

The pull request contains a regression test and the __html__ methods for Form, BoundField and Media.

[Moritz Sichert]

I looked into django's and jinja2's template code and found out what the problem is:

The django template engine calls django.template.base.render_value_in_context for each variable. There the object gets converted to a string with force_text. That just calls __str__ or __unicode__ of the object and correctly gets a SafeText.

jinja2 however doesn't use force_text or str(), it uses escape from the markupsafe library.
Markupsafe then sees that the form, field or media doesn't have a __html__ method so it decides to mark it unsafe and escape the html characters.

[Aymeric Augustin]

Thanks a lot for doing this research. That's the reason I suspected, but I wasn't sure.

[Tim Graham]

Pending Aymeric's review.

[Aymeric Augustin]

I'm fine with the patch as is. Let's commit it rather than delay RC1.

Like I said on IRC, generally speaking, I'm wondering if adding this __html__ method on a case by case basis to some classes is the best idea in the long run.

Can we identify every class whose __str__ method returns a SafeStr? If there's three of them, perhaps the ad-hoc method is fine. If there's seventeen, then it's a different story.

Turning it into a mixin wouldn't save much code, would add a layer of indirection and a small overhead, but it would also allow us to provide a docstring.

[Tim Graham <timograham@…>]

In 6bff343:

Refs #24469 -- Fixed escaping of forms, fields, and media in non-Django templates.

[Tim Graham <timograham@…>]

In 571e093a:

[1.8.x] Refs #24469 -- Fixed escaping of forms, fields, and media in non-Django templates.

Backport of 6bff3439894ac22d80f270f36513fc86586273f3 from master

[Tim Graham]

Leaving this ticket open at Aymeric's request pending his further investigation.

[Moritz Sichert]

I looked into it and found 8 more classes that return SafeStr in their __str__:
django.contrib.gis.maps.google.overlays.GEvent
django.contrib.gis.maps.google.overlays.GOverlayBase
django.forms.formsets.BaseFormSet
django.forms.utils.ErrorDict
django.forms.utils.ErrorList
django.forms.widgets.SubWidget
django.forms.widgets.ChoiceInput
django.forms.widgets.ChoiceFieldRenderer

I agree that using a mixin would add too much overhead, what about a decorator that adds the __html__ method instead?
Something like:

@html_safe
class MyClass(object):
    def __str__(self):
        ...

[Tim Graham]

Sounds good to me.

[Moritz Sichert]

I added the decorator in a ​new PR.
I'll work on adding it to where it's needed.

[Moritz Sichert]

I added the decorator to the affected classes.
Tests are passing on python 2.7 and 3.4 with selenium.

[Tim Graham]

Reviewed the PR.

[Tim Graham]

Looks good to me. Aymeric, are you happy with the approach? If so, we can squash commits and changed "Refs" to "Fixed" in the commit message when merging.

[Moritz Sichert]

I squashed the commits and tests are passing.

[Aymeric Augustin]

I'm happy with the general approach. It looks appropriate given the number of classes that needed the decorator.

I left two questions on the PR about the implementation of the decorator.

[Tim Graham <timograham@…>]

In 1f2abf7:

Fixed #24469 -- Refined escaping of Django's form elements in non-Django templates.

[Tim Graham <timograham@…>]

In 44a05a8a:

[1.8.x] Fixed #24469 -- Refined escaping of Django's form elements in non-Django templates.

Backport of 1f2abf784a9fe550959de242d91963b2ad6f7e9c from master