Django: Ticket #10912: Autoescaping variable input in template tagshttps://code.djangoproject.com/ticket/10912
<p>
(revised description)
</p>
<p>
When variables are used as input to template tags, we should handle autoescaping a bit better.
</p>
<p>
Original description was even shorter:
</p>
<p>
I'd expect firstof to escape variables.
</p>
en-usDjangohttps://www.djangoproject.com/s/img/site/hdr_logo.gifhttps://code.djangoproject.com/ticket/10912
Trac 1.2Andrew BadrFri, 24 Apr 2009 03:06:54 GMTattachment sethttps://code.djangoproject.com/ticket/10912
https://code.djangoproject.com/ticket/10912
<ul>
<li><strong>attachment</strong>
set to <em>firstof_autoescape_t10912_r10630.diff</em>
</li>
</ul>
<p>
Fix, with test
</p>
TicketAndrew BadrFri, 24 Apr 2009 03:09:46 GMThttps://code.djangoproject.com/ticket/10912#comment:1
https://code.djangoproject.com/ticket/10912#comment:1
<p>
My expected behavior for Django is: The results of all template tags should be escaped unless marked safe. For builtin templatetags that return safe strings, this should include escaping any variables involved in rendering that template tag.
</p>
<p>
The included patch solves locally what I see as part of a bigger problem.
</p>
TicketMalcolm TredinnickSun, 26 Apr 2009 20:36:20 GMTneeds_better_patch, summary, description, stage changed; cc sethttps://code.djangoproject.com/ticket/10912#comment:2
https://code.djangoproject.com/ticket/10912#comment:2
<ul>
<li><strong>cc</strong>
<em>Malcolm Tredinnick</em> added
</li>
<li><strong>needs_better_patch</strong>
set
</li>
<li><strong>summary</strong>
changed from <em>firstof template tag should autoescape variables</em> to <em>Autoescaping variable input in template tags</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10912?action=diff&amp;version=2">diff</a>)
</li>
<li><strong>stage</strong>
changed from <em>Unreviewed</em> to <em>Accepted</em>
</li>
</ul>
<p>
Hmm. This is a bit of a slight change in policy, but not necessarily a bad one (right now, intentionally, only filters autoescape). However,
</p>
<ol><li>Import <code>_render_change_in_context()</code> smells and not of roses. Needs to be renamed or not imported or something. That leading underscore is a big clue.
</li><li>We need something similar to the <code>autoescape</code> parameter that is passed to filters, so that template tags do not autoescape results when they are being used inside a block that does not do auto-escaping.
</li></ol><p>
I guess template tags know whether they're putting the results into the context -- which means the output shouldn't be autoescaped, since it will be autoescaped on output and you don't know at template tag usage time whether output will involve auto-escaping or not, so it must be delayed until rendering -- or directly displaying the results? I suspect so, but that's something to think about and confirm.
</p>
<p>
What are the backwards compatibility issues here? In particular, right now, a template tag doesn't have to care about calling <code>mark_safe()</code> on the result, since it just inserts it into the template. If you add the auto-escaping behaviour at the wrong place, you'll undo that and a lot of existing template tags will break. That's going to be a little fiddly to work around.
</p>
<p>
I'm unenthusiastic about fixing just this single tag, since it does change the currently consistent behaviour across all template tags. I'm more enthusiastic about addressing the general issue, but it's a lot more tricky. I'm really enthusiastic about you doing it, Andrew, since I know you guys work in the template space a lot, so the results will have some validity and it means I don't have to work on this (that latter point is important to me; maybe less so to you).
</p>
TicketkrystalWed, 24 Jun 2009 16:53:01 GMThttps://code.djangoproject.com/ticket/10912#comment:3
https://code.djangoproject.com/ticket/10912#comment:3
<p>
Just to mention that I just discovered a flaw on my website due to this non-escaping-feature when using firstof tags ; It should really be at least explicitly documented so people get aware of this while reading the firstof (or cycle) doc.
</p>
<p>
A little paragraph with a one line example should be nice until a good way to do is found.
</p>
TicketkrystalThu, 25 Jun 2009 15:44:45 GMTmilestone sethttps://code.djangoproject.com/ticket/10912#comment:4
https://code.djangoproject.com/ticket/10912#comment:4
<ul>
<li><strong>milestone</strong>
set to <em>1.1</em>
</li>
</ul>
<p>
I move this to milestone 1.1 so the big-boss-team give his opinion ; default django security is involved and the doc clearly say that "firstof" is equivalent to {%if bla %}{{varx}} for now, it's very missleading.
</p>
<p>
I can wrote a patch for the documentation if needed, just tell me if I have to open a new ticket.
</p>
TicketAndrew BadrThu, 25 Jun 2009 21:06:05 GMTmilestone deletedhttps://code.djangoproject.com/ticket/10912#comment:5
https://code.djangoproject.com/ticket/10912#comment:5
<ul>
<li><strong>milestone</strong>
<em>1.1</em> deleted
</li>
</ul>
<p>
I'm not big-boss-team, but this feature is definitely not making it into 1.1, so I'm removing the milestone. I think your concern about documentation is valid, but I would make a new ticket.
</p>
TicketAndrew BadrSat, 12 Dec 2009 23:27:45 GMTowner changedhttps://code.djangoproject.com/ticket/10912#comment:6
https://code.djangoproject.com/ticket/10912#comment:6
<ul>
<li><strong>owner</strong>
changed from <em>Andrew Badr</em> to <em>nobody</em>
</li>
</ul>
TicketRussell Keith-MageeTue, 23 Mar 2010 14:44:08 GMTmilestone sethttps://code.djangoproject.com/ticket/10912#comment:7
https://code.djangoproject.com/ticket/10912#comment:7
<ul>
<li><strong>milestone</strong>
set to <em>1.3</em>
</li>
</ul>
<p>
Promoting this to ensure it is looked at in 1.3
</p>
TicketChris BeavenFri, 08 Apr 2011 11:06:34 GMTstage changed; type, severity set; milestone deletedhttps://code.djangoproject.com/ticket/10912#comment:8
https://code.djangoproject.com/ticket/10912#comment:8
<ul>
<li><strong>stage</strong>
changed from <em>Accepted</em> to <em>Design decision needed</em>
</li>
<li><strong>type</strong>
set to <em>Bug</em>
</li>
<li><strong>severity</strong>
set to <em>Release blocker</em>
</li>
<li><strong>milestone</strong>
<em>1.3</em> deleted
</li>
</ul>
<p>
Guess it didn't get looked at in 1.3... I'm going to bump severity.
</p>
<p>
Since the accepted was 2 years ago, I'm also going to push back to a design decision.
</p>
TicketkenthSat, 12 Nov 2011 16:37:04 GMTstatus, owner changed; ui_ux, easy sethttps://code.djangoproject.com/ticket/10912#comment:9
https://code.djangoproject.com/ticket/10912#comment:9
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>assigned</em>
</li>
<li><strong>owner</strong>
changed from <em>nobody</em> to <em>kenth</em>
</li>
<li><strong>ui_ux</strong>
unset
</li>
<li><strong>easy</strong>
unset
</li>
</ul>
TicketkenthSat, 12 Nov 2011 19:05:27 GMTstatus changed; owner deletedhttps://code.djangoproject.com/ticket/10912#comment:10
https://code.djangoproject.com/ticket/10912#comment:10
<ul>
<li><strong>owner</strong>
<em>kenth</em> deleted
</li>
<li><strong>status</strong>
changed from <em>assigned</em> to <em>new</em>
</li>
</ul>
<p>
There are two template tags which generate output that is not autoescaped: firstof &amp; cycle. This behavior might be unexpected, but it appears to be well documented even back in the version 1.0 documents. There is a policy statement in the documentation stating output is not escaped "... because template tags do not escape their content."
</p>
<p>
The code appears to work as documented &amp; the documentation appears to be clear. As to if the behavior is as it should be, that's clearly a design decision...
</p>
TicketAdrian HolovatyFri, 09 Dec 2011 19:52:21 GMThttps://code.djangoproject.com/ticket/10912#comment:11
https://code.djangoproject.com/ticket/10912#comment:11
<p>
In <a class="changeset" href="https://code.djangoproject.com/changeset/17176/">[17176]</a>:
</p>
<div class="message"><p>
Added a test that 'firstof' template filter doesn't auto-escape. cycle tag already has such a test (cycle20). Refs <a class="closed ticket" href="https://code.djangoproject.com/ticket/10912" title="#10912: Bug: Autoescaping variable input in template tags (closed: fixed)">#10912</a><br />
</p>
</div>
TicketAdrian HolovatyFri, 09 Dec 2011 19:53:07 GMThttps://code.djangoproject.com/ticket/10912#comment:12
https://code.djangoproject.com/ticket/10912#comment:12
<p>
In <a class="changeset" href="https://code.djangoproject.com/changeset/17177/">[17177]</a>:
</p>
<div class="message"><p>
Tweaked templates/builtins.txt to make it clearer that cycle and firstof filters don't auto-escape. Refs <a class="closed ticket" href="https://code.djangoproject.com/ticket/10912" title="#10912: Bug: Autoescaping variable input in template tags (closed: fixed)">#10912</a><br />
</p>
</div>
TicketAdrian HolovatyFri, 09 Dec 2011 19:55:50 GMTstatus changed; resolution sethttps://code.djangoproject.com/ticket/10912#comment:13
https://code.djangoproject.com/ticket/10912#comment:13
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
OK, I'm marking this as fixed because the documentation is clear about the fact that firstof and cycle don't auto-escape. I also added a test for firstof (we already had one for cycle).
</p>
<p>
Just for future reference, we added some original docs in <a class="changeset" href="https://code.djangoproject.com/changeset/11163/">[11163]</a>.
</p>
Ticket