Opened 6 years ago

Closed 6 years ago

Last modified 6 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 Changed 6 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin
Status: newassigned

comment:2 Changed 6 years ago by Collin Anderson

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

comment:3 Changed 6 years ago by Tim Graham

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

comment:4 Changed 6 years ago by Aymeric Augustin <aymeric.augustin@…>

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 Changed 6 years ago by Aymeric Augustin <aymeric.augustin@…>

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