Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#12554 closed (fixed)

silent_variable_failure not suppressing exception during dict lookup

Reported by: Thomas Steinacher <tom@…> Owned by: nobody
Component: Template system Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

Example:

from django.template import Context, Template

class E(Exception): silent_variable_failure = True

class A(object):
	def __getitem__(self, key):
		raise E

t = Template("{{ a.b }}")

t.render(Context({"a": A()}))

According to Django docs, the exception should be suppresed and an empty string should be returned. However, instead, Django raises the exception:

---------------------------------------------------------------------------
TemplateSyntaxError                       Traceback (most recent call last)

/home/user/user/<ipython console> in <module>()

/usr/lib/python2.5/site-packages/django/template/__init__.pyc in render(self, context)
    176     def render(self, context):
    177         "Display stage -- can be called many times"
--> 178         return self.nodelist.render(context)
    179 
    180 def compile_string(template_string, origin):

/usr/lib/python2.5/site-packages/django/template/__init__.pyc in render(self, context)
    777         for node in self:
    778             if isinstance(node, Node):
--> 779                 bits.append(self.render_node(node, context))
    780             else:
    781                 bits.append(node)

/usr/lib/python2.5/site-packages/django/template/debug.py in render_node(self, node, context)
     79             wrapped.source = node.source
     80             wrapped.exc_info = exc_info()
---> 81             raise wrapped
     82         return result
     83 

TemplateSyntaxError: Caught an exception while rendering: 

Original Traceback (most recent call last):
  File "/usr/lib/python2.5/site-packages/django/template/debug.py", line 71, in render_node
    result = node.render(context)
  File "/usr/lib/python2.5/site-packages/django/template/debug.py", line 87, in render
    output = force_unicode(self.filter_expression.resolve(context))
  File "/usr/lib/python2.5/site-packages/django/template/__init__.py", line 546, in resolve
    obj = self.var.resolve(context)
  File "/usr/lib/python2.5/site-packages/django/template/__init__.py", line 687, in resolve
    value = self._resolve_lookup(context)
  File "/usr/lib/python2.5/site-packages/django/template/__init__.py", line 713, in _resolve_lookup
    current = current[bit]
  File "<ipython console>", line 4, in __getitem__
E

Proposed patch:

Index: django/template/__init__.py
===================================================================
--- django/template/__init__.py (revision 11929)
+++ django/template/__init__.py (working copy)
@@ -745,11 +745,11 @@
                             TypeError,  # unsubscriptable object
                             ):
                         raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit, current)) # missing attribute
-                except Exception, e:
-                    if getattr(e, 'silent_variable_failure', False):
-                        current = settings.TEMPLATE_STRING_IF_INVALID
-                    else:
-                        raise
+            except Exception, e:
+                if getattr(e, 'silent_variable_failure', False):
+                    current = settings.TEMPLATE_STRING_IF_INVALID
+                else:
+                    raise
 
         return current

Attachments (3)

12554.diff (1.5 KB) - added by copelco 4 years ago.
add b2's patch
r12819_with_tests.diff (2.7 KB) - added by copelco 4 years ago.
add patch with tests
r12819_with_tests_dict.diff (2.0 KB) - added by copelco 4 years ago.
Only dict lookup fix and test

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by russellm

  • milestone set to 1.2
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 4 years ago by b__2@…

The same error occurs when a variable that doesn't exist is referenced as the argument of a filter
for example:

{% if use_styleId|default_if_none:my_styleId %}

The stack trace shows the problem to be the naked call to resolve at around line 576:

arg_vals.append(arg.resolve(context))

My quick fix was to duplicate much of the exception logic used in the primary case just above:

                    try:
                        arg_val = arg.resolve(context)
                    except VariableDoesNotExist:
                        if ignore_failures:
                            arg_val = None
                        else:
                            if settings.TEMPLATE_STRING_IF_INVALID:
                                global invalid_var_format_string
                                if invalid_var_format_string is None:
                                    invalid_var_format_string = '%s' in settings.TEMPLATE_STRING_IF_INVALID
                                if invalid_var_format_string:
                                    arg_val= settings.TEMPLATE_STRING_IF_INVALID % self.var
                                else:
                                    arg_val= settings.TEMPLATE_STRING_IF_INVALID
                            else:
                                arg_val = settings.TEMPLATE_STRING_IF_INVALID
                    arg_vals.append(arg_val)

However, it seems to me that Variable.resolve() should be taking care of the settings.TEMPLATE_STRING_IF_INVALID semantic, not FilterExpression.
The patch suggested doesn't seem to do anything since VariableDoesNotExist is not derived from ObjectDoesNotExist. I did not use it.

comment:3 Changed 4 years ago by b__2@…

I realized I wasn't working from the trunk. Here is a real proposed patch:

===================================================================
--- __init__.py	(revision 12610)
+++ __init__.py	(working copy)
@@ -583,7 +583,24 @@
                 if not lookup:
                     arg_vals.append(mark_safe(arg))
                 else:
-                    arg_vals.append(arg.resolve(context))
+                    #arg_vals.append(arg.resolve(context))
+                    try:
+                        arg_val = arg.resolve(context)
+                    except VariableDoesNotExist:
+                        if ignore_failures:
+                            arg_val = None
+                        else:
+                            if settings.TEMPLATE_STRING_IF_INVALID:
+                                global invalid_var_format_string
+                                if invalid_var_format_string is None:
+                                    invalid_var_format_string = '%s' in settings.TEMPLATE_STRING_IF_INVALID
+                                if invalid_var_format_string:
+                                    arg_val= settings.TEMPLATE_STRING_IF_INVALID % self.var
+                                else:
+                                    arg_val= settings.TEMPLATE_STRING_IF_INVALID
+                            else:
+                                arg_val = settings.TEMPLATE_STRING_IF_INVALID
+                    arg_vals.append(arg_val)                  
             if getattr(func, 'needs_autoescape', False):

Here is a test case:
>>> from django.template import Context, Template
>>> t = Template("{{'test'|default:notreal}}")
>>> t.render(Context())
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/opt/local/OnQueue2/OQTools/pythonlib/trunk/django/template/__init__.py", line 184, in render
    return self._render(context)
  File "/opt/local/OnQueue2/OQTools/pythonlib/trunk/django/template/__init__.py", line 178, in _render
    return self.nodelist.render(context)
  File "/opt/local/OnQueue2/OQTools/pythonlib/trunk/django/template/__init__.py", line 799, in render
    bits.append(self.render_node(node, context))
  File "/opt/local/OnQueue2/OQTools/pythonlib/trunk/django/template/debug.py", line 82, in render_node
    raise wrapped
TemplateSyntaxError: Caught an exception while rendering: Failed lookup for key [notreal] in u'[{}]'


Changed 4 years ago by copelco

add b2's patch

comment:4 Changed 4 years ago by copelco

  • Owner changed from nobody to copelco
  • Status changed from new to assigned

Changed 4 years ago by copelco

add patch with tests

comment:5 Changed 4 years ago by copelco

  • Owner changed from copelco to nobody
  • Status changed from assigned to new

comment:6 Changed 4 years ago by kmtracey

This looks to me like two different problems: the problem in the original description regarding an exception with silent_variable_failure=True not being silenced, and a different problem with non-existent variables passed to filters raising exceptions. I'd like to narrow the focus of this ticket to the first problem, which seems clearly wrong. (I'm less sure of what the right behavior is supposed to be for the 2nd situation.)

Changed 4 years ago by copelco

Only dict lookup fix and test

comment:7 Changed 4 years ago by copelco

Since we're not sure what should happen in the template filter scenario, I opened #13167.

comment:8 Changed 4 years ago by kmtracey

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

(In [12823]) Fixed #12554: Silence exceptions that have specified silent_variable_failure=True. Thanks Thomas Steinacher, copelco, mlavin.

comment:9 Changed 4 years ago by kmtracey

(In [12824]) [1.1.X] Fixed #12554: Silence exceptions that have specified silent_variable_failure=True. Thanks Thomas Steinacher, copelco, mlavin.

r12823 from trunk.

comment:10 Changed 4 years ago by anonymous

  • Resolution fixed deleted
  • Status changed from closed to reopened

When I do this:

from django.template import Context, Template
from myapp.models import MyModel

MyModel(models.Model):
    some = model.OneToOneField(AnotherModel)

obj = MyModel.objects.get(id=1)

t = Template("{{ obj.some }}")

t.render(Context({"obj": obj}))

Django raises the exception:


---------------------------------------------------------------------------
TemplateSyntaxError                       Traceback (most recent call last)

/home/alejandro/inmegen/<ipython console> in <module>()

/usr/lib/python2.6/site-packages/django/template/__init__.pyc in render(self, context)
    169         context.render_context.push()
    170         try:
--> 171             return self._render(context)
    172         finally:
    173             context.render_context.pop()

/usr/lib/python2.6/site-packages/django/template/__init__.pyc in _render(self, context)
    163 
    164     def _render(self, context):
--> 165         return self.nodelist.render(context)
    166 
    167     def render(self, context):

/usr/lib/python2.6/site-packages/django/template/__init__.pyc in render(self, context)
    795         for node in self:
    796             if isinstance(node, Node):
--> 797                 bits.append(self.render_node(node, context))
    798             else:
    799                 bits.append(node)

TemplateSyntaxError: Caught DoesNotExist while rendering: AnotherModel matching query does not exist.

Proposed patch:

Index: django/template/__init__.py
===================================================================
--- django/template/__init__.py	(revision 12823)
+++ django/template/__init__.py	(working copy)
@@ -745,6 +745,11 @@
                             TypeError,  # unsubscriptable object
                             ):
                         raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit, current)) # missing attribute
+                except Exception, e:
+                    if getattr(e, 'silent_variable_failure', False):
+                        current = settings.TEMPLATE_STRING_IF_INVALID
+                    else:
+                        raise
             except Exception, e:
                 if getattr(e, 'silent_variable_failure', False):
                     current = settings.TEMPLATE_STRING_IF_INVALID

comment:11 Changed 4 years ago by kmtracey

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

(In [12834]) Fixed #12554 again: Corrected regression in silencing attribute lookups introduced in r12823, plus added a test for this so it doesn't regress again.

comment:12 Changed 4 years ago by kmtracey

(In [12835]) [1.1.X] Fixed #12554 again: Corrected regression in silencing attribute lookups introduced in r12824, plus added a test for this so it doesn't regress again.

r12834 from trunk.

comment:13 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.