Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #13373 -- Ensured that {% if %} statements will short circuit t…

…emplate logic and not evaluate clauses that don't require evaluation. Thanks to Jerry Stratton for the report.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@13001 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit fef0d25bdccbb8f01314cf3a94651d3cfe67a34e 1 parent ebfe938
Russell Keith-Magee authored April 19, 2010
33  django/template/smartif.py
@@ -56,7 +56,7 @@ def led(self, left, parser):
56 56
 
57 57
         def eval(self, context):
58 58
             try:
59  
-                return func(self.first.eval(context), self.second.eval(context))
  59
+                return func(context, self.first, self.second)
60 60
             except Exception:
61 61
                 # Templates shouldn't throw exceptions when rendering.  We are
62 62
                 # most likely to get exceptions for things like {% if foo in bar
@@ -81,7 +81,7 @@ def nud(self, parser):
81 81
 
82 82
         def eval(self, context):
83 83
             try:
84  
-                return func(self.first.eval(context))
  84
+                return func(context, self.first)
85 85
             except Exception:
86 86
                 return False
87 87
 
@@ -91,20 +91,21 @@ def eval(self, context):
91 91
 # Operator precedence follows Python.
92 92
 # NB - we can get slightly more accurate syntax error messages by not using the
93 93
 # same object for '==' and '='.
94  
-
  94
+# We defer variable evaluation to the lambda to ensure that terms are
  95
+# lazily evaluated using Python's boolean parsing logic.
95 96
 OPERATORS = {
96  
-    'or': infix(6, lambda x, y: x or y),
97  
-    'and': infix(7, lambda x, y: x and y),
98  
-    'not': prefix(8, operator.not_),
99  
-    'in': infix(9, lambda x, y: x in y),
100  
-    'not in': infix(9, lambda x, y: x not in y),
101  
-    '=': infix(10, operator.eq),
102  
-    '==': infix(10, operator.eq),
103  
-    '!=': infix(10, operator.ne),
104  
-    '>': infix(10, operator.gt),
105  
-    '>=': infix(10, operator.ge),
106  
-    '<': infix(10, operator.lt),
107  
-    '<=': infix(10, operator.le),
  97
+    'or': infix(6, lambda context, x, y: x.eval(context) or y.eval(context)),
  98
+    'and': infix(7, lambda context, x, y: x.eval(context) and y.eval(context)),
  99
+    'not': prefix(8, lambda context, x: not x.eval(context)),
  100
+    'in': infix(9, lambda context, x, y: x.eval(context) in y.eval(context)),
  101
+    'not in': infix(9, lambda context, x, y: x.eval(context) not in y.eval(context)),
  102
+    '=': infix(10, lambda context, x, y: x.eval(context) == y.eval(context)),
  103
+    '==': infix(10, lambda context, x, y: x.eval(context) == y.eval(context)),
  104
+    '!=': infix(10, lambda context, x, y: x.eval(context) != y.eval(context)),
  105
+    '>': infix(10, lambda context, x, y: x.eval(context) > y.eval(context)),
  106
+    '>=': infix(10, lambda context, x, y: x.eval(context) >= y.eval(context)),
  107
+    '<': infix(10, lambda context, x, y: x.eval(context) < y.eval(context)),
  108
+    '<=': infix(10, lambda context, x, y: x.eval(context) <= y.eval(context)),
108 109
 }
109 110
 
110 111
 # Assign 'id' to each:
@@ -151,7 +152,7 @@ class IfParser(object):
151 152
     error_class = ValueError
152 153
 
153 154
     def __init__(self, tokens):
154  
-        # pre-pass necessary to turn  'not','in' into single token 
  155
+        # pre-pass necessary to turn  'not','in' into single token
155 156
         l = len(tokens)
156 157
         mapped_tokens = []
157 158
         i = 0
31  tests/regressiontests/templates/tests.py
@@ -7,6 +7,7 @@
7 7
     settings.configure()
8 8
 
9 9
 from datetime import datetime, timedelta
  10
+import time
10 11
 import os
11 12
 import sys
12 13
 import traceback
@@ -97,6 +98,17 @@ class OtherClass:
97 98
     def method(self):
98 99
         return "OtherClass.method"
99 100
 
  101
+class TestObj(object):
  102
+    def is_true(self):
  103
+        return True
  104
+
  105
+    def is_false(self):
  106
+        return False
  107
+
  108
+    def is_bad(self):
  109
+        time.sleep(0.3)
  110
+        return True
  111
+
100 112
 class SilentGetItemClass(object):
101 113
     def __getitem__(self, key):
102 114
         raise SomeException
@@ -342,6 +354,11 @@ def test_template_loader(template_name, template_dirs=None):
342 354
         old_invalid = settings.TEMPLATE_STRING_IF_INVALID
343 355
         expected_invalid_str = 'INVALID'
344 356
 
  357
+        # Warm the URL reversing cache. This ensures we don't pay the cost
  358
+        # warming the cache during one of the tests.
  359
+        urlresolvers.reverse('regressiontests.templates.views.client_action',
  360
+                             kwargs={'id':0,'action':"update"})
  361
+
345 362
         for name, vals in tests:
346 363
             if isinstance(vals[2], tuple):
347 364
                 normal_string_result = vals[2][0]
@@ -367,9 +384,14 @@ def test_template_loader(template_name, template_dirs=None):
367 384
                         start = datetime.now()
368 385
                         test_template = loader.get_template(name)
369 386
                         end = datetime.now()
370  
-                        output = self.render(test_template, vals)
371 387
                         if end-start > timedelta(seconds=0.2):
372 388
                             failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s'): %s -- FAILED. Took too long to parse test" % (is_cached, invalid_str, name))
  389
+
  390
+                        start = datetime.now()
  391
+                        output = self.render(test_template, vals)
  392
+                        end = datetime.now()
  393
+                        if end-start > timedelta(seconds=0.2):
  394
+                            failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s'): %s -- FAILED. Took too long to render test" % (is_cached, invalid_str, name))
373 395
                     except ContextStackException:
374 396
                         failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s'): %s -- FAILED. Context stack was left imbalanced" % (is_cached, invalid_str, name))
375 397
                         continue
@@ -782,6 +804,13 @@ def get_template_tests(self):
782 804
             'if-tag-error11': ("{% if 1 == %}yes{% endif %}", {}, template.TemplateSyntaxError),
783 805
             'if-tag-error12': ("{% if a not b %}yes{% endif %}", {}, template.TemplateSyntaxError),
784 806
 
  807
+            # If evaluations are shortcircuited where possible
  808
+            # These tests will fail by taking too long to run. When the if clause
  809
+            # is shortcircuiting correctly, the is_bad() function shouldn't be
  810
+            # evaluated, and the deliberate sleep won't happen.
  811
+            'if-tag-shortcircuit01': ('{% if x.is_true or x.is_bad %}yes{% else %}no{% endif %}', {'x': TestObj()}, "yes"),
  812
+            'if-tag-shortcircuit02': ('{% if x.is_false and x.is_bad %}yes{% else %}no{% endif %}', {'x': TestObj()}, "no"),
  813
+
785 814
             # Non-existent args
786 815
             'if-tag-badarg01':("{% if x|default_if_none:y %}yes{% endif %}", {}, ''),
787 816
             'if-tag-badarg02':("{% if x|default_if_none:y %}yes{% endif %}", {'y': 0}, ''),

0 notes on commit fef0d25

Please sign in to comment.
Something went wrong with that request. Please try again.