[Django] #23831: mark_safe and mark_for_escaping should account for __html__
Django
-------------------------------------+------------------------
Reporter: aaugustin | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23831>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* owner: nobody => aaugustin
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23831#comment:1>
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
This change makes sense to me, but I don't feel like I'm much of an expert
here.
--
Ticket URL: <https://code.djangoproject.com/ticket/23831#comment:2>
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
* version: master => 1.7
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23831#comment:3>
Django
Reporter: aaugustin | Owner: aaugustin
Component: Utilities | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6d52f6f8e688b5c4e70be8352eb02c05fea60e85"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23831#comment:4>
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: closed
Component: Utilities | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"3483682749577b4b5a8141a766489d5b460e30e9"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23831#comment:5>