Opened 2 years ago

Last modified 11 months ago

#20296 new New feature

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

Reported by: bmispelon Owned by: nobody
Component: Utilities Version: master
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

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 (9)

comment:1 Changed 2 years ago by bmispelon

  • Cc bmispelon@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 2 years ago by fon

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 2 years ago by fon

  • Triage Stage changed from Accepted to Ready for checkin

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

comment:4 Changed 2 years ago by bmispelon

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 Changed 2 years ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 2ee447fb5f8974b432d3dd421af9a242215aea44:

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

comment:6 Changed 17 months 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 17 months ago by bmispelon

  • Resolution fixed deleted
  • Status changed from closed to new
  • Triage Stage changed from Ready for checkin to Accepted

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

comment:8 Changed 13 months ago by timo

  • Patch needs improvement set

The current PR does not merge cleanly.

comment:9 Changed 11 months ago by bmispelon

  • 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.

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