From a0f0df97d6b366fb7179dcb03183983cae25efa8 Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Sat, 1 Jun 2024 08:49:07 -0300 Subject: [PATCH 01/12] Fix overriding font-family for both light and dark themes (#1930) --- debug_toolbar/static/debug_toolbar/css/toolbar.css | 6 ++++-- docs/changes.rst | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/debug_toolbar/static/debug_toolbar/css/toolbar.css b/debug_toolbar/static/debug_toolbar/css/toolbar.css index e028a67b7..c7e201d07 100644 --- a/debug_toolbar/static/debug_toolbar/css/toolbar.css +++ b/debug_toolbar/static/debug_toolbar/css/toolbar.css @@ -1,6 +1,5 @@ /* Variable definitions */ -:root, -#djDebug[data-theme="light"] { +:root { /* Font families are the same as in Django admin/css/base.css */ --djdt-font-family-primary: "Segoe UI", system-ui, Roboto, "Helvetica Neue", Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", @@ -10,7 +9,10 @@ "Source Code Pro", "Fira Mono", "Droid Sans Mono", "Courier New", monospace, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji"; +} +:root, +#djDebug[data-theme="light"] { --djdt-font-color: black; --djdt-background-color: white; --djdt-panel-content-background-color: #eee; diff --git a/docs/changes.rst b/docs/changes.rst index a0215d09c..20272fda7 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,6 +4,8 @@ Change log Pending ------- +* Fix overriding font-family for both light and dark themes. + 4.4.2 (2024-05-27) ------------------ From cf0cc0dfe86e134ea2a0352a4f0833efbcf9f2c4 Mon Sep 17 00:00:00 2001 From: Ceesjan Luiten Date: Sat, 1 Jun 2024 13:49:49 +0200 Subject: [PATCH 02/12] Restore compatibility with iptools.IpRangeList (#1929) --- debug_toolbar/middleware.py | 15 ++++++++++++--- tests/test_integration.py | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/debug_toolbar/middleware.py b/debug_toolbar/middleware.py index 0513e2379..65b5282c5 100644 --- a/debug_toolbar/middleware.py +++ b/debug_toolbar/middleware.py @@ -20,8 +20,14 @@ def show_toolbar(request): """ Default function to determine whether to show the toolbar on a given page. """ - internal_ips = list(settings.INTERNAL_IPS) + if not settings.DEBUG: + return False + # Test: settings + if request.META.get("REMOTE_ADDR") in settings.INTERNAL_IPS: + return True + + # Test: Docker try: # This is a hack for docker installations. It attempts to look # up the IP address of the docker host. @@ -31,11 +37,14 @@ def show_toolbar(request): ".".join(socket.gethostbyname("host.docker.internal").rsplit(".")[:-1]) + ".1" ) - internal_ips.append(docker_ip) + if request.META.get("REMOTE_ADDR") == docker_ip: + return True except socket.gaierror: # It's fine if the lookup errored since they may not be using docker pass - return settings.DEBUG and request.META.get("REMOTE_ADDR") in internal_ips + + # No test passed + return False @lru_cache(maxsize=None) diff --git a/tests/test_integration.py b/tests/test_integration.py index 465804651..71525affe 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -75,6 +75,28 @@ def test_show_toolbar_docker(self, mocked_gethostbyname): self.assertTrue(show_toolbar(self.request)) mocked_gethostbyname.assert_called_once_with("host.docker.internal") + def test_not_iterating_over_INTERNAL_IPS(self): + """Verify that the middleware does not iterate over INTERNAL_IPS in some way. + + Some people use iptools.IpRangeList for their INTERNAL_IPS. This is a class + that can quickly answer the question if the setting contain a certain IP address, + but iterating over this object will drain all performance / blow up. + """ + + class FailOnIteration: + def __iter__(self): + raise RuntimeError( + "The testcase failed: the code should not have iterated over INTERNAL_IPS" + ) + + def __contains__(self, x): + return True + + with self.settings(INTERNAL_IPS=FailOnIteration()): + response = self.client.get("/regular/basic/") + self.assertEqual(response.status_code, 200) + self.assertContains(response, "djDebug") # toolbar + def test_should_render_panels_RENDER_PANELS(self): """ The toolbar should force rendering panels on each request From 9c4eb6778899eb796cd97c92fae9fdcece34939e Mon Sep 17 00:00:00 2001 From: Matthias Kestenholz Date: Sat, 1 Jun 2024 13:50:28 +0200 Subject: [PATCH 03/12] Amend the changelog --- docs/changes.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changes.rst b/docs/changes.rst index 20272fda7..2eaece240 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,7 +4,8 @@ Change log Pending ------- -* Fix overriding font-family for both light and dark themes. +* Fixed overriding font-family for both light and dark themes. +* Restored compatibility with ``iptools.IpRangeList``. 4.4.2 (2024-05-27) ------------------ From b5cf78460d8a36d83ff8aadb97f01a6346527632 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 10 Jun 2024 19:53:44 +0200 Subject: [PATCH 04/12] [pre-commit.ci] pre-commit autoupdate (#1931) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/adamchainz/django-upgrade: 1.17.0 → 1.18.0](https://github.com/adamchainz/django-upgrade/compare/1.17.0...1.18.0) - [github.com/pre-commit/mirrors-eslint: v9.3.0 → v9.4.0](https://github.com/pre-commit/mirrors-eslint/compare/v9.3.0...v9.4.0) - [github.com/astral-sh/ruff-pre-commit: v0.4.5 → v0.4.8](https://github.com/astral-sh/ruff-pre-commit/compare/v0.4.5...v0.4.8) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index af8ab8b6f..85d167b43 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,7 @@ repos: hooks: - id: doc8 - repo: https://github.com/adamchainz/django-upgrade - rev: 1.17.0 + rev: 1.18.0 hooks: - id: django-upgrade args: [--target-version, "4.2"] @@ -32,7 +32,7 @@ repos: args: - --trailing-comma=es5 - repo: https://github.com/pre-commit/mirrors-eslint - rev: v9.3.0 + rev: v9.4.0 hooks: - id: eslint additional_dependencies: @@ -44,7 +44,7 @@ repos: args: - --fix - repo: https://github.com/astral-sh/ruff-pre-commit - rev: 'v0.4.5' + rev: 'v0.4.8' hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] From 0a622dcab4c78ce0f1d339f0e3147a1023350efa Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 17 Jun 2024 19:43:35 +0200 Subject: [PATCH 05/12] [pre-commit.ci] pre-commit autoupdate (#1934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-eslint: v9.4.0 → v9.5.0](https://github.com/pre-commit/mirrors-eslint/compare/v9.4.0...v9.5.0) - [github.com/astral-sh/ruff-pre-commit: v0.4.8 → v0.4.9](https://github.com/astral-sh/ruff-pre-commit/compare/v0.4.8...v0.4.9) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 85d167b43..04399e7cc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: args: - --trailing-comma=es5 - repo: https://github.com/pre-commit/mirrors-eslint - rev: v9.4.0 + rev: v9.5.0 hooks: - id: eslint additional_dependencies: @@ -44,7 +44,7 @@ repos: args: - --fix - repo: https://github.com/astral-sh/ruff-pre-commit - rev: 'v0.4.8' + rev: 'v0.4.9' hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] From 2b0790527a20354123b0f7fa093a0ec7a41bbe5a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 24 Jun 2024 20:01:00 +0200 Subject: [PATCH 06/12] [pre-commit.ci] pre-commit autoupdate (#1939) --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 04399e7cc..62d50b2d8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -44,7 +44,7 @@ repos: args: - --fix - repo: https://github.com/astral-sh/ruff-pre-commit - rev: 'v0.4.9' + rev: 'v0.4.10' hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] From c54f898c0900c1bd94c29f9896ed97730ba812f7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 1 Jul 2024 20:04:17 +0200 Subject: [PATCH 07/12] [pre-commit.ci] pre-commit autoupdate (#1940) --- .pre-commit-config.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 62d50b2d8..54a49e4d6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,7 @@ repos: hooks: - id: doc8 - repo: https://github.com/adamchainz/django-upgrade - rev: 1.18.0 + rev: 1.19.0 hooks: - id: django-upgrade args: [--target-version, "4.2"] @@ -32,7 +32,7 @@ repos: args: - --trailing-comma=es5 - repo: https://github.com/pre-commit/mirrors-eslint - rev: v9.5.0 + rev: v9.6.0 hooks: - id: eslint additional_dependencies: @@ -44,7 +44,7 @@ repos: args: - --fix - repo: https://github.com/astral-sh/ruff-pre-commit - rev: 'v0.4.10' + rev: 'v0.5.0' hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] From 2d9c6a7cdd448429047623ab4985c9ef984b9735 Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Wed, 3 Jul 2024 11:06:58 -0500 Subject: [PATCH 08/12] Limit the cases for E001 to likely scenarios (#1925) Only when the user is customizing both the show toolbar callback setting and the URLs aren't installed will the underlying NoReverseMatch error occur. --- debug_toolbar/apps.py | 32 +++++++++++++- docs/changes.rst | 3 ++ docs/configuration.rst | 10 +++++ tests/test_checks.py | 99 +++++++++++++++++++++++++++--------------- 4 files changed, 108 insertions(+), 36 deletions(-) diff --git a/debug_toolbar/apps.py b/debug_toolbar/apps.py index a2e977d84..a49875bac 100644 --- a/debug_toolbar/apps.py +++ b/debug_toolbar/apps.py @@ -5,10 +5,12 @@ from django.conf import settings from django.core.checks import Error, Warning, register from django.middleware.gzip import GZipMiddleware +from django.urls import NoReverseMatch, reverse from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ -from debug_toolbar import settings as dt_settings +from debug_toolbar import APP_NAME, settings as dt_settings +from debug_toolbar.settings import CONFIG_DEFAULTS class DebugToolbarConfig(AppConfig): @@ -213,7 +215,33 @@ def debug_toolbar_installed_when_running_tests_check(app_configs, **kwargs): """ Check that the toolbar is not being used when tests are running """ - if not settings.DEBUG and dt_settings.get_config()["IS_RUNNING_TESTS"]: + # Check if show toolbar callback has changed + show_toolbar_changed = ( + dt_settings.get_config()["SHOW_TOOLBAR_CALLBACK"] + != CONFIG_DEFAULTS["SHOW_TOOLBAR_CALLBACK"] + ) + try: + # Check if the toolbar's urls are installed + reverse(f"{APP_NAME}:render_panel") + toolbar_urls_installed = True + except NoReverseMatch: + toolbar_urls_installed = False + + # If the user is using the default SHOW_TOOLBAR_CALLBACK, + # then the middleware will respect the change to settings.DEBUG. + # However, if the user has changed the callback to: + # DEBUG_TOOLBAR_CONFIG = {"SHOW_TOOLBAR_CALLBACK": lambda request: DEBUG} + # where DEBUG is not settings.DEBUG, then it won't pick up that Django' + # test runner has changed the value for settings.DEBUG, and the middleware + # will inject the toolbar, while the URLs aren't configured leading to a + # NoReverseMatch error. + likely_error_setup = show_toolbar_changed and not toolbar_urls_installed + + if ( + not settings.DEBUG + and dt_settings.get_config()["IS_RUNNING_TESTS"] + and likely_error_setup + ): return [ Error( "The Django Debug Toolbar can't be used with tests", diff --git a/docs/changes.rst b/docs/changes.rst index 2eaece240..6dbfe829a 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -6,6 +6,9 @@ Pending * Fixed overriding font-family for both light and dark themes. * Restored compatibility with ``iptools.IpRangeList``. +* Limit ``E001`` check to likely error cases when the + ``SHOW_TOOLBAR_CALLBACK`` has changed, but the toolbar's URL + paths aren't installed. 4.4.2 (2024-05-27) ------------------ diff --git a/docs/configuration.rst b/docs/configuration.rst index 04694aceb..80e44984f 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -152,6 +152,16 @@ Toolbar options implication is that it is possible to execute arbitrary SQL through the SQL panel when the ``SECRET_KEY`` value is leaked somehow. + .. warning:: + + Do not use + ``DEBUG_TOOLBAR_CONFIG = {"SHOW_TOOLBAR_CALLBACK": lambda request: DEBUG}`` + in your project's settings.py file. The toolbar expects to use + ``django.conf.settings.DEBUG``. Using your project's setting's ``DEBUG`` + is likely to cause unexpected results when running your tests. This is because + Django automatically sets ``settings.DEBUG = False``, but your project's + setting's ``DEBUG`` will still be set to ``True``. + .. _OBSERVE_REQUEST_CALLBACK: * ``OBSERVE_REQUEST_CALLBACK`` diff --git a/tests/test_checks.py b/tests/test_checks.py index 827886db1..27db92a9d 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -1,9 +1,9 @@ from unittest.mock import patch -from django.core.checks import Error, Warning, run_checks +from django.core.checks import Warning, run_checks from django.test import SimpleTestCase, override_settings +from django.urls import NoReverseMatch -from debug_toolbar import settings as dt_settings from debug_toolbar.apps import debug_toolbar_installed_when_running_tests_check @@ -239,39 +239,70 @@ def test_check_w007_invalid(self, mocked_guess_type): ], ) - def test_debug_toolbar_installed_when_running_tests(self): - with self.settings(DEBUG=True): - # Update the config options because self.settings() - # would require redefining DEBUG_TOOLBAR_CONFIG entirely. - dt_settings.get_config()["IS_RUNNING_TESTS"] = True - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual(len(errors), 0) - - dt_settings.get_config()["IS_RUNNING_TESTS"] = False - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual(len(errors), 0) - with self.settings(DEBUG=False): - dt_settings.get_config()["IS_RUNNING_TESTS"] = False - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual(len(errors), 0) + @patch("debug_toolbar.apps.reverse") + def test_debug_toolbar_installed_when_running_tests(self, reverse): + params = [ + { + "debug": True, + "running_tests": True, + "show_callback_changed": True, + "urls_installed": False, + "errors": False, + }, + { + "debug": False, + "running_tests": False, + "show_callback_changed": True, + "urls_installed": False, + "errors": False, + }, + { + "debug": False, + "running_tests": True, + "show_callback_changed": False, + "urls_installed": False, + "errors": False, + }, + { + "debug": False, + "running_tests": True, + "show_callback_changed": True, + "urls_installed": True, + "errors": False, + }, + { + "debug": False, + "running_tests": True, + "show_callback_changed": True, + "urls_installed": False, + "errors": True, + }, + ] + for config in params: + with self.subTest(**config): + config_setting = { + "RENDER_PANELS": False, + "IS_RUNNING_TESTS": config["running_tests"], + "SHOW_TOOLBAR_CALLBACK": ( + (lambda *args: True) + if config["show_callback_changed"] + else "debug_toolbar.middleware.show_toolbar" + ), + } + if config["urls_installed"]: + reverse.side_effect = lambda *args: None + else: + reverse.side_effect = NoReverseMatch() - dt_settings.get_config()["IS_RUNNING_TESTS"] = True - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual( - errors, - [ - Error( - "The Django Debug Toolbar can't be used with tests", - hint="Django changes the DEBUG setting to False when running " - "tests. By default the Django Debug Toolbar is installed because " - "DEBUG is set to True. For most cases, you need to avoid installing " - "the toolbar when running tests. If you feel this check is in error, " - "you can set `DEBUG_TOOLBAR_CONFIG['IS_RUNNING_TESTS'] = False` to " - "bypass this check.", - id="debug_toolbar.E001", - ) - ], - ) + with self.settings( + DEBUG=config["debug"], DEBUG_TOOLBAR_CONFIG=config_setting + ): + errors = debug_toolbar_installed_when_running_tests_check(None) + if config["errors"]: + self.assertEqual(len(errors), 1) + self.assertEqual(errors[0].id, "debug_toolbar.E001") + else: + self.assertEqual(len(errors), 0) @override_settings( DEBUG_TOOLBAR_CONFIG={ From 325ea1940dc178ac05718504be01e07250298de3 Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Wed, 3 Jul 2024 11:14:13 -0500 Subject: [PATCH 09/12] Introduce debug_toolbar_urls to simplify installation (#1926) While this isn't a huge improvement, it provides a hook for us to more easily introduce library level controls for when urls are installed. It also eliminates the user from having to write an if statement for the most basic of installations. --- debug_toolbar/toolbar.py | 28 +++++++++++++++++++++++++++- docs/changes.rst | 2 ++ docs/installation.rst | 16 +++++++++------- example/urls.py | 11 +++-------- tests/test_toolbar.py | 17 +++++++++++++++++ 5 files changed, 58 insertions(+), 16 deletions(-) create mode 100644 tests/test_toolbar.py diff --git a/debug_toolbar/toolbar.py b/debug_toolbar/toolbar.py index fc07543b5..04502ab09 100644 --- a/debug_toolbar/toolbar.py +++ b/debug_toolbar/toolbar.py @@ -2,16 +2,18 @@ The main DebugToolbar class that loads and renders the Toolbar. """ +import re import uuid from collections import OrderedDict from functools import lru_cache from django.apps import apps +from django.conf import settings from django.core.exceptions import ImproperlyConfigured from django.dispatch import Signal from django.template import TemplateSyntaxError from django.template.loader import render_to_string -from django.urls import 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 @@ -186,3 +188,27 @@ def observe_request(request): Determine whether to update the toolbar from a client side request. """ return True + + +def debug_toolbar_urls(prefix="__debug__"): + """ + Return a URL pattern for serving toolbar in debug mode. + + from django.conf import settings + from debug_toolbar.toolbar import debug_toolbar_urls + + urlpatterns = [ + # ... the rest of your URLconf goes here ... + ] + debug_toolbar_urls() + """ + if not prefix: + raise ImproperlyConfigured("Empty urls prefix not permitted") + elif not settings.DEBUG: + # No-op if not in debug mode. + return [] + return [ + re_path( + r"^%s/" % re.escape(prefix.lstrip("/")), + include("debug_toolbar.urls"), + ), + ] diff --git a/docs/changes.rst b/docs/changes.rst index 6dbfe829a..10883b1e2 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -9,6 +9,8 @@ Pending * Limit ``E001`` check to likely error cases when the ``SHOW_TOOLBAR_CALLBACK`` has changed, but the toolbar's URL paths aren't installed. +* Introduce helper function ``debug_toolbar_urls`` to + simplify installation. 4.4.2 (2024-05-27) ------------------ diff --git a/docs/installation.rst b/docs/installation.rst index 657450fac..9200504b7 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -95,14 +95,14 @@ Add django-debug-toolbar's URLs to your project's URLconf: .. code-block:: python from django.urls import include, path + from debug_toolbar.toolbar import debug_toolbar_urls urlpatterns = [ - # ... - path("__debug__/", include("debug_toolbar.urls")), - ] + # ... the rest of your URLconf goes here ... + ] + debug_toolbar_urls() -This example uses the ``__debug__`` prefix, but you can use any prefix that -doesn't clash with your application's URLs. +By default this uses the ``__debug__`` prefix for the paths, but you can +use any prefix that doesn't clash with your application's URLs. 5. Add the Middleware @@ -180,11 +180,13 @@ You should also modify your URLconf file: .. code-block:: python + from django.conf import settings + from debug_toolbar.toolbar import debug_toolbar_urls + if not settings.TESTING: urlpatterns = [ *urlpatterns, - path("__debug__/", include("debug_toolbar.urls")), - ] + ] + debug_toolbar_urls() Alternatively, you can check out the :ref:`IS_RUNNING_TESTS ` option. diff --git a/example/urls.py b/example/urls.py index 7569a57f9..b64aa1c37 100644 --- a/example/urls.py +++ b/example/urls.py @@ -1,8 +1,8 @@ -from django.conf import settings from django.contrib import admin -from django.urls import include, path +from django.urls import path from django.views.generic import TemplateView +from debug_toolbar.toolbar import debug_toolbar_urls from example.views import increment urlpatterns = [ @@ -34,9 +34,4 @@ ), path("admin/", admin.site.urls), path("ajax/increment", increment, name="ajax_increment"), -] - -if settings.ENABLE_DEBUG_TOOLBAR: - urlpatterns += [ - path("__debug__/", include("debug_toolbar.urls")), - ] +] + debug_toolbar_urls() diff --git a/tests/test_toolbar.py b/tests/test_toolbar.py new file mode 100644 index 000000000..7155d3fcb --- /dev/null +++ b/tests/test_toolbar.py @@ -0,0 +1,17 @@ +from django.core.exceptions import ImproperlyConfigured + +from debug_toolbar.toolbar import debug_toolbar_urls +from tests.base import BaseTestCase + + +class DebugToolbarUrlsTestCase(BaseTestCase): + def test_empty_prefix_errors(self): + with self.assertRaises(ImproperlyConfigured): + debug_toolbar_urls(prefix="") + + def test_empty_when_debug_is_false(self): + self.assertEqual(debug_toolbar_urls(), []) + + def test_has_path(self): + with self.settings(DEBUG=True): + self.assertEqual(len(debug_toolbar_urls()), 1) From d0f09b3a6bd86414d94bb397bf2e5dfe0f4db376 Mon Sep 17 00:00:00 2001 From: "Bart K. de Koning" <118313986+bkdekoning@users.noreply.github.com> Date: Thu, 4 Jul 2024 12:18:41 -0400 Subject: [PATCH 10/12] Fixed #1682 -- alert user when using file field without proper encoding (#1933) * Fixed issue #1682 -- alert user when using file field without proper encoding * Changed issues to alerts, added docstring to check_invalid_... function, changed call in nav_subtitle to get_stats * Update AlertsPanel documentation to list pre-defined alert cases * added check for file inputs that directly reference a form, including tests; also added tests for setting encoding in submit input type * Update the alert messages to be on the panel as a map. This also explicitly mentions what attribute the form needs in the message. * Expose a page in the example app that triggers the alerts panel. --------- Co-authored-by: Tim Schilling --- debug_toolbar/panels/alerts.py | 148 ++++++++++++++++++ debug_toolbar/settings.py | 1 + .../debug_toolbar/panels/alerts.html | 12 ++ docs/changes.rst | 2 + docs/configuration.rst | 1 + docs/panels.rst | 11 ++ example/templates/bad_form.html | 14 ++ example/templates/index.html | 1 + example/urls.py | 5 + tests/panels/test_alerts.py | 101 ++++++++++++ tests/panels/test_history.py | 1 + 11 files changed, 297 insertions(+) create mode 100644 debug_toolbar/panels/alerts.py create mode 100644 debug_toolbar/templates/debug_toolbar/panels/alerts.html create mode 100644 example/templates/bad_form.html create mode 100644 tests/panels/test_alerts.py diff --git a/debug_toolbar/panels/alerts.py b/debug_toolbar/panels/alerts.py new file mode 100644 index 000000000..27a7119ee --- /dev/null +++ b/debug_toolbar/panels/alerts.py @@ -0,0 +1,148 @@ +from html.parser import HTMLParser + +from django.utils.translation import gettext_lazy as _ + +from debug_toolbar.panels import Panel + + +class FormParser(HTMLParser): + """ + HTML form parser, used to check for invalid configurations of forms that + take file inputs. + """ + + def __init__(self): + super().__init__() + self.in_form = False + self.current_form = {} + self.forms = [] + self.form_ids = [] + self.referenced_file_inputs = [] + + def handle_starttag(self, tag, attrs): + attrs = dict(attrs) + if tag == "form": + self.in_form = True + form_id = attrs.get("id") + if form_id: + self.form_ids.append(form_id) + self.current_form = { + "file_form": False, + "form_attrs": attrs, + "submit_element_attrs": [], + } + elif ( + self.in_form + and tag == "input" + and attrs.get("type") == "file" + and (not attrs.get("form") or attrs.get("form") == "") + ): + self.current_form["file_form"] = True + elif ( + self.in_form + and ( + (tag == "input" and attrs.get("type") in {"submit", "image"}) + or tag == "button" + ) + and (not attrs.get("form") or attrs.get("form") == "") + ): + self.current_form["submit_element_attrs"].append(attrs) + elif tag == "input" and attrs.get("form"): + self.referenced_file_inputs.append(attrs) + + def handle_endtag(self, tag): + if tag == "form" and self.in_form: + self.forms.append(self.current_form) + self.in_form = False + + +class AlertsPanel(Panel): + """ + A panel to alert users to issues. + """ + + messages = { + "form_id_missing_enctype": _( + 'Form with id "{form_id}" contains file input, but does not have the attribute enctype="multipart/form-data".' + ), + "form_missing_enctype": _( + 'Form contains file input, but does not have the attribute enctype="multipart/form-data".' + ), + "input_refs_form_missing_enctype": _( + 'Input element references form with id "{form_id}", but the form does not have the attribute enctype="multipart/form-data".' + ), + } + + title = _("Alerts") + + template = "debug_toolbar/panels/alerts.html" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.alerts = [] + + @property + def nav_subtitle(self): + alerts = self.get_stats()["alerts"] + if alerts: + alert_text = "alert" if len(alerts) == 1 else "alerts" + return f"{len(alerts)} {alert_text}" + else: + return "" + + def add_alert(self, alert): + self.alerts.append(alert) + + def check_invalid_file_form_configuration(self, html_content): + """ + Inspects HTML content for a form that includes a file input but does + not have the encoding type set to multipart/form-data, and warns the + user if so. + """ + parser = FormParser() + parser.feed(html_content) + + # Check for file inputs directly inside a form that do not reference + # any form through the form attribute + for form in parser.forms: + if ( + form["file_form"] + and form["form_attrs"].get("enctype") != "multipart/form-data" + and not any( + elem.get("formenctype") == "multipart/form-data" + for elem in form["submit_element_attrs"] + ) + ): + if form_id := form["form_attrs"].get("id"): + alert = self.messages["form_id_missing_enctype"].format( + form_id=form_id + ) + else: + alert = self.messages["form_missing_enctype"] + self.add_alert({"alert": alert}) + + # Check for file inputs that reference a form + form_attrs_by_id = { + form["form_attrs"].get("id"): form["form_attrs"] for form in parser.forms + } + + for attrs in parser.referenced_file_inputs: + form_id = attrs.get("form") + if form_id and attrs.get("type") == "file": + form_attrs = form_attrs_by_id.get(form_id) + if form_attrs and form_attrs.get("enctype") != "multipart/form-data": + alert = self.messages["input_refs_form_missing_enctype"].format( + form_id=form_id + ) + self.add_alert({"alert": alert}) + + return self.alerts + + def generate_stats(self, request, response): + html_content = response.content.decode(response.charset) + self.check_invalid_file_form_configuration(html_content) + + # Further alert checks can go here + + # Write all alerts to record_stats + self.record_stats({"alerts": self.alerts}) diff --git a/debug_toolbar/settings.py b/debug_toolbar/settings.py index ca7036c34..48483cf40 100644 --- a/debug_toolbar/settings.py +++ b/debug_toolbar/settings.py @@ -67,6 +67,7 @@ def get_config(): "debug_toolbar.panels.sql.SQLPanel", "debug_toolbar.panels.staticfiles.StaticFilesPanel", "debug_toolbar.panels.templates.TemplatesPanel", + "debug_toolbar.panels.alerts.AlertsPanel", "debug_toolbar.panels.cache.CachePanel", "debug_toolbar.panels.signals.SignalsPanel", "debug_toolbar.panels.redirects.RedirectsPanel", diff --git a/debug_toolbar/templates/debug_toolbar/panels/alerts.html b/debug_toolbar/templates/debug_toolbar/panels/alerts.html new file mode 100644 index 000000000..df208836d --- /dev/null +++ b/debug_toolbar/templates/debug_toolbar/panels/alerts.html @@ -0,0 +1,12 @@ +{% load i18n %} + +{% if alerts %} +

{% trans "Alerts found" %}

+ {% for alert in alerts %} +
    +
  • {{ alert.alert }}
    • +
+ {% endfor %} +{% else %} +

{% trans "No alerts found" %}

+{% endif %} diff --git a/docs/changes.rst b/docs/changes.rst index 10883b1e2..fd642e980 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,6 +4,8 @@ Change log Pending ------- +* Added alert panel with warning when form is using file fields + without proper encoding type. * Fixed overriding font-family for both light and dark themes. * Restored compatibility with ``iptools.IpRangeList``. * Limit ``E001`` check to likely error cases when the diff --git a/docs/configuration.rst b/docs/configuration.rst index 80e44984f..e4ccc1dae 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -29,6 +29,7 @@ default value is:: 'debug_toolbar.panels.sql.SQLPanel', 'debug_toolbar.panels.staticfiles.StaticFilesPanel', 'debug_toolbar.panels.templates.TemplatesPanel', + 'debug_toolbar.panels.alerts.AlertsPanel', 'debug_toolbar.panels.cache.CachePanel', 'debug_toolbar.panels.signals.SignalsPanel', 'debug_toolbar.panels.redirects.RedirectsPanel', diff --git a/docs/panels.rst b/docs/panels.rst index 33359ea46..c9ea6bbf0 100644 --- a/docs/panels.rst +++ b/docs/panels.rst @@ -9,6 +9,17 @@ Default built-in panels The following panels are enabled by default. +Alerts +~~~~~~~ + +.. class:: debug_toolbar.panels.alerts.AlertsPanel + +This panel shows alerts for a set of pre-defined cases: + +- Alerts when the response has a form without the + ``enctype="multipart/form-data"`` attribute and the form contains + a file input. + History ~~~~~~~ diff --git a/example/templates/bad_form.html b/example/templates/bad_form.html new file mode 100644 index 000000000..f50662c6e --- /dev/null +++ b/example/templates/bad_form.html @@ -0,0 +1,14 @@ +{% load cache %} + + + + + Bad form + + +

Bad form test

+
+ +
+ + diff --git a/example/templates/index.html b/example/templates/index.html index ee00d5f05..527f5d2a3 100644 --- a/example/templates/index.html +++ b/example/templates/index.html @@ -14,6 +14,7 @@

Index of Tests

  • Prototype 1.7.3.0
  • Hotwire Turbo
  • htmx
    • +
  • Bad form

    • Django Admin

    {% endcache %} diff --git a/example/urls.py b/example/urls.py index b64aa1c37..6dded2da7 100644 --- a/example/urls.py +++ b/example/urls.py @@ -7,6 +7,11 @@ urlpatterns = [ path("", TemplateView.as_view(template_name="index.html"), name="home"), + path( + "bad-form/", + TemplateView.as_view(template_name="bad_form.html"), + name="bad_form", + ), path("jquery/", TemplateView.as_view(template_name="jquery/index.html")), path("mootools/", TemplateView.as_view(template_name="mootools/index.html")), path("prototype/", TemplateView.as_view(template_name="prototype/index.html")), diff --git a/tests/panels/test_alerts.py b/tests/panels/test_alerts.py new file mode 100644 index 000000000..e61c8da12 --- /dev/null +++ b/tests/panels/test_alerts.py @@ -0,0 +1,101 @@ +from django.http import HttpResponse +from django.template import Context, Template + +from ..base import BaseTestCase + + +class AlertsPanelTestCase(BaseTestCase): + panel_id = "AlertsPanel" + + def test_alert_warning_display(self): + """ + Test that the panel (does not) display[s] an alert when there are + (no) problems. + """ + self.panel.record_stats({"alerts": []}) + self.assertNotIn("alerts", self.panel.nav_subtitle) + + self.panel.record_stats({"alerts": ["Alert 1", "Alert 2"]}) + self.assertIn("2 alerts", self.panel.nav_subtitle) + + def test_file_form_without_enctype_multipart_form_data(self): + """ + Test that the panel displays a form invalid message when there is + a file input but encoding not set to multipart/form-data. + """ + test_form = '
    ' + result = self.panel.check_invalid_file_form_configuration(test_form) + expected_error = ( + 'Form with id "test-form" contains file input, ' + 'but does not have the attribute enctype="multipart/form-data".' + ) + self.assertEqual(result[0]["alert"], expected_error) + self.assertEqual(len(result), 1) + + def test_file_form_no_id_without_enctype_multipart_form_data(self): + """ + Test that the panel displays a form invalid message when there is + a file input but encoding not set to multipart/form-data. + + This should use the message when the form has no id. + """ + test_form = '
    ' + result = self.panel.check_invalid_file_form_configuration(test_form) + expected_error = ( + "Form contains file input, but does not have " + 'the attribute enctype="multipart/form-data".' + ) + self.assertEqual(result[0]["alert"], expected_error) + self.assertEqual(len(result), 1) + + def test_file_form_with_enctype_multipart_form_data(self): + test_form = """
    + +
    """ + result = self.panel.check_invalid_file_form_configuration(test_form) + + self.assertEqual(len(result), 0) + + def test_file_form_with_enctype_multipart_form_data_in_button(self): + test_form = """
    + + +
    """ + result = self.panel.check_invalid_file_form_configuration(test_form) + + self.assertEqual(len(result), 0) + + def test_referenced_file_input_without_enctype_multipart_form_data(self): + test_file_input = """
    + """ + result = self.panel.check_invalid_file_form_configuration(test_file_input) + + expected_error = ( + 'Input element references form with id "test-form", ' + 'but the form does not have the attribute enctype="multipart/form-data".' + ) + self.assertEqual(result[0]["alert"], expected_error) + self.assertEqual(len(result), 1) + + def test_referenced_file_input_with_enctype_multipart_form_data(self): + test_file_input = """
    +
    + """ + result = self.panel.check_invalid_file_form_configuration(test_file_input) + + self.assertEqual(len(result), 0) + + def test_integration_file_form_without_enctype_multipart_form_data(self): + t = Template('
    ') + c = Context({}) + rendered_template = t.render(c) + response = HttpResponse(content=rendered_template) + + self.panel.generate_stats(self.request, response) + + self.assertIn("1 alert", self.panel.nav_subtitle) + self.assertIn( + "Form with id "test-form" contains file input, " + "but does not have the attribute enctype="multipart/form-data".", + self.panel.content, + ) diff --git a/tests/panels/test_history.py b/tests/panels/test_history.py index 2e0aa2179..4c5244934 100644 --- a/tests/panels/test_history.py +++ b/tests/panels/test_history.py @@ -75,6 +75,7 @@ class HistoryViewsTestCase(IntegrationTestCase): "SQLPanel", "StaticFilesPanel", "TemplatesPanel", + "AlertsPanel", "CachePanel", "SignalsPanel", "ProfilingPanel", From eff9128f2233f8c6341e1fba8a44d5db54cd9ae2 Mon Sep 17 00:00:00 2001 From: "Michael J. Nicholson" Date: Thu, 4 Jul 2024 18:29:51 +0200 Subject: [PATCH 11/12] Remove rem units from svg (#1942) * Use css properties for height and width --- .gitignore | 2 ++ debug_toolbar/static/debug_toolbar/css/toolbar.css | 2 ++ .../templates/debug_toolbar/includes/theme_selector.html | 6 ------ docs/changes.rst | 1 + 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index ee3559cc4..988922d50 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,5 @@ htmlcov .tox geckodriver.log coverage.xml +.direnv/ +.envrc diff --git a/debug_toolbar/static/debug_toolbar/css/toolbar.css b/debug_toolbar/static/debug_toolbar/css/toolbar.css index c7e201d07..e495eeb0c 100644 --- a/debug_toolbar/static/debug_toolbar/css/toolbar.css +++ b/debug_toolbar/static/debug_toolbar/css/toolbar.css @@ -734,4 +734,6 @@ #djDebug[data-theme="dark"] #djToggleThemeButton svg.theme-dark, #djDebug[data-theme="auto"] #djToggleThemeButton svg.theme-auto { display: block; + height: 1rem; + width: 1rem; } diff --git a/debug_toolbar/templates/debug_toolbar/includes/theme_selector.html b/debug_toolbar/templates/debug_toolbar/includes/theme_selector.html index 372727900..926ff250b 100644 --- a/debug_toolbar/templates/debug_toolbar/includes/theme_selector.html +++ b/debug_toolbar/templates/debug_toolbar/includes/theme_selector.html @@ -2,8 +2,6 @@ aria-hidden="true" class="djdt-hidden theme-auto" fill="currentColor" - width="1rem" - height="1rem" viewBox="0 0 32 32" version="1.1" xmlns="http://www.w3.org/2000/svg" @@ -15,8 +13,6 @@