Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#23831 closed Bug (fixed)

mark_safe and mark_for_escaping should account for __html__

Reported by: Aymeric Augustin Owned by: Aymeric Augustin
Component: Utilities Version: 1.7
Severity: Normal Keywords:
Cc: 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

The changes made #7261 aren't complete. They made it possible for other libraries to interpret strings marked explicitly as safe or unsafe in Django. However Django doesn't always interpret correctly strings marked by other libraries.

The reason is that mark_safe and mark_for_escaping don't account for __html__. As a consequence escaping information is lost once these functions get called, and they're called in many places.

Thanks mitsuhiko for the report.

Change History (5)

comment:1 by Aymeric Augustin, 10 years ago

Owner: changed from nobody to Aymeric Augustin
Status: newassigned

comment:2 by Collin Anderson, 10 years ago

This change makes sense to me, but I don't feel like I'm much of an expert here.

comment:3 by Tim Graham, 10 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin
Version: master1.7

comment:4 by Aymeric Augustin <aymeric.augustin@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 6d52f6f8e688b5c4e70be8352eb02c05fea60e85:

Fixed #23831 -- Supported strings escaped by third-party libs in Django.

Refs #7261 -- Made strings escaped by Django usable in third-party libs.

The changes in mark_safe and mark_for_escaping are straightforward. The
more tricky part is to handle correctly objects that implement html.

Historically escape() has escaped SafeData. Even if that doesn't seem a
good behavior, changing it would create security concerns. Therefore
support for html() was only added to conditional_escape() where this
concern doesn't exist.

Then using conditional_escape() instead of escape() in the Django
template engine makes it understand data escaped by other libraries.

Template filter |escape accounts for html() when it's available.
|force_escape forces the use of Django's HTML escaping implementation.

Here's why the change in render_value_in_context() is safe. Before Django
1.7 conditional_escape() was implemented as follows:

if isinstance(text, SafeData):

return text

else:

return escape(text)

render_value_in_context() never called escape() on SafeData. Therefore
replacing escape() with conditional_escape() doesn't change the
autoescaping logic as it was originally intended.

This change should be backported to Django 1.7 because it corrects a
feature added in Django 1.7.

Thanks mitsuhiko for the report.

comment:5 by Aymeric Augustin <aymeric.augustin@…>, 10 years ago

In 3483682749577b4b5a8141a766489d5b460e30e9:

[1.7.x] Fixed #23831 -- Supported strings escaped by third-party libs in Django.

Refs #7261 -- Made strings escaped by Django usable in third-party libs.

The changes in mark_safe and mark_for_escaping are straightforward. The
more tricky part is to handle correctly objects that implement html.

Historically escape() has escaped SafeData. Even if that doesn't seem a
good behavior, changing it would create security concerns. Therefore
support for html() was only added to conditional_escape() where this
concern doesn't exist.

Then using conditional_escape() instead of escape() in the Django
template engine makes it understand data escaped by other libraries.

Template filter |escape accounts for html() when it's available.
|force_escape forces the use of Django's HTML escaping implementation.

Here's why the change in render_value_in_context() is safe. Before Django
1.7 conditional_escape() was implemented as follows:

if isinstance(text, SafeData):

return text

else:

return escape(text)

render_value_in_context() never called escape() on SafeData. Therefore
replacing escape() with conditional_escape() doesn't change the
autoescaping logic as it was originally intended.

This change should be backported to Django 1.7 because it corrects a
feature added in Django 1.7.

Thanks mitsuhiko for the report.

Backport of 6d52f6f from master.

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