[Django] #33405: Documentation for template filter 'escapejs' is extremely unclear
Django
-----------------------------------------+------------------------
Reporter: Jon Ribbens | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 4.0
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 |
-----------------------------------------+------------------------
The documentation says:
Escapes characters for use in JavaScript strings. This does not make
the string safe for use in HTML or JavaScript template literals, but does
protect you from syntax errors when using templates to generate
JavaScript/JSON.
The first sentence is unclear - JavaScript strings ''where''? Inside `on`
attributes? Inside `<script>` blocks?
The second sentence appears entirely meaningless, in that the second half
seems to contradict the first half. If it doesn't "make the string safe",
what ''does'' it do? If it "protects you from syntax errors" then in what
way is it unsafe, and why?
There needs to be an example of what this filter is supposed to be used
for, and an explanation of in which circumstances it is unsafe.
(Also the documentation may say it "does ''not'' make the string safe" but
the code literally does mark the string safe, so...)
As far as I can see it ought to be safe for use in, e.g.:
<script>
const thing = '{{ context_str|escapejs }}'
but I can't tell if the documentation is saying you should do this or you
definitely shouldn't do this (and if not, why not).
--
Ticket URL: <https://code.djangoproject.com/ticket/33405>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Jon Ribbens | Owner: nobody
Component: Documentation | Version: 4.0
Severity: Normal | Resolution: invalid
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 Carlton Gibson):
* cc: Adam Johnson (added)
* status: new => closed
* resolution: => invalid
Comment:
Hi Jon.
Answering your last question first, generally it's not safe, and you
shouldn't do this. `escapejs` is just to put put escape sequences into
strings.
You want to look into the newer
[https://docs.djangoproject.com/en/3.2/ref/templates/builtins/#json-script
json_script] tag.
[https://adamj.eu/tech/2020/02/18/safely-including-data-for-javascript-
in-a-django-template/ Adam Johnson has a good post of this topic a while
back].
I'm going to close, as I think the text is OK... **but** happy to look at
concrete suggestions.
(I also wonder if we might not deprecate `escapejs` as of questionable
value, but perhaps that needs some discussion...)
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:1>
Django
Reporter: Jon Ribbens | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 4.0
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 Jon Ribbens):
* status: closed => new
* resolution: invalid =>
Comment:
Reopening this as the comment doesn't address it at all I'm afraid.
Answering your last question first, generally it's not safe, and you
shouldn't do this. escapejs is just to put put escape sequences into
strings.
What does "put escape sequences into strings" ''mean''? That is precisely
what I was doing in my example above, which you just said was wrong.
As far as I can see, actually it is safe. If it isn't, can anyone explain
why and, more importantly, explain what escapejs ''should'' be used for.
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:2>
Django
Reporter: Jon Ribbens | Owner: nobody
Component: Documentation | Version: 4.0
Severity: Normal | Resolution: invalid
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 Carlton Gibson):
* status: new => closed
* resolution: => invalid
Comment:
This looks like a usage question really. For which see
TicketClosingReasons/UseSupportChannels.
Nonetheless, `escapejs` is used avoid syntax errors when constructing
Javascript using the DTL.
Take something like this:
{{{
<script>
function example() {
query = '{{ my_var | escapejs }}';
}
</script>
}}}
If `my_var` included a single quote `'`, without the `escapejs` you'd get
a
syntax error, since the string would be improperly closed. `escapejs` hex
encodes the `'` leaving a valid string.
In addition it encodes various other characters including `<`, and `>`
which
makes it look like it's good for security, but it's not. See #29055.
The source is in `django.utils.html` is you want to see exactly what's
encoded.
As I said above, I think the current text is ok **but** happy to look at
concrete suggestions for improvements to the docs.
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:3>
Django
Reporter: Jon Ribbens | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 4.0
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 Jon Ribbens):
* status: closed => new
* resolution: invalid =>
Comment:
This is not a support request, I am reporting a bug in the documentation,
which is so garbled as to be essentially completely meaningless and to
render a long-standing and potentially-useful Django feature useless.
Your reference to #29055 is helpful as it helped me understand the
documentation is even worse than I thought it was - when it says
"JavaScript template literals" this doesn't mean "a JavaScript literal
created from a Django template" which would be the obvious interpretation
given that is precisely what this feature is for, it means the relatively-
recent JavaScript backtick syntax.
Add that to the fact that the second sentence has two clauses which
contradict each other, and I cannot understand how you can possibly think
the current text is "ok" - it's complete gibberish.
Since you're asking for a concrete proposal I would suggest:
Escapes characters for use in JavaScript strings. This makes the
string safe for use in HTML and JavaScript or JSON string literals. Note
that it does ''not'' make it safe for use in "JavaScript template
literals" (i.e. the JavaScript backtick syntax).
For example:
<script>
let myVar = '{{ value|escapejs }}'
If you're tempted to reply that it ''doesn't'' make the string safe for
use in HTML (or JavaScript) then I would suggest examining that idea
briefly first. This filter escapes all of the characters that the 'escape'
filter does, so it definitely does make it safe for HTML. And if it
somehow fails to make it safe for a JavaScript string literal then I am
failing to see how.
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:4>
Django
Reporter: Jon Ribbens | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 4.0
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 Jon Ribbens):
* cc: Jon Ribbens (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:5>
Django
Reporter: Jon Ribbens | Owner: nobody
Component: Documentation | Version: 4.0
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 Carlton Gibson):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Not sure on the exact wording, but if you'd like to make a PR, please do.
Accepting on that assumption.
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:6>
Django
Reporter: Jon Ribbens | Owner: Jon
Type: | Ribbens
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Ribbens):
* owner: nobody => Jon Ribbens
* status: new => assigned
* has_patch: 0 => 1
* version: 4.0 => dev
Comment:
Pull Request available at https://github.com/django/django/pull/15430
Note the PR "fails" the docs check as the spelling checker doesn't
recognise the valid word "backtick".
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:7>
Django
Reporter: Jon Ribbens | Owner: Jon
Type: | Ribbens
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:8>
Django
Reporter: Jon Ribbens | Owner: Jon
Type: | Ribbens
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Ribbens):
* needs_better_patch: 1 => 0
Comment:
Note I improved the patch to fix the test failure 15 months ago and it now
passes.
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:9>
Django
Reporter: Jon Ribbens | Owner: Jon
Type: | Ribbens
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:10>
Django
Reporter: Jon Ribbens | Owner: Jon
Type: | Ribbens
Component: Documentation | Version: dev
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 Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"adfb3dfa89b62ee0c838a64d3d480c03dd3ec869" adfb3dfa]:
{{{
#!CommitTicketReference repository=""
revision="adfb3dfa89b62ee0c838a64d3d480c03dd3ec869"
Fixed #33405, Refs #7177 -- Clarified docs for filter escapejs regarding
safe and unsafe usages.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:11>
Django
Reporter: Jon Ribbens | Owner: Jon
Type: | Ribbens
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
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 Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"e54f711d4287b3ea57026a02b48ab7e28ca6dcc1" e54f711]:
{{{
#!CommitTicketReference repository=""
revision="e54f711d4287b3ea57026a02b48ab7e28ca6dcc1"
[4.2.x] Fixed #33405, Refs #7177 -- Clarified docs for filter escapejs
regarding safe and unsafe usages.
Backport of adfb3dfa89b62ee0c838a64d3d480c03dd3ec869 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33405#comment:12>