Opened 11 years ago

Closed 2 years ago

#20296 closed New feature (fixed)

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

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

Description

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 }}')
activate('fr')
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: https://github.com/bmispelon/django/compare/lazy-safedata

Change History (14)

comment:1 by Baptiste Mispelon, 11 years ago

Cc: bmispelon@… added

comment:2 by fon, 11 years ago

Triage Stage: UnreviewedAccepted

comment:3 by fon, 11 years ago

Triage Stage: AcceptedReady for checkin

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

comment:4 by Baptiste Mispelon, 11 years ago

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]

[1] https://github.com/django/django/blob/master/django/utils/functional.py#L106

comment:5 by Claude Paroz <claude@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 2ee447fb5f8974b432d3dd421af9a242215aea44:

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

comment:6 by Baptiste Mispelon <bmispelon@…>, 10 years ago

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 by Baptiste Mispelon, 10 years ago

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

A better fix for the issue is here: https://github.com/django/django/pull/2234

comment:8 by Tim Graham, 10 years ago

Patch needs improvement: set

The current PR does not merge cleanly.

comment:9 by Baptiste Mispelon, 10 years ago

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 by Tim Graham, 7 years ago

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.

comment:11 by Theofilos Alexiou, 2 years ago

Owner: changed from nobody to Theofilos Alexiou
Status: newassigned

This should be an easy fix now I believe. mark_safe no longer operates on both bytes and text, so wrapping it with keep_lazy (the new allow_lazy) should solve the issue.

comment:12 by Theofilos Alexiou, 2 years ago

Has patch: set

comment:13 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 659d2421:

Fixed #20296 -- Prevented mark_safe() from evaluating lazy objects.

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