Inconsistent behavior of DEBUG, debug_toolbar_urls()

[candletrick]

I just spent an hour debugging why djdt was breaking a portion of our site after a deploy (notably the django admin). The 'djdt' namespace went missing.

It turns out it's because of inconsistent behavior in some functions around DEBUG.

Most of the functions ( notably in this case, debug_toolbar_urls() ), explicitly return nothing when DEBUG=False.

However, SHOW_TOOLBAR_CALLBACK allows the DJDT to appear in other contexts, notably in middleware.py, and then they mutually break each other.

One suggestion is shown below to make behavior consistent:

diff --git a/debug_toolbar/middleware.py b/debug_toolbar/middleware.py
index aae69975..6f06e939 100644
--- a/debug_toolbar/middleware.py
+++ b/debug_toolbar/middleware.py
@@ -79,6 +79,9 @@ def show_toolbar_func_or_path() -> Callable:

     Cached to avoid importing multiple times.
     """
+    if not settings.DEBUG:
+        return False
+
     # If SHOW_TOOLBAR_CALLBACK is a string, which is the recommended
     # setup, resolve it to the corresponding callable.
     func_or_path = dt_settings.get_config()["SHOW_TOOLBAR_CALLBACK"]

Recommend changing this, or debug_toolbar_urls ... or the documentation ... This was confusing and tricky to track down.

[tim-schilling]

Thanks for the report. Are you overriding SHOW_TOOLBAR_CALLBACK in your project? Can you share your custom configuration please?

[tim-schilling]

I wonder if we should implement wrappers around reverse() and {% url %} so that we can capture these errors, then inspect the configuration and provide a more helpful error or log to identify the problem quicker.

[candletrick]

@tim-schilling Hi, thanks for your response. Yes we are overriding SHOW_TOOLBAR_CALLBACK. Let's just say for sake of argument that our config makes this function return True. The confusion is then that djdt initializes, however, the debug_toolbar_urls() is coded to return a hard stop empty array if DEBUG is False in toolbars.py, so it breaks overall:

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"^{}/".format(re.escape(prefix.lstrip("/"))),
            include("debug_toolbar.urls"),
        ),
    ]

A variety of possible solutions come to mind...

• You could wrap reverse as you suggested. But if there are multiple places where this occurs that might be more work;
• You could supply a warning message that debug_toolbar_urls() is inactive because DEBUG=False,
• You could change debug_toolbar_urls() with an override param, etc.
  ( To be fair, it appears before that helper function was added, that this problem would not occur if people were adding the re_path() calls directly )
• You could make SHOW_TOOLBAR_CALLBACK not apply if DEBUG == False, as I suggested above, because it seems unclear the overall intention. It seems that in other parts of the code it is intended to truly only work if DEBUG == True.
• You could update the documentation around how to add the url paths depending on if DEBUG is being used.

Thanks for your consideration!

[matthiask]

I personally think the toolbar should only ever be used with settings.DEBUG being True but people seem to have use cases which require running the toolbar in production even though this is discouraged.

I wonder if we shouldn't be consistent though and change debug_toolbar_urls and friends to not evaluate settings.DEBUG.

I'm sure this will break other people's setup. So, I'm not sure what the best way is.

(My first large decision was to remove the automatic configuration of the toolbar. I think breaking unreasonable setups isn't necessarily terrible.)

[candletrick]

Hi @matthiask ... thanks for your comment and wrapping my head around this again.

Perhaps the simplest and most consistent behavior would be if debug_toolbar_urls used the output of SHOW_TOOLBAR_CALLBACK (with a fallback to DEBUG), rather than the hardcoded use of DEBUG only.

Otherwise the callback is somewhat meaningless / broken.

As to whether or not it should be used in production environments--yes, you're probably right. Regardless, if this function is to be used at all it seems that debug_toolbar_urls should follow it.

Those are my thoughts for the moment.

[tim-schilling]

I personally think the toolbar should only ever be used with settings.DEBUG being True but people seem to have use cases which require running the toolbar in production even though this is discouraged.

I wonder if we shouldn't be consistent though and change debug_toolbar_urls and friends to not evaluate settings.DEBUG.

I'd like to see the project get to the state where that's not our recommendation.

[matthiask]

Perhaps the simplest and most consistent behavior would be if debug_toolbar_urls used the output of SHOW_TOOLBAR_CALLBACK (with a fallback to DEBUG), rather than the hardcoded use of DEBUG only.

I think this may be the way to go here.

I'd like to see the project get to the state where that's not our recommendation.

I think we have been moving in this direction for some time and we're not far away. The issue I see is that we do not just have to care about exploits, SQL injection or whatever, but also about DoS and memory exhaustion etc. and I don't know if we can own the stack (our libraries and dependencies) enough and if putting time into this goal specifically is the best way to advance the capabilities of the toolbar. That's worth discussing certainly (either at #2383 or generally in the discussions tab of this repo)