Opened 7 days ago
Last modified 3 days ago
#35897 assigned Cleanup/optimization
Template system: escape() calls in get_exception_info() should be removed
Reported by: | Klaas van Schelven | Owned by: | Klaas van Schelven |
---|---|---|---|
Component: | Error reporting | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Keryn Knight, Florian Apolloner | 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
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 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 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).
Attachments (1)
Change History (7)
by , 7 days ago
Attachment: | 2024-11-08_1877x538.png added |
---|
comment:1 by , 7 days ago
Has patch: | set |
---|
comment:2 by , 7 days ago
Cc: | added |
---|---|
Component: | Uncategorized → Error reporting |
Type: | Uncategorized → Cleanup/optimization |
comment:4 by , 6 days ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:5 by , 6 days ago
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.
comment:6 by , 3 days ago
Triage Stage: | Accepted → Ready for checkin |
---|
Given #33461 added
force_escape
tomessage
, we might need to move that escaping into the template