From e628c916889b32ee02c86144c35fdc9893eabf82 Mon Sep 17 00:00:00 2001 From: Dustin Martin Date: Mon, 22 Jul 2024 12:50:40 -0400 Subject: [PATCH 1/2] Make toolbar compatible with `FORCE_SCRIPT_NAME` Previously, if a project used the `FORCE_SCRIPT_NAME` setting (common when hosting a Django application on a subdirectory path via a reverse proxy), the `django.urls.resolve()` call would always raise `Resolver404` in the middleware. As a result, `is_toolbar_request()` always returned False. This caused internal toolbar URLs to be inspected, and also indirectly led to a request loop when refreshing the history panel. In most cases (if `FORCE_SCRIPT_NAME` is unset), `get_script_prefix()` will return "/" and the `replace()` will be a no-op. --- debug_toolbar/toolbar.py | 5 +++-- docs/changes.rst | 3 +++ tests/test_integration.py | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/debug_toolbar/toolbar.py b/debug_toolbar/toolbar.py index 04502ab09..7858558f5 100644 --- a/debug_toolbar/toolbar.py +++ b/debug_toolbar/toolbar.py @@ -13,7 +13,7 @@ from django.dispatch import Signal from django.template import TemplateSyntaxError from django.template.loader import render_to_string -from django.urls import include, path, re_path, resolve +from django.urls import get_script_prefix, include, path, re_path, resolve from django.urls.exceptions import Resolver404 from django.utils.module_loading import import_string from django.utils.translation import get_language, override as lang_override @@ -165,7 +165,8 @@ def is_toolbar_request(cls, request): # not have resolver_match set. try: resolver_match = request.resolver_match or resolve( - request.path, getattr(request, "urlconf", None) + request.path.replace(get_script_prefix(), "/", 1), + getattr(request, "urlconf", None), ) except Resolver404: return False diff --git a/docs/changes.rst b/docs/changes.rst index e82c598c2..ddbbdb319 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,6 +4,9 @@ Change log Pending ------- +* Fixed internal toolbar requests being instrumented if the Django setting + ``FORCE_SCRIPT_NAME`` was set. + 4.4.6 (2024-07-10) ------------------ diff --git a/tests/test_integration.py b/tests/test_integration.py index e6863e7a9..eeba37694 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -244,6 +244,23 @@ def test_is_toolbar_request_override_request_urlconf(self): self.request.path = "/__debug__/render_panel/" self.assertTrue(self.toolbar.is_toolbar_request(self.request)) + @patch("debug_toolbar.toolbar.get_script_prefix", return_value="/path/") + def test_is_toolbar_request_with_script_prefix(self, mocked_get_script_prefix): + """ + Test cases when Django is running under a path prefix, such as via the + FORCE_SCRIPT_NAME setting. + """ + self.request.path = "/path/__debug__/render_panel/" + self.assertTrue(self.toolbar.is_toolbar_request(self.request)) + + self.request.path = "/path/invalid/__debug__/render_panel/" + self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + + self.request.path = "/path/render_panel/" + self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + + self.assertEqual(mocked_get_script_prefix.call_count, 3) + def test_data_gone(self): response = self.client.get( "/__debug__/render_panel/?store_id=GONE&panel_id=RequestPanel" From fb354f1662dd0b6890e846377c1895e184a017a7 Mon Sep 17 00:00:00 2001 From: Dustin Martin Date: Mon, 22 Jul 2024 15:25:12 -0400 Subject: [PATCH 2/2] Use `path_info` when resolving requests Maintains support for `SCRIPT_NAME` without manually calling `get_script_prefix()`. Tests that involved manually setting the `path` attribute of a request have been reworked to use a `RequestFactory` so that the `path` and `path_info` attributes are both set appropriately. --- debug_toolbar/panels/request.py | 2 +- debug_toolbar/toolbar.py | 5 ++-- tests/panels/test_request.py | 21 ++++++++------ tests/test_integration.py | 50 +++++++++++++++------------------ 4 files changed, 38 insertions(+), 40 deletions(-) diff --git a/debug_toolbar/panels/request.py b/debug_toolbar/panels/request.py index 8df382fb3..f9375b381 100644 --- a/debug_toolbar/panels/request.py +++ b/debug_toolbar/panels/request.py @@ -41,7 +41,7 @@ def generate_stats(self, request, response): "view_urlname": "None", } try: - match = resolve(request.path) + match = resolve(request.path_info) func, args, kwargs = match view_info["view_func"] = get_name_from_obj(func) view_info["view_args"] = args diff --git a/debug_toolbar/toolbar.py b/debug_toolbar/toolbar.py index 7858558f5..e1b5474de 100644 --- a/debug_toolbar/toolbar.py +++ b/debug_toolbar/toolbar.py @@ -13,7 +13,7 @@ from django.dispatch import Signal from django.template import TemplateSyntaxError from django.template.loader import render_to_string -from django.urls import get_script_prefix, include, path, re_path, resolve +from django.urls import include, path, re_path, resolve from django.urls.exceptions import Resolver404 from django.utils.module_loading import import_string from django.utils.translation import get_language, override as lang_override @@ -165,8 +165,7 @@ def is_toolbar_request(cls, request): # not have resolver_match set. try: resolver_match = request.resolver_match or resolve( - request.path.replace(get_script_prefix(), "/", 1), - getattr(request, "urlconf", None), + request.path_info, getattr(request, "urlconf", None) ) except Resolver404: return False diff --git a/tests/panels/test_request.py b/tests/panels/test_request.py index ea7f1681a..316e09ed4 100644 --- a/tests/panels/test_request.py +++ b/tests/panels/test_request.py @@ -1,7 +1,10 @@ from django.http import QueryDict +from django.test import RequestFactory from ..base import BaseTestCase +rf = RequestFactory() + class RequestPanelTestCase(BaseTestCase): panel_id = "RequestPanel" @@ -13,9 +16,9 @@ def test_non_ascii_session(self): self.assertIn("où", self.panel.content) def test_object_with_non_ascii_repr_in_request_params(self): - self.request.path = "/non_ascii_request/" - response = self.panel.process_request(self.request) - self.panel.generate_stats(self.request, response) + request = rf.get("/non_ascii_request/") + response = self.panel.process_request(request) + self.panel.generate_stats(request, response) self.assertIn("nôt åscíì", self.panel.content) def test_insert_content(self): @@ -23,11 +26,11 @@ def test_insert_content(self): Test that the panel only inserts content after generate_stats and not the process_request. """ - self.request.path = "/non_ascii_request/" - response = self.panel.process_request(self.request) + request = rf.get("/non_ascii_request/") + response = self.panel.process_request(request) # ensure the panel does not have content yet. self.assertNotIn("nôt åscíì", self.panel.content) - self.panel.generate_stats(self.request, response) + self.panel.generate_stats(request, response) # ensure the panel renders correctly. content = self.panel.content self.assertIn("nôt åscíì", content) @@ -99,9 +102,9 @@ def test_list_for_request_in_method_post(self): self.assertIn("[{'a': 1}, {'b': 2}]", content) def test_namespaced_url(self): - self.request.path = "/admin/login/" - response = self.panel.process_request(self.request) - self.panel.generate_stats(self.request, response) + request = rf.get("/admin/login/") + response = self.panel.process_request(request) + self.panel.generate_stats(request, response) panel_stats = self.panel.get_stats() self.assertEqual(panel_stats["view_urlname"], "admin:login") diff --git a/tests/test_integration.py b/tests/test_integration.py index eeba37694..9571fcaca 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -128,10 +128,10 @@ def test_should_render_panels_multiprocess(self): def _resolve_stats(self, path): # takes stats from Request panel - self.request.path = path + request = rf.get(path) panel = self.toolbar.get_panel_by_id("RequestPanel") - response = panel.process_request(self.request) - panel.generate_stats(self.request, response) + response = panel.process_request(request) + panel.generate_stats(request, response) return panel.get_stats() def test_url_resolving_positional(self): @@ -215,52 +215,48 @@ def test_cache_disable_instrumentation(self): self.assertEqual(len(response.toolbar.get_panel_by_id("CachePanel").calls), 0) def test_is_toolbar_request(self): - self.request.path = "/__debug__/render_panel/" - self.assertTrue(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/__debug__/render_panel/") + self.assertTrue(self.toolbar.is_toolbar_request(request)) - self.request.path = "/invalid/__debug__/render_panel/" - self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/invalid/__debug__/render_panel/") + self.assertFalse(self.toolbar.is_toolbar_request(request)) - self.request.path = "/render_panel/" - self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/render_panel/") + self.assertFalse(self.toolbar.is_toolbar_request(request)) @override_settings(ROOT_URLCONF="tests.urls_invalid") def test_is_toolbar_request_without_djdt_urls(self): """Test cases when the toolbar urls aren't configured.""" - self.request.path = "/__debug__/render_panel/" - self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/__debug__/render_panel/") + self.assertFalse(self.toolbar.is_toolbar_request(request)) - self.request.path = "/render_panel/" - self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/render_panel/") + self.assertFalse(self.toolbar.is_toolbar_request(request)) @override_settings(ROOT_URLCONF="tests.urls_invalid") def test_is_toolbar_request_override_request_urlconf(self): """Test cases when the toolbar URL is configured on the request.""" - self.request.path = "/__debug__/render_panel/" - self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/__debug__/render_panel/") + self.assertFalse(self.toolbar.is_toolbar_request(request)) # Verify overriding the urlconf on the request is valid. - self.request.urlconf = "tests.urls" - self.request.path = "/__debug__/render_panel/" - self.assertTrue(self.toolbar.is_toolbar_request(self.request)) + request.urlconf = "tests.urls" + self.assertTrue(self.toolbar.is_toolbar_request(request)) - @patch("debug_toolbar.toolbar.get_script_prefix", return_value="/path/") - def test_is_toolbar_request_with_script_prefix(self, mocked_get_script_prefix): + def test_is_toolbar_request_with_script_prefix(self): """ Test cases when Django is running under a path prefix, such as via the FORCE_SCRIPT_NAME setting. """ - self.request.path = "/path/__debug__/render_panel/" - self.assertTrue(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/__debug__/render_panel/", SCRIPT_NAME="/path/") + self.assertTrue(self.toolbar.is_toolbar_request(request)) - self.request.path = "/path/invalid/__debug__/render_panel/" - self.assertFalse(self.toolbar.is_toolbar_request(self.request)) + request = rf.get("/invalid/__debug__/render_panel/", SCRIPT_NAME="/path/") + self.assertFalse(self.toolbar.is_toolbar_request(request)) - self.request.path = "/path/render_panel/" + request = rf.get("/render_panel/", SCRIPT_NAME="/path/") self.assertFalse(self.toolbar.is_toolbar_request(self.request)) - self.assertEqual(mocked_get_script_prefix.call_count, 3) - def test_data_gone(self): response = self.client.get( "/__debug__/render_panel/?store_id=GONE&panel_id=RequestPanel"