Early modification bug in Django 1.6. with mark_safe + ugettext_lazy

[Neal Todd]

Hi,

I'm in the process of upgrading a Django application from 1.5 to 1.6 and I've come across a mark_safe + ugettext_lazy issue in form field labels that I think might be a bug introduced in 1.6 (possibly related to the introduction of the new Form.label_suffix?).

I reported this on django-users first to see if people thought it is a bug but had no responses.

In essence, given a label that is a translatable string containing HTML and wrapped in mark_safe, e.g. mark_safe(_("<em>Foo</em>"), calling label_tag on the field in a template:

in Django 1.5 results in a non-escaped string: <em>Foo</em>
in Django 1.6 results in an escaped string: <em>Foo</em>:

(including the default label suffix of ":" for 1.6).

I was expecting 1.6 to render: <em>Foo</em>:

It seems as though the SafeString is being modified at some point and becoming unsafe again (that may mean it's not restricted to label_tag).

Here's a shell example for 1.5 and 1.6 that distills what I'm getting:

>>> import django
>>> django.get_version()
'1.5.5'
>>> from django import forms
>>> from django.template import Template, Context
>>> from django.utils.safestring import mark_safe
>>> from django.utils.translation import ugettext_lazy as _
>>> class FooForm(forms.Form):
...     foo = forms.IntegerField(label=mark_safe(_("<em>Foo</em>")))
... 
>>> fooForm = FooForm()
>>> fooForm.fields['foo'].label
u'<em>Foo</em>'
>>> Template("{{ f }}").render(Context({'f': fooForm}))
u'<tr><th><label for="id_foo"><em>Foo</em>:</label></th><td><input id="id_foo" name="foo" type="text" /></td></tr>'
>>> Template("{{ f.foo.label }}").render(Context({'f': fooForm}))
u'<em>Foo</em>'
>>> Template("{{ f.foo.label_tag }}").render(Context({'f': fooForm}))
u'<label for="id_foo"><em>Foo</em></label>'

>>> import django
>>> django.get_version()
'1.6.1'
>>> from django import forms
>>> from django.template import Template, Context
>>> from django.utils.safestring import mark_safe
>>> from django.utils.translation import ugettext_lazy as _
>>> class FooForm(forms.Form):
...     foo = forms.IntegerField(label=mark_safe(_("<em>Foo</em>")))
... 
>>> fooForm = FooForm()
>>> fooForm.fields['foo'].label
<django.utils.functional.__proxy__ object at 0xabba38c>
>>> Template("{{ f }}").render(Context({'f': fooForm}))
u'<tr><th><label for="id_foo"><em>Foo</em>:</label></th><td><input id="id_foo" name="foo" type="number" /></td></tr>'
>>> Template("{{ f.foo.label }}").render(Context({'f': fooForm}))
u'<em>Foo</em>'
>>> Template("{{ f.foo.label_tag }}").render(Context({'f': fooForm}))
u'<label for="id_foo">&lt;em&gt;Foo&lt;/em&gt;:</label>'

It may or may not be related but when I tried delaying the translation as described in ​https://docs.djangoproject.com/en/dev/topics/i18n/translation/#other-uses-of-lazy-in-delayed-translations I got a TypeError under 1.6 when passing the lazy object to ugettext:

>>> import django
>>> django.get_version()
'1.5.5'
>>> from django.utils import six  # Python 3 compatibility
>>> from django.utils.functional import lazy
>>> from django.utils.safestring import mark_safe
>>> from django.utils.translation import ugettext_lazy as _
>>> mark_safe_lazy = lazy(mark_safe, six.text_type)
>>> lazy_string = mark_safe_lazy(_("<p>My <strong>string!</strong></p>"))
>>> lazy_string
<django.utils.functional.__proxy__ object at 0x9a0052c>
>>> from django.utils.translation import ugettext
>>> ugettext(lazy_string)
u'<p>My <strong>string!</strong></p>'

>>> import django
>>> django.get_version()
'1.6.1'
>>> from django.utils import six  # Python 3 compatibility
>>> from django.utils.functional import lazy
>>> from django.utils.safestring import mark_safe
>>> from django.utils.translation import ugettext_lazy as _
>>> mark_safe_lazy = lazy(mark_safe, six.text_type)
>>> lazy_string = mark_safe_lazy(_("<p>My <strong>string!</strong></p>"))
>>> lazy_string
<django.utils.functional.__proxy__ object at 0xb00b24c>
>>> from django.utils.translation import ugettext
>>> ugettext(lazy_string)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File ".../lib/python2.7/site-packages/django/utils/translation/__init__.py", line 76, in ugettext
    return _trans.ugettext(message)
  File ".../lib/python2.7/site-packages/django/utils/translation/trans_real.py", line 281, in ugettext
    return do_translate(message, 'ugettext')
  File ".../lib/python2.7/site-packages/django/utils/translation/trans_real.py", line 256, in do_translate
    eol_message = message.replace(str('\r\n'), str('\n')).replace(str('\r'), str('\n'))
  File ".../lib/python2.7/site-packages/django/utils/functional.py", line 129, in __wrapper__
    raise TypeError("Lazy object returned unexpected type.")
TypeError: Lazy object returned unexpected type.

Any ideas as to whether it may be a bug, or whether I'm just doing it wrong and 1.6 has exposed that?

Cheers, Neal

[Baptiste Mispelon]

Hi,

As I suspected, this is probably my fault.
I traced the regression back to 2ee447fb5f8974b432d3dd421af9a242215aea44.

I'll look into it.

Thanks.

[Baptiste Mispelon]

I've looked at this for a bit and my brain has started to melt so I'll take a short break.

I think there's a general problem in that SafeData (the class that allows auto-escaping in Django) and lazy objects are sort of incompatible in the current state of things (see also #20017, #21221, #20222, and #20223).

It's possible to work around this issue and for example, this ticket can simply be solved with the following patch:

django/utils/html.py

@@ -67 +67 @@
     """
     Similar to escape(), except that it doesn't operate on pre-escaped strings.
     """
+    text = force_text(text)
     if hasattr(text, '__html__'):
         return text.__html__()
     else:

I'll try to see if I can come up with a better fix than that and if not, I'll commit this (with some regression tests) in the next few days.
I'll also be backporting the fix to the 1.6 branch since this is a regression.

I'm going to mark the ticket as a release blocker just so I don't forget.

Thanks for catching this.

[Neal Todd]

Hi Baptiste,

Thanks for looking into that (glad it wasn't just my brain melting ;-).

I've tried the force_text coercion at the start of the 1.6 implementation of conditional_escape (different from the 1.7 implementation) and that's resolving the issues I was seeing. So, thanks for doing the backport too.

Cheers, Neal

[Neal Todd]

I forgot to ask - do you think this'll get into the 1.6.2 bugfix release?

[Baptiste Mispelon]

Well, this turned out to be more complicated than what I originally thought.

Calling force_text inside the function works but only because it removes the lazy property of the string which is not really what you want.

What's needed instead is a way to mark Promise objects as safe but while still delaying their evaluation.
I achieved this by creating subclasses of both Promise and SafeData and the result seems to be working (all the test pass and the regression test I added is fixed).

Here's the pull request: ​https://github.com/django/django/pull/2234/files
Reviews welcome.

Hopefully this should make it into the upcoming 1.6.2 release, provided I can get another set of eyes on the patch to make sure it's not doing anything too crazy.

[Baptiste Mispelon <bmispelon@…>]

In a878bf9b093bf15d751b070d132fec52a7523a47:

Revert "Fixed #20296 -- Allowed SafeData and EscapeData to be lazy"

This reverts commit 2ee447fb5f8974b432d3dd421af9a242215aea44.

That commit introduced a regression (#21882) and didn't really
do what it was supposed to: while it did delay the evaluation
of lazy objects passed to mark_safe(), they weren't actually
marked as such so they could end up being escaped twice.

Refs #21882.

[Baptiste Mispelon]

As noted by the comment above, the regression was fixed by reverting the commit that introduced it.

I also backported the fix to the 1.6.x branch so it will be part of the upcoming 1.6.2 release.

Note that using lazy objects with mark_safe doesn't really work since mark_safe will force their evaluation (this is what #20296 was trying to fix).