Code

Opened 6 years ago

Closed 4 years ago

Last modified 3 years ago

#6752 closed (fixed)

Documentation slightly unclear about "safe" strings in filters

Reported by: <removed at reporter's request> Owned by: Rupe
Component: Documentation Version: master
Severity: Keywords: safe, auto-escape, escapejs
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

My objective here is to prevent somebody else from falling into the same trap as I.

This is with regard to auto-escaping, the safe filter, and escapejs. I had auto-escaping on and tried the following:

template_string|safe|escape

I now now this is clearly wrong because in the docs it states. "This does /not/ make the string safe for use in HTML." But when I first read it, I thought this was perfect and that all I need to do is the above statement. I would use safe to clear the auto-escaping and the escape the string for javascript. But this is not how it works. Albeit, I might have over-thinked it but I figured that "safe" either set a bit in the template context so that auto-escaping would not happen at the end. I didn't realize that "This does /not/ make the string safe for use in HTML" meaning that auto-escaping would pick up on this fact and escape the string. The output of another filter might make the string unsafe again.

So the logic is correct. I would like to see a change to the docs here are some ideas:

  1. Update the documentation for Safe to be something like the following:

"Marks a string as not requiring further HTML escaping prior to output. When this filter is not the last filter applied, subsequent filters might make string /not/ safe for HTML output and the string will be escaped if auto-escaping is on. When autoescaping is off, this filter has no effect."

That's obviously too wordy but I think it brings across the point.

  1. Update auto-escaping docs to mention that safe will turn off auto-escaping unless a filter that follows safe makes the string unsafe for HTML output.
  1. Or maybe just a more technical explaination of how safe, escape, and auto-escape work under the covers.

Thanks.

Attachments (3)

template_safe_string_note.diff (641 bytes) - added by Rupe 4 years ago.
Added a note to the safe string filter text
t6752.diff (645 bytes) - added by Alex 4 years ago.
Slightly different wording.
t6752_revised.diff (529 bytes) - added by Rupe 4 years ago.
Changed the term unescaped to demonstrate HTML vs safestring escaping

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Hi, can you remove or obfuscate my email address? I didn't realize it would get posted. Maybe a little warning next time. :)

comment:2 Changed 6 years ago by jacob

  • Reporter changed from djensen47@… to <removed at reporter's request>
  • Triage Stage changed from Unreviewed to Accepted

Changed 4 years ago by Rupe

Added a note to the safe string filter text

comment:3 Changed 4 years ago by Rupe

  • milestone set to 1.2
  • Owner changed from nobody to Rupe
  • Status changed from new to assigned

That should do it.

Changed 4 years ago by Alex

Slightly different wording.

comment:4 Changed 4 years ago by Alex

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

comment:5 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [13171]) Fixed #6752 -- Corrected the interaction of the safe template filter with other filters. Thanks to Rupe and Alex Gaynor for their work on the patch.

comment:6 Changed 4 years ago by russellm

(In [13177]) [1.1.X] Fixed #6752 -- Corrected the interaction of the safe template filter with other filters. Thanks to Rupe and Alex Gaynor for their work on the patch.

Backport of r13171 from trunk.

comment:7 Changed 4 years ago by Rupe

  • Resolution fixed deleted
  • Status changed from closed to reopened

I think the example I tried to use is a confusing one, in terms of being unescaped. The patch language refers to the escape filter causing the string to no longer be safestring-escaped, so thus you can say the string is unescaped, but at the same time the language is inferring the escape filter is taking preference. At first read it sounds like it's the escape filter that is not being applied when in fact it is.

I submited an update. Does it make sense?

Changed 4 years ago by Rupe

Changed the term unescaped to demonstrate HTML vs safestring escaping

comment:8 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from reopened to closed

I'm not convinced the proposed update text is an improvement. As far as I can make out, "Unescaped" is perfectly accurate - if var contains HTML markup, it will be output without the usual safety protection, even though |safe has been applied.

comment:9 Changed 4 years ago by Rupe

I never said it wasn't accurate, the terminology was just confusing. I think the main issue here is that the default is to automatically html-escape a string. Thus the escaping you are referring to will prevent this so that the string will not be html-escaped. I think that the average user reading the example will miss the distinction between safe string escaping and html escaping, especially because the escape filter is being used.

comment:10 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.