Opened 9 years ago

Last modified 5 years ago

#20296 new New feature

django.utils.safestring.mark_safe forces evaluation of lazy objects

Reported by: Baptiste Mispelon Owned by: nobody
Component: Utilities Version: dev
Severity: Normal Keywords:
Cc: bmispelon@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Consider the following example:

from django.utils.safestring import mark_safe
from django.utils.translation import activate, ugettext_lazy as _

s = mark_safe(_("username"))
tpl = Template('{{ s }}')
print(tpl.render(Context({'s': s})))

I would expect this to output nom d'utilisateur (which is the french translation of username) but what happens instead is that it outputs username.
The reason for this is that mark_safe will force the evaluation of the lazy string provided by ugettext_lazy when it's called.

Unfortunately, the solution to this it trickier than simply wrapping mark_safe with django.utils.functional.allow_lazy, because mark_safe can operate both on bytes and text (and allow_lazy needs to know the type of object return by the wrapped function).

I wrote some tests and a proposed solution on my branch:

Change History (10)

comment:1 Changed 9 years ago by Baptiste Mispelon

Cc: bmispelon@… added

comment:2 Changed 9 years ago by fon

Triage Stage: UnreviewedAccepted

comment:3 Changed 9 years ago by fon

Triage Stage: AcceptedReady for checkin

I verified the problem exists.
The patch fixes the problem, and has tests.

comment:4 Changed 9 years ago by Baptiste Mispelon

Since it might not be clear, I'd like to point that the reason we can't simply decorate mark_safe with allow_lazy is that mark_safe can return either bytes or text.

The allow_lazy decorator cannot handle this case (there are specific checks in the code for it). [1]


comment:5 Changed 9 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 2ee447fb5f8974b432d3dd421af9a242215aea44:

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

comment:6 Changed 8 years ago by 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.

comment:7 Changed 8 years ago by Baptiste Mispelon

Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

A better fix for the issue is here:

comment:8 Changed 8 years ago by Tim Graham

Patch needs improvement: set

The current PR does not merge cleanly.

comment:9 Changed 7 years ago by Baptiste Mispelon

Has patch: unset
Patch needs improvement: unset

I closed the PR (it is still there for anyone who'd like to see how it looked like).

If I have some time, I'll try to see if the approach still works and I'll reopen it.

Thanks for the ping.

comment:10 Changed 5 years ago by Tim Graham

In the steps to reproduce, should mark_safe() be inside ugettext_lazy() as in #27803 instead of the other way around? If so, maybe this is a wontfix, assuming the documentation is clear about proper usage.

Note: See TracTickets for help on using tickets.
Back to Top