Skip to content

Commit f7fc3d9

Browse files
committed
Fix infinite recursion bug with template loaders that use SQL.
Move the get_template_info stacktrace loop into a utility method. Check for calls to get_template_context to prevent the infinite recursion. The bug requires TEMPLATE_DEBUG=True, for the template to extend another template and SQL to be run in the template loader. With this configuration the SQL Panel would identify the SQL query while loading the template, then try to load the template again which would create a new SQL query ending in an infinite recursion. Skip the infinite recursion regression test for Django < 1.5 The test is being skipped because Django 1.4 loads the TEMPLATE_LOADERS before override_settings can modify it. This means that the SQL query isn't run when loading the template which causes the test to fail. The other solution would have been to add a conditional setting, but that could introduce unexpected results in future tests. Skipping this test is the safer, more conservative approach. Closes django-commons#598.
1 parent 1afcb96 commit f7fc3d9

File tree

7 files changed

+82
-39
lines changed

7 files changed

+82
-39
lines changed

debug_toolbar/panels/cache.py

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from django.core.cache import cache as original_cache, get_cache as original_get_cache
1010
from django.core.cache.backends.base import BaseCache
1111
from django.dispatch import Signal
12-
from django.template import Node
1312
from django.utils.translation import ugettext_lazy as _, ungettext
1413
try:
1514
from collections import OrderedDict
@@ -37,19 +36,7 @@ def wrapped(self, *args, **kwargs):
3736
else:
3837
stacktrace = []
3938

40-
template_info = None
41-
cur_frame = sys._getframe().f_back
42-
try:
43-
while cur_frame is not None:
44-
if cur_frame.f_code.co_name == 'render':
45-
node = cur_frame.f_locals['self']
46-
if isinstance(node, Node):
47-
template_info = get_template_info(node.source)
48-
break
49-
cur_frame = cur_frame.f_back
50-
except Exception:
51-
pass
52-
del cur_frame
39+
template_info = get_template_info()
5340
cache_called.send(sender=self.__class__, time_taken=t,
5441
name=method.__name__, return_value=value,
5542
args=args, kwargs=kwargs, trace=stacktrace,

debug_toolbar/panels/sql/tracking.py

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
from __future__ import absolute_import, unicode_literals
22

3-
import sys
4-
53
import json
64
from threading import local
75
from time import time
86

9-
from django.template import Node
107
from django.utils.encoding import force_text
118
from django.utils import six
129

@@ -115,19 +112,7 @@ def _record(self, method, sql, params):
115112
except Exception:
116113
pass # object not JSON serializable
117114

118-
template_info = None
119-
cur_frame = sys._getframe().f_back
120-
try:
121-
while cur_frame is not None:
122-
if cur_frame.f_code.co_name == 'render':
123-
node = cur_frame.f_locals['self']
124-
if isinstance(node, Node):
125-
template_info = get_template_info(node.source)
126-
break
127-
cur_frame = cur_frame.f_back
128-
except Exception:
129-
pass
130-
del cur_frame
115+
template_info = get_template_info()
131116

132117
alias = getattr(self.db, 'alias', 'default')
133118
conn = self.db.connection

debug_toolbar/utils.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import django
1717
from django.core.exceptions import ImproperlyConfigured
18+
from django.template import Node
1819
from django.utils.encoding import force_text
1920
from django.utils.html import escape
2021
from django.utils.safestring import mark_safe
@@ -86,7 +87,34 @@ def render_stacktrace(trace):
8687
return mark_safe('\n'.join(stacktrace))
8788

8889

89-
def get_template_info(source, context_lines=3):
90+
def get_template_info():
91+
template_info = None
92+
cur_frame = sys._getframe().f_back
93+
try:
94+
while cur_frame is not None:
95+
in_utils_module = cur_frame.f_code.co_filename.endswith(
96+
"/debug_toolbar/utils.py"
97+
)
98+
is_get_template_context = (
99+
cur_frame.f_code.co_name == get_template_context.__name__
100+
)
101+
if in_utils_module and is_get_template_context:
102+
# If the method in the stack trace is this one
103+
# then break from the loop as it's being check recursively.
104+
break
105+
elif cur_frame.f_code.co_name == 'render':
106+
node = cur_frame.f_locals['self']
107+
if isinstance(node, Node):
108+
template_info = get_template_context(node.source)
109+
break
110+
cur_frame = cur_frame.f_back
111+
except Exception:
112+
pass
113+
del cur_frame
114+
return template_info
115+
116+
117+
def get_template_context(source, context_lines=3):
90118
line = 0
91119
upto = 0
92120
source_lines = []

tests/loaders.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
from django.contrib.auth.models import User
2+
from django.template.loaders.app_directories import Loader
3+
4+
5+
class LoaderWithSQL(Loader):
6+
def load_template_source(self, template_name, template_dirs=None):
7+
# Force the template loader to run some SQL. Simulates a CMS.
8+
User.objects.all().count()
9+
return super(LoaderWithSQL, self).load_template_source(
10+
template_name, template_dirs=template_dirs)

tests/panels/test_sql.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22

33
from __future__ import absolute_import, unicode_literals
44

5+
import django
56
from django.contrib.auth.models import User
67
from django.db import connection
78
from django.db.utils import DatabaseError
9+
from django.shortcuts import render
810
from django.utils import unittest
11+
from django.test.utils import override_settings
912

1013
from ..base import BaseTestCase
1114

@@ -84,3 +87,30 @@ def test_disable_stacktraces(self):
8487

8588
# ensure the stacktrace is empty
8689
self.assertEqual([], query[1]['stacktrace'])
90+
91+
@unittest.skipIf(django.VERSION < (1, 5),
92+
"Django 1.4 loads the TEMPLATE_LOADERS before "
93+
"override_settings can modify the settings.")
94+
@override_settings(DEBUG=True, TEMPLATE_DEBUG=True,
95+
TEMPLATE_LOADERS=('tests.loaders.LoaderWithSQL',))
96+
def test_regression_infinite_recursion(self):
97+
"""
98+
Test case for when the template loader runs a SQL query that causes
99+
an infinite recursion in the SQL panel.
100+
"""
101+
self.assertEqual(len(self.panel._queries), 0)
102+
103+
render(self.request, "basic.html", {})
104+
105+
# ensure queries were logged
106+
# It's more than one because the SQL run in the loader is run every time
107+
# the template is rendered which is more than once.
108+
self.assertEqual(len(self.panel._queries), 3)
109+
query = self.panel._queries[0]
110+
self.assertEqual(query[0], 'default')
111+
self.assertTrue('sql' in query[1])
112+
self.assertTrue('duration' in query[1])
113+
self.assertTrue('stacktrace' in query[1])
114+
115+
# ensure the stacktrace is populated
116+
self.assertTrue(len(query[1]['stacktrace']) > 0)

tests/templates/base.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>{{ title }}</title>
5+
</head>
6+
<body>
7+
{% block content %}{% endblock %}
8+
</body>
9+
</html>

tests/templates/basic.html

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,2 @@
1-
<!DOCTYPE html>
2-
<html>
3-
<head>
4-
<title>{{ title }}</title>
5-
</head>
6-
<body>
7-
</body>
8-
</html>
1+
{% extends "base.html" %}
2+
{% block content %}Test for {{ title }}{% endblock %}

0 commit comments

Comments
 (0)