Non-existent arg passed to template filter raises VariableDoesNotExist

[Colin Copeland]

Branched from #12554.

​How Invalid Variables Are Handled seems to imply that if a variable doesn't exist, the template system will insert the value of the TEMPLATE_STRING_IF_INVALID setting and not raise a VariableDoesNotExist exception. Currently, if a non-existent variable is used as an argument to a template filter, a VariableDoesNotExist exception is raised. For example:

from django.template import Context, Template
Template('{{ foo|default:notreal }}').render(Context({'foo': ''}))

Raises: TemplateSyntaxError: Caught VariableDoesNotExist while rendering: Failed lookup for key [notreal] in u"[{'foo': }]"

I'm not sure what the proper handling should be in this case.

[Colin Copeland]

Simple tags seem to do this as well, so maybe filters should continue to follow suit:

from django import template

register = template.Library()

def simple_echo(var):
    return var

register.simple_tag(simple_echo)
template.libraries['testtags'] = register
template.Template('{% load testtags %}{% simple_echo notreal %}').render(template.Context({}))

Raises: TemplateSyntaxError: Caught VariableDoesNotExist while rendering: Failed lookup for key [notreal] in u'[{}]'

[ohmi2]

This is a breaking change from 1.1.

All my production templates started breaking, which is why I tracked it down and submitted the patch to #12554 since I could not duplicate its failure mode but could duplicate mine.
Simple tags are currently failing because of the same bug - that doesn't happen in 1.1.

Invalid variable handling should be consistent no matter how the variable is referenced in a template expression of any sort.

[Karen Tracey]

I'm confused, because near as I can tell, the described behavior has existed since at least 1.1. If I try applying the tests from the patch and running them on the 1.1.X branch, without the code change, they fail:

======================================================================
FAIL: test_templates (regressiontests.templates.tests.Templates)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 269, in test_templates
    ('-'*70, ("\n%s\n" % ('-'*70)).join(failures)))
AssertionError: Tests failed:
----------------------------------------------------------------------
Template test (TEMPLATE_STRING_IF_INVALID=''): filter-syntax21 -- FAILED. Got <class 'django.template.VariableDoesNotExist'>, exception: Failed lookup for key [notreal] in u"[{'foo': 'test'}]"
Traceback (most recent call last):

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 243, in test_templates
    output = self.render(test_template, vals)

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 274, in render
    output = test_template.render(context)

  File "/home/kmt/django/branch1.1.X/django/test/utils.py", line 29, in instrumented_test_render
    return self.nodelist.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 779, in render
    bits.append(self.render_node(node, context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 792, in render_node
    return node.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 826, in render
    output = self.filter_expression.resolve(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 563, in resolve
    arg_vals.append(arg.resolve(context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 684, in resolve
    value = self._resolve_lookup(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 737, in _resolve_lookup
    raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit, current)) # missing attribute

VariableDoesNotExist: Failed lookup for key [notreal] in u"[{'foo': 'test'}]"

----------------------------------------------------------------------
Template test (TEMPLATE_STRING_IF_INVALID='INVALID'): filter-syntax21 -- FAILED. Got <class 'django.template.VariableDoesNotExist'>, exception: Failed lookup for key [notreal] in u"[{'foo': 'test'}]"
Traceback (most recent call last):

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 243, in test_templates
    output = self.render(test_template, vals)

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 274, in render
    output = test_template.render(context)

  File "/home/kmt/django/branch1.1.X/django/test/utils.py", line 29, in instrumented_test_render
    return self.nodelist.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 779, in render
    bits.append(self.render_node(node, context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 792, in render_node
    return node.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 826, in render
    output = self.filter_expression.resolve(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 563, in resolve
    arg_vals.append(arg.resolve(context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 684, in resolve
    value = self._resolve_lookup(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 737, in _resolve_lookup
    raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit, current)) # missing attribute

VariableDoesNotExist: Failed lookup for key [notreal] in u"[{'foo': 'test'}]"

----------------------------------------------------------------------
Template test (TEMPLATE_STRING_IF_INVALID=''): filter-syntax22 -- FAILED. Got <class 'django.template.VariableDoesNotExist'>, exception: Failed lookup for key [notreal] in u"[{'foo': ''}]"
Traceback (most recent call last):

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 243, in test_templates
    output = self.render(test_template, vals)

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 274, in render
    output = test_template.render(context)

  File "/home/kmt/django/branch1.1.X/django/test/utils.py", line 29, in instrumented_test_render
    return self.nodelist.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 779, in render
    bits.append(self.render_node(node, context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 792, in render_node
    return node.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 826, in render
    output = self.filter_expression.resolve(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 563, in resolve
    arg_vals.append(arg.resolve(context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 684, in resolve
    value = self._resolve_lookup(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 737, in _resolve_lookup
    raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit, current)) # missing attribute

VariableDoesNotExist: Failed lookup for key [notreal] in u"[{'foo': ''}]"

----------------------------------------------------------------------
Template test (TEMPLATE_STRING_IF_INVALID='INVALID'): filter-syntax22 -- FAILED. Got <class 'django.template.VariableDoesNotExist'>, exception: Failed lookup for key [notreal] in u"[{'foo': ''}]"
Traceback (most recent call last):

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 243, in test_templates
    output = self.render(test_template, vals)

  File "/home/kmt/django/branch1.1.X/tests/regressiontests/templates/tests.py", line 274, in render
    output = test_template.render(context)

  File "/home/kmt/django/branch1.1.X/django/test/utils.py", line 29, in instrumented_test_render
    return self.nodelist.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 779, in render
    bits.append(self.render_node(node, context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 792, in render_node
    return node.render(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 826, in render
    output = self.filter_expression.resolve(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 563, in resolve
    arg_vals.append(arg.resolve(context))

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 684, in resolve
    value = self._resolve_lookup(context)

  File "/home/kmt/django/branch1.1.X/django/template/__init__.py", line 737, in _resolve_lookup
    raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit, current)) # missing attribute

VariableDoesNotExist: Failed lookup for key [notreal] in u"[{'foo': ''}]"


----------------------------------------------------------------------
Ran 18 tests in 1.360s

FAILED (failures=1)

I also verified that they fail when run with 1.1 and 1.1.1 release level code, so I am missing how this behavior a break from how 1.1 behaved.

The doc referenced explicitly mentions applying filters to nonexistent variables, that is something like:

{{ notreal|default:foo }}

where foo exists in the context but notreal does not. In that case is where you see no VariableDoesNotExist raised.

The issue here concerns how filter arguments are handled, and so far as I can see the doc does not mention that behavior explicitly. Given it seems to have resulted in VariableDoesNotExist being raised at least as far back as 1.1, I'd be reluctant to change it at this point. It also seems consistent with how arguments to custom template tags registered via simple_tag work. The behavior noted by copelco above is consistent with what I recall of my earliest days working with Django, back around the 0.95 timeframe.

[Russell Keith-Magee]

Following discussion on django-sprint with myself, James B and Karen:

There is a larger issue here that needs to be decided on, which is a clarification of exactly when templates should raise exceptions, and when they should fail silently. In the interests of debugging, vocal exceptions are helpful; in the interests of production being error-free, silent failure is helpful.

One option on the table is to use TEMPLATE_DEBUG; if True, raise exceptions; otherwise, fail silently. This policy could then be retroactively applied across the rest of the template system - e.g., with the {% url %} tag raising exceptions.

Whatever the final decision, it requires further discussion. Putting on the list for 1.3.

[ohmi2]

Here is the test case that shows a breaking behavior from 1.1.1

1.1.1

>>> from django.template import Context, Template
>>> Template("{% if use_primary_sort_key|default_if_none:my_primary_sort_key %}{%endif%}").render(Context({}))
u''
>>> 

1.2 SVN

>>> from django.template import Context, Template
>>> Template("{% if use_primary_sort_key|default_if_none:my_primary_sort_key %}{%endif%}").render(Context({}))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/__init__.py", line 171, in render
    return self._render(context)
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/__init__.py", line 165, in _render
    return self.nodelist.render(context)
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/__init__.py", line 794, in render
    bits.append(self.render_node(node, context))
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/debug.py", line 72, in render_node
    result = node.render(context)
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/defaulttags.py", line 244, in render
    if self.var.eval(context):
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/defaulttags.py", line 745, in eval
    return self.value.resolve(context, ignore_failures=True)
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/__init__.py", line 573, in resolve
    arg_vals.append(arg.resolve(context))
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/__init__.py", line 694, in resolve
    value = self._resolve_lookup(context)
  File "/opt/local/OnQueue2/OQTools/pythonlib/django/template/__init__.py", line 747, in _resolve_lookup
    raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit, current)) # missing attribute
TemplateSyntaxError: Caught VariableDoesNotExist while rendering: Failed lookup for key [my_primary_sort_key] in u'[{}]'

So this is something that really should be addressed in 1.2.

It doesn't seem like that difficult a call to me - the resolve function catches the VariableDoesNotExist exception and applies the TEMPLATE_STRING_IF_INVALID semantic - that is its job. It can do this job on more than one variable per invocation - i.e., when it is given a filter expression. It doesn't make any sense to raise VariableDoesNotExist for variables that appear in the filter arguments - it would prevent the default_if_none filter from being chained for example:

>>> django.get_version()
'1.1.1'
>>> Template("{% if x|default_if_none:y|default_if_none:z %}{%endif%}").render(Context({}))
u''
>>> 

although it is true that {{x|default_if_none:y|default_if_none:z}} throws the exception in 1.1.1, so obviously something about how the if tag is parsed is also contributing to the bug.

As it stands now I will not be able to switch to 1.2 and I suspect others will find themselves in the same boat.
The TEMPLATE_DEBUG option is fine by me, but really, I can't see any reason to not treat any missing variable exactly the same way in all circumstances.

[Karen Tracey]

The change in behavior came about with r11806 (smart if). As we now have an example of a change in behavior from 1.1, we do need to address this before 1.2. Templates that did not generate server errors in 1.1 should not do so in 1.2 (excepting for cases noted in the backwards-incompatible list for 1.2, and I don't see that this is covered by anything there).

[Karen Tracey]

Added 3 attachments:

• 13167_test has an integrated test that demonstrates the problem. All added tests pass on 1.1.X, badarg01 fails on current trunk due to VariableDoesNotExist being raised. (The other two were just to verify nothing else broke.)
• 13167_filterargfix is the originally-proposed fix, modified to allow for continuing to raise exceptions in this case if TEMPLATE_DEBUG is True. This removes the VariableDoesNotExist exception from the test, but does not allow the test to pass unchanged. Difference is this fix changes the behavior of the if tag in the case where TEMPLATE_STRING_IF_INVALID is set to something other than the empty string. The old if tag would always evaluate to False if VariableDoesNotExist was raised -- with this proposed fix, it will evaluate to True if TEMPLATE_STRING_IF_INVALID is set to something. So the test has to be modified to account for that. It's unlikely anyone would observe this difference in behavior on a production site, since it's unlikely (I think) anyone runs production with TEMPLATE_STRING_IF_INVALID set to something, but it is a difference in behavior.
• 13167_iffix is an alternative fix that changes the if tag behavior to match what it used to do in this case prior to r11806 (I think, it's a quick fix to code I'm not familiar with). With this fix the added test passes unchanged on trunk.

Both options do pass the full test suite. I'm inclined to favor fixing the if tag behavior to match what it used to do, since that seems least likely to introduce other problems. Other opinions?

[Karen Tracey]

Updated patches to include a test for the else leg being non-empty, and correct the iffix approach to fixing the problem to properly handle that case.

[ohmi2]

For what is worth, I believe 13167_filterargfix is required. In my opinion, the change in behavior noted is completely consistent with the design and purpose of the TEMPLATE_STRING_IF_INVALID semantic and how nonexistant variables should interact with other template constructs like the if tag. The behavior today in 1.1.x masks the real bug.

I am not certain about this, but I doubt the if tag fix will do the right thing here:

{% if notreal|default_if_none:also_not_real|default_if_none:1 %}yes{%else%}no{%endif%}

The output here should be yes but in 1.1 and with the proposed fix the answer is no, because also_not_real raises an exception and stops the further evaluation of the filter.
Also, interactions with the or operator are likely to not work:

>>> django.get_version()
u'1.2 beta 1 SVN-12841'
>>> from django.template import Template, Context
>>> Template("{%if not_real|default_if_none:also_not_real or 1%}yes{%else%}no{%endif%}").render(Context({}))
u'no'
>>> Template("{%if not_real|default_if_none:also_not_real or real%}yes{%else%}no{%endif%}").render(Context({"real":"yes"}))
u'no'
>>> Template("{%if not_real|default_if_none:real or real%}yes{%else%}no{%endif%}").render(Context({"real":"yes"}))
u'yes'

In all three cases the result should be "yes"

I argue that the current 1.1 if behavior masks the real bug, defeats the purpose of the default_if_none filter, and produces incorrect evaluations of or clauses. I think the filter fix will produce the correct result.

That being said, I think the if fix should also be added anyway. The VariableDoesNotExist exception simply should not ever be raised out of the template system unless TEMPLATE_DEBUG is being used. Defensive programming here helps ensure that goal.

Thank you for paying attention to this issue.

[Gabriel Hurley]

The "has patch" box never got checked. Just doing a little cleanup...

[Russell Keith-Magee]

(In [12954]) Refs #13167 -- Corrected a regression in the way non-existent variables are handled by {% if %} tags. Thanks to ohmi2 for pointing out the regression in 1.2, and Karen for the patch.

[Russell Keith-Magee]

I agree with Karen. Applying the fix to the FilterExpression may have passed the if tests, but it introduced other regressions (with the same sort of chained expressions, but applied as {{ }} rather than as if expressions). Given that the root problem has existed for a while, I'm inclined to just fix the regression (i.e., that {% if %} should never raise a VariableDoesNotExist error), and address the deeper problem as part of 1.3.

[Karen Tracey]

#13672 (concerned the behavior of the cache tag, specifically, when handed non-existed arguments) was closed as a dupe of this one.

[Jacob]

Milestone 1.3 deleted

[Jacob]

I believe this was fixed years ago. If there's still a bug here, please open a new ticket; following what's going on here is tough :)

[Karen Tracey]

The two calls from the original description still produce the exact same result (an exception during template rendering), so if this is going to be closed the resolution should be wontfix. History here is that this was claimed to be a regression from 1.1 to 1.2 while 1.2 was in development, but in fact was not. The actual regression was teased out in the discussion, and was slightly different from the original report. THAT regression was fixed. But the originally reported problem here was not a regression, behavior here has been consistent since probably prior to 1.0. I'm fine with wontfixing that, though it does rather seem to go against "template errors don't raise exceptions" philosophy.

[Tim Graham]

#28172 asks to revisit this issue.