From f7fc3d938192d5e80fbb37995ed557b56386d9f8 Mon Sep 17 00:00:00 2001 From: tschilling Date: Sat, 25 Oct 2014 10:38:48 -0400 Subject: [PATCH] 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 #598. --- debug_toolbar/panels/cache.py | 15 +------------- debug_toolbar/panels/sql/tracking.py | 17 +--------------- debug_toolbar/utils.py | 30 +++++++++++++++++++++++++++- tests/loaders.py | 10 ++++++++++ tests/panels/test_sql.py | 30 ++++++++++++++++++++++++++++ tests/templates/base.html | 9 +++++++++ tests/templates/basic.html | 10 ++-------- 7 files changed, 82 insertions(+), 39 deletions(-) create mode 100644 tests/loaders.py create mode 100644 tests/templates/base.html diff --git a/debug_toolbar/panels/cache.py b/debug_toolbar/panels/cache.py index 443f25beb..1731fa4b8 100644 --- a/debug_toolbar/panels/cache.py +++ b/debug_toolbar/panels/cache.py @@ -9,7 +9,6 @@ from django.core.cache import cache as original_cache, get_cache as original_get_cache from django.core.cache.backends.base import BaseCache from django.dispatch import Signal -from django.template import Node from django.utils.translation import ugettext_lazy as _, ungettext try: from collections import OrderedDict @@ -37,19 +36,7 @@ def wrapped(self, *args, **kwargs): else: stacktrace = [] - template_info = None - cur_frame = sys._getframe().f_back - try: - while cur_frame is not None: - if cur_frame.f_code.co_name == 'render': - node = cur_frame.f_locals['self'] - if isinstance(node, Node): - template_info = get_template_info(node.source) - break - cur_frame = cur_frame.f_back - except Exception: - pass - del cur_frame + template_info = get_template_info() cache_called.send(sender=self.__class__, time_taken=t, name=method.__name__, return_value=value, args=args, kwargs=kwargs, trace=stacktrace, diff --git a/debug_toolbar/panels/sql/tracking.py b/debug_toolbar/panels/sql/tracking.py index b6a787d92..f2bc8a7e3 100644 --- a/debug_toolbar/panels/sql/tracking.py +++ b/debug_toolbar/panels/sql/tracking.py @@ -1,12 +1,9 @@ from __future__ import absolute_import, unicode_literals -import sys - import json from threading import local from time import time -from django.template import Node from django.utils.encoding import force_text from django.utils import six @@ -115,19 +112,7 @@ def _record(self, method, sql, params): except Exception: pass # object not JSON serializable - template_info = None - cur_frame = sys._getframe().f_back - try: - while cur_frame is not None: - if cur_frame.f_code.co_name == 'render': - node = cur_frame.f_locals['self'] - if isinstance(node, Node): - template_info = get_template_info(node.source) - break - cur_frame = cur_frame.f_back - except Exception: - pass - del cur_frame + template_info = get_template_info() alias = getattr(self.db, 'alias', 'default') conn = self.db.connection diff --git a/debug_toolbar/utils.py b/debug_toolbar/utils.py index 05fa68104..2633605f4 100644 --- a/debug_toolbar/utils.py +++ b/debug_toolbar/utils.py @@ -15,6 +15,7 @@ import django from django.core.exceptions import ImproperlyConfigured +from django.template import Node from django.utils.encoding import force_text from django.utils.html import escape from django.utils.safestring import mark_safe @@ -86,7 +87,34 @@ def render_stacktrace(trace): return mark_safe('\n'.join(stacktrace)) -def get_template_info(source, context_lines=3): +def get_template_info(): + template_info = None + cur_frame = sys._getframe().f_back + try: + while cur_frame is not None: + in_utils_module = cur_frame.f_code.co_filename.endswith( + "/debug_toolbar/utils.py" + ) + is_get_template_context = ( + cur_frame.f_code.co_name == get_template_context.__name__ + ) + if in_utils_module and is_get_template_context: + # If the method in the stack trace is this one + # then break from the loop as it's being check recursively. + break + elif cur_frame.f_code.co_name == 'render': + node = cur_frame.f_locals['self'] + if isinstance(node, Node): + template_info = get_template_context(node.source) + break + cur_frame = cur_frame.f_back + except Exception: + pass + del cur_frame + return template_info + + +def get_template_context(source, context_lines=3): line = 0 upto = 0 source_lines = [] diff --git a/tests/loaders.py b/tests/loaders.py new file mode 100644 index 000000000..8a5bfc2ba --- /dev/null +++ b/tests/loaders.py @@ -0,0 +1,10 @@ +from django.contrib.auth.models import User +from django.template.loaders.app_directories import Loader + + +class LoaderWithSQL(Loader): + def load_template_source(self, template_name, template_dirs=None): + # Force the template loader to run some SQL. Simulates a CMS. + User.objects.all().count() + return super(LoaderWithSQL, self).load_template_source( + template_name, template_dirs=template_dirs) diff --git a/tests/panels/test_sql.py b/tests/panels/test_sql.py index ece533c68..0203b40b5 100644 --- a/tests/panels/test_sql.py +++ b/tests/panels/test_sql.py @@ -2,10 +2,13 @@ from __future__ import absolute_import, unicode_literals +import django from django.contrib.auth.models import User from django.db import connection from django.db.utils import DatabaseError +from django.shortcuts import render from django.utils import unittest +from django.test.utils import override_settings from ..base import BaseTestCase @@ -84,3 +87,30 @@ def test_disable_stacktraces(self): # ensure the stacktrace is empty self.assertEqual([], query[1]['stacktrace']) + + @unittest.skipIf(django.VERSION < (1, 5), + "Django 1.4 loads the TEMPLATE_LOADERS before " + "override_settings can modify the settings.") + @override_settings(DEBUG=True, TEMPLATE_DEBUG=True, + TEMPLATE_LOADERS=('tests.loaders.LoaderWithSQL',)) + def test_regression_infinite_recursion(self): + """ + Test case for when the template loader runs a SQL query that causes + an infinite recursion in the SQL panel. + """ + self.assertEqual(len(self.panel._queries), 0) + + render(self.request, "basic.html", {}) + + # ensure queries were logged + # It's more than one because the SQL run in the loader is run every time + # the template is rendered which is more than once. + self.assertEqual(len(self.panel._queries), 3) + query = self.panel._queries[0] + self.assertEqual(query[0], 'default') + self.assertTrue('sql' in query[1]) + self.assertTrue('duration' in query[1]) + self.assertTrue('stacktrace' in query[1]) + + # ensure the stacktrace is populated + self.assertTrue(len(query[1]['stacktrace']) > 0) diff --git a/tests/templates/base.html b/tests/templates/base.html new file mode 100644 index 000000000..ea0d773ac --- /dev/null +++ b/tests/templates/base.html @@ -0,0 +1,9 @@ + + + + {{ title }} + + + {% block content %}{% endblock %} + + diff --git a/tests/templates/basic.html b/tests/templates/basic.html index bfdc973ac..46f88e4da 100644 --- a/tests/templates/basic.html +++ b/tests/templates/basic.html @@ -1,8 +1,2 @@ - - - - {{ title }} - - - - +{% extends "base.html" %} +{% block content %}Test for {{ title }}{% endblock %}