[Django] #26478: Template Context should validate names

24 views
Skip to first unread message

Django

unread,
Apr 7, 2016, 8:30:41 PM4/7/16
to django-...@googlegroups.com
#26478: Template Context should validate names
-------------------------------+----------------------
Reporter: steveire | Owner: nobody
Type: New feature | Status: new
Component: Uncategorized | Version: 1.9
Severity: Normal | Keywords: template
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------
The 'for' tag does not validate names of unpacked variables, allowing
things like

`{% for k|upper, "v" in mapping.items %}`

without throwing an error. Such 'variables' are not useful within the
`for` block.

{{{#!python
#!/usr/bin/env python

from django.template import Template, Context
from django.template.engine import Engine

e = Engine()

c = Context()
c["m"] = {"one": "1", "two": "2"}

t = e.from_string('{% for k|upper, v in m.items %}{{ k|upper }} : {{ v
}}\n{% endfor %}')
print t.render(c)
# : 2
# : 1

t = e.from_string('{% for "k", v in m.items %}{{ "k" }} : {{ v }}\n{%
endfor %}')
print t.render(c)
# k : 2
# k : 1
}}}

The for tag should error on an attempt to unpack to variables which
contain FILTER_SEPARATOR, double-quoted string or single-quoted string.

The underlying issue is that `Context` does not validate keys it is given,
so the `cycle` tag also has this issue in the form of `{% cycle 'a' 'b'
'c' as "letter" %}`, as does `widthratio` and any other tag which has an
'as' form.

--
Ticket URL: <https://code.djangoproject.com/ticket/26478>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 8, 2016, 7:18:02 AM4/8/16
to django-...@googlegroups.com
#26478: Template Context should validate names
---------------------------------+------------------------------------

Reporter: steveire | Owner: nobody
Type: New feature | Status: new
Component: Template system | Version: 1.9
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 timgraham):

* needs_better_patch: => 0
* component: Uncategorized => Template system
* needs_tests: => 0
* keywords: template =>
* needs_docs: => 0
* stage: Unreviewed => Accepted


Comment:

We need to be careful as this sort of "helpful validation" may break
working code, even if a bit odd. Accepting for further investigation.

--
Ticket URL: <https://code.djangoproject.com/ticket/26478#comment:1>

Django

unread,
Dec 17, 2016, 12:21:42 PM12/17/16
to django-...@googlegroups.com
#26478: Template Context should validate names
---------------------------------+--------------------------------------
Reporter: Stephen Kelly | Owner: Tim Martin
Type: New feature | Status: assigned

Component: Template system | Version: 1.9
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 Tim Martin):

* owner: nobody => Tim Martin
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/26478#comment:2>

Django

unread,
Dec 18, 2016, 10:07:07 AM12/18/16
to django-...@googlegroups.com
#26478: Template Context should validate names
---------------------------------+--------------------------------------
Reporter: Stephen Kelly | Owner: Tim Martin
Type: New feature | Status: assigned
Component: Template system | Version: 1.9
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
---------------------------------+--------------------------------------

Comment (by Tim Martin):

I've created a patch that fixes this by having the `do_for` function
validate the variables against known failure cases. However, this isn't
the most general solution, since there are lots of other cases of invalid
syntax that won't be caught by this. Would it make sense instead to
validate tokens against the requirements for Python identifiers as
described
[https://docs.python.org/3/reference/lexical_analysis.html#identifiers
here]?

--
Ticket URL: <https://code.djangoproject.com/ticket/26478#comment:3>

Django

unread,
Dec 20, 2016, 6:09:51 AM12/20/16
to django-...@googlegroups.com
#26478: Template Context should validate names
---------------------------------+--------------------------------------
Reporter: Stephen Kelly | Owner: Tim Martin
Type: New feature | Status: assigned
Component: Template system | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------------------------
Changes (by Tim Martin):

* has_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/26478#comment:4>

Django

unread,
Dec 29, 2016, 11:15:35 AM12/29/16
to django-...@googlegroups.com
#26478: Prohibit quotes and vertical bar in {% for %} unpacking variable names
-------------------------------------+-------------------------------------

Reporter: Stephen Kelly | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: assigned

Component: Template system | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* type: New feature => Cleanup/optimization
* stage: Accepted => Ready for checkin


Comment:

I'm unsure about further changes, I think we can go with your patch for
now.

--
Ticket URL: <https://code.djangoproject.com/ticket/26478#comment:5>

Django

unread,
Dec 30, 2016, 10:17:26 AM12/30/16
to django-...@googlegroups.com
#26478: Prohibit quotes and vertical bar in {% for %} unpacking variable names
-------------------------------------+-------------------------------------
Reporter: Stephen Kelly | Owner: Tim
Type: | Martin
Cleanup/optimization | Status: closed

Component: Template system | Version: 1.9
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"e3f095b086225d3bd3eae6266ec1a0580a5d49e8" e3f095b0]:
{{{
#!CommitTicketReference repository=""
revision="e3f095b086225d3bd3eae6266ec1a0580a5d49e8"
Fixed #26478 -- Made {% for %} reject invalid unpacking vars with quotes
or vertical bars.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26478#comment:6>

Reply all
Reply to author
Forward
0 new messages