Opened 8 years ago

Closed 7 years ago

#26478 closed Cleanup/optimization (fixed)

Prohibit quotes and vertical bar in {% for %} unpacking variable names

Reported by: Stephen Kelly Owned by: Tim Martin
Component: Template system Version: 1.9
Severity: Normal Keywords:
Cc: 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 '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.

#!/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.

Change History (6)

comment:1 by Tim Graham, 8 years ago

Component: UncategorizedTemplate system
Keywords: template removed
Triage Stage: UnreviewedAccepted

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

comment:2 by Tim Martin, 7 years ago

Owner: changed from nobody to Tim Martin
Status: newassigned

comment:3 by Tim Martin, 7 years ago

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 here?

Last edited 7 years ago by Tim Martin (previous) (diff)

comment:4 by Tim Martin, 7 years ago

Has patch: set

comment:5 by Tim Graham, 7 years ago

Summary: Template Context should validate namesProhibit quotes and vertical bar in {% for %} unpacking variable names
Triage Stage: AcceptedReady for checkin
Type: New featureCleanup/optimization

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

comment:6 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In e3f095b0:

Fixed #26478 -- Made {% for %} reject invalid unpacking vars with quotes or vertical bars.

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