Make toolbar compatible with FORCE_SCRIPT_NAME

[dmartin]

Description

Previously, if a project used the FORCE_SCRIPT_NAME setting (such as when hosting a Django application under a subdirectory path via a reverse proxy), is_toolbar_request() would always return False.

This is because request.resolver_match is not yet available in the middleware context, so django.urls.resolve() is called instead. resolve() then raised Resolver404, because it does not take FORCE_SCRIPT_NAME into account.

This caused internal toolbar URLs to be inspected, leading to a request loop when refreshing the history panel.

This PR makes calls to django.urls.resolve() take request.path_info rather than request.path. This effectively strips the request's leading SCRIPT_NAME if one is present.

Fixes #1961

Checklist:

• I have added the relevant tests for this change.
• I have added an item to the Pending section of docs/changes.rst.

[matthiask]

Could you try using request.path_info instead of request.path? The former should do the right thing if Django isn't mounted at '/' without having to use the script prefix explicitly. (I haven't tested it myself.)

[dmartin]

Could you try using request.path_info instead of request.path? The former should do the right thing if Django isn't mounted at '/' without having to use the script prefix explicitly. (I haven't tested it myself.)

Oh, that does work and is a lot cleaner! Thanks for the suggestion. I can make that change, but what would you like to do with the existing tests that just override request.path? For example:

@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))

I can also add matching request.path_info overrides to each of these, or I suppose they could be outright replaced with request.path_info overrides for all tests except mine which is explicitly testing the behavior when they differ.

[tim-schilling]

@dmartin could we switch them with a RequestFactory().get('/__debug__/render_panel/') request?

[dmartin]

I made those changes, though I didn't rework every test to use a RequestFactory to keep the scope of the PR under control, only the tests that were manually setting request.path.

[tim-schilling]

I'm good with this change.

It definitely highlights a problem we have in the test suite where we manipulate a request after initializing the toolbar. It's possible we may run into a bug where the the toolbar does something slightly differently because of this. However, if that were the case, I would have expected it to crop up before now.

[matthiask]

Thank you!