[Django] #35897: Template system: escape() calls in get_exception_info() should be removed

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed
-------------------------------------+-------------------------------------
Reporter: Klaas van Schelven | Type:
| Uncategorized
Status: new | Component:
| Uncategorized
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
[https://github.com/django/django/blob/042b381e2e37c0c37b8a8f6cc9947f1a2ebfa0dd/django/template/base.py#L250
Here] there are some calls to `escape()`

They shouldn't be there: escaping happens in templates for non-safe
strings anyway, so there's no need.

And there _is_ a drawback: as an example, the Python Sentry SDK
[https://github.com/getsentry/sentry-
python/blob/200d0cdde8eed2caa89b91db8b17baabe983d2de/sentry_sdk/integrations/django/templates.py#L32
copies this info], but because it gets sent over the wire (as a JSON
string) the information that this has already been escaped is lost, and on
the receiving end it is escaped again.

This means that on the server-side [https://www.bugsink.com/ the Error-
tracking, in my case Bugsink] will show doubly escaped html code snippets.
This looks something like this:


{{{
<p class="relative text-slate-600 text-base md:text-xl mb-4
md:mb-5">
}}}

Removing the calls to `escape` simply solves this. Which makes sense:
calling `escape` is simply not the responsibility of this piece of code,
it should just stay marked as unsafe and be escape at the edges (on
rendering).
--
Ticket URL: <https://code.djangoproject.com/ticket/35897>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed

------------------------------------+--------------------------------------
Reporter: Klaas van Schelven | Owner: (none)

Type: Uncategorized | Status: new
Component: Uncategorized | Version: dev

Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

------------------------------------+--------------------------------------
Changes (by Klaas van Schelven):

* Attachment "2024-11-08_1877x538.png" added.

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed
------------------------------------+--------------------------------------
Reporter: Klaas van Schelven | Owner: (none)
Type: Uncategorized | Status: new
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------------+--------------------------------------
Changes (by Klaas van Schelven):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35897#comment:1>

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed

-------------------------------------+-------------------------------------

Reporter: Klaas van Schelven | Owner: (none)

Type: | Status: new
Cleanup/optimization |
Component: Error reporting | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* cc: Keryn Knight (added)
* component: Uncategorized => Error reporting
* type: Uncategorized => Cleanup/optimization

Comment:

Given #33461 added `force_escape` to `message`, we might need to move that
escaping into the template
--
Ticket URL: <https://code.djangoproject.com/ticket/35897#comment:2>

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed

--------------------------------------+------------------------------------

Reporter: Klaas van Schelven | Owner: (none)

Type: Cleanup/optimization | Status: new

Component: Error reporting | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* cc: Florian Apolloner (added)
* stage: Unreviewed => Accepted

Comment:

Tentatively accepting
--
Ticket URL: <https://code.djangoproject.com/ticket/35897#comment:3>

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed

-------------------------------------+-------------------------------------
Reporter: Klaas van Schelven | Owner: Klaas van
Type: | Schelven
Cleanup/optimization | Status: assigned

Component: Error reporting | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* owner: (none) => Klaas van Schelven
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35897#comment:4>

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed
-------------------------------------+-------------------------------------
Reporter: Klaas van Schelven | Owner: Klaas van
Type: | Schelven
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Klaas van Schelven):

I don't fully understand the linked issue, which puts me in dangerous
waters, but AFAICT in that issue one of the things that was required for
the problem to exist was user input, incorrectly marked as safe. That
would mean it doesn't really apply here, since the thing that was
previously (before my proposed change) being escape was lines of source
code. Presumably not affected by user input, and also not mark-safe'd.
--
Ticket URL: <https://code.djangoproject.com/ticket/35897#comment:5>

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed
-------------------------------------+-------------------------------------
Reporter: Klaas van Schelven | Owner: Klaas van
Type: | Schelven
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: dev
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 Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35897#comment:6>

[Django]

#35897: Template system: escape() calls in get_exception_info() should be removed
-------------------------------------+-------------------------------------
Reporter: Klaas van Schelven | Owner: Klaas van
Type: | Schelven

Cleanup/optimization | Status: closed

Component: Error reporting | Version: dev

Severity: Normal | Resolution: fixed

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 Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"1722f2db5808708de6fc6e0f48af2d518be1e348" 1722f2d]:
{{{#!CommitTicketReference repository=""
revision="1722f2db5808708de6fc6e0f48af2d518be1e348"
Fixed #35897 -- Removed unnecessary escaping in template's
get_exception_info().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35897#comment:7>