Opened 3 years ago

Closed 17 months ago

Last modified 17 months ago

#33405 closed Cleanup/optimization (fixed)

Documentation for template filter 'escapejs' is extremely unclear

Reported by: Jon Ribbens Owned by: Jon Ribbens
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Adam Johnson, Jon Ribbens 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 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).

Change History (12)

comment:1 by Carlton Gibson, 3 years ago

Cc: Adam Johnson added
Resolution: invalid
Status: newclosed

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 json_script tag.
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...)

comment:2 by Jon Ribbens, 3 years ago

Resolution: invalid
Status: closednew

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.

comment:3 by Carlton Gibson, 3 years ago

Resolution: invalid
Status: newclosed

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.

comment:4 by Jon Ribbens, 3 years ago

Resolution: invalid
Status: closednew

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.

comment:5 by Jon Ribbens, 3 years ago

Cc: Jon Ribbens added

comment:6 by Carlton Gibson, 3 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Not sure on the exact wording, but if you'd like to make a PR, please do. Accepting on that assumption.

comment:7 by Jon Ribbens, 3 years ago

Has patch: set
Owner: changed from nobody to Jon Ribbens
Status: newassigned
Version: 4.0dev

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".

comment:8 by Carlton Gibson, 3 years ago

Patch needs improvement: set

comment:9 by Jon Ribbens, 18 months ago

Patch needs improvement: unset

Note I improved the patch to fix the test failure 15 months ago and it now passes.

comment:10 by Mariusz Felisiak, 17 months ago

Triage Stage: AcceptedReady for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 17 months ago

Resolution: fixed
Status: assignedclosed

In adfb3dfa:

Fixed #33405, Refs #7177 -- Clarified docs for filter escapejs regarding safe and unsafe usages.

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 17 months ago

In e54f711:

[4.2.x] Fixed #33405, Refs #7177 -- Clarified docs for filter escapejs regarding safe and unsafe usages.

Backport of adfb3dfa89b62ee0c838a64d3d480c03dd3ec869 from main

Note: See TracTickets for help on using tickets.
Back to Top