From fbac2a4dd846b52c4f379eacb5bab654fe9540cc Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 2 Jun 2022 12:34:41 +0200 Subject: [PATCH] Fixed #33700 -- Skipped extra resolution for successful requests not ending with /. By moving a should_redirect_with_slash call out of an if block, commit 9390da7fb6e251eaa9a785692f987296cb14523f negated the performance fix of commit 434d309ef6dbecbfd2b322d3a1da78aa5cb05fa8 (#24720). Meanwhile, the logging issue #26293 that it targeted was subsequently fixed more fully by commit 40b69607c751c4afa453edfd41d2ed155e58187e (#26504), so it is no longer needed. This effectively reverts it. This speeds up successful requests not ending with / when APPEND_SLASH is enabled (the default, and still useful in projects with a mix of URLs with and without trailing /). The amount of speedup varies from about 5% in a typical project to nearly 50% on a benchmark with many routes. Signed-off-by: Anders Kaseorg --- django/middleware/common.py | 21 ++++++++++----------- tests/middleware/tests.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/django/middleware/common.py b/django/middleware/common.py index c652374aec..12d006127b 100644 --- a/django/middleware/common.py +++ b/django/middleware/common.py @@ -46,19 +46,18 @@ class CommonMiddleware(MiddlewareMixin): # Check for a redirect based on settings.PREPEND_WWW host = request.get_host() - must_prepend = settings.PREPEND_WWW and host and not host.startswith("www.") - redirect_url = ("%s://www.%s" % (request.scheme, host)) if must_prepend else "" - # Check if a slash should be appended - if self.should_redirect_with_slash(request): - path = self.get_full_path_with_slash(request) - else: - path = request.get_full_path() + if settings.PREPEND_WWW and host and not host.startswith("www."): + # Check if we also need to append a slash so we can do it all + # with a single redirect. (This check may be somewhat expensive, + # so we only do it if we already know we're sending a redirect, + # or in process_response if we get a 404.) + if self.should_redirect_with_slash(request): + path = self.get_full_path_with_slash(request) + else: + path = request.get_full_path() - # Return a redirect if necessary - if redirect_url or path != request.get_full_path(): - redirect_url += path - return self.response_redirect_class(redirect_url) + return self.response_redirect_class(f"{request.scheme}://www.{host}{path}") def should_redirect_with_slash(self, request): """ diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py index cb40b36321..63eaa5d28f 100644 --- a/tests/middleware/tests.py +++ b/tests/middleware/tests.py @@ -80,7 +80,11 @@ class CommonMiddlewareTest(SimpleTestCase): """ request = self.rf.get("/slash") r = CommonMiddleware(get_response_empty).process_request(request) + self.assertIsNone(r) + response = HttpResponseNotFound() + r = CommonMiddleware(get_response_empty).process_response(request, response) self.assertEqual(r.status_code, 301) + self.assertEqual(r.url, "/slash/") @override_settings(APPEND_SLASH=True) def test_append_slash_redirect_querystring(self): @@ -164,6 +168,9 @@ class CommonMiddlewareTest(SimpleTestCase): # Use 4 slashes because of RequestFactory behavior. request = self.rf.get("////evil.com/security") r = CommonMiddleware(get_response_404).process_request(request) + self.assertIsNone(r) + response = HttpResponseNotFound() + r = CommonMiddleware(get_response_404).process_response(request, response) self.assertEqual(r.status_code, 301) self.assertEqual(r.url, "/%2Fevil.com/security/") r = CommonMiddleware(get_response_404)(request) @@ -354,6 +361,9 @@ class CommonMiddlewareTest(SimpleTestCase): request = self.rf.get("/slash") request.META["QUERY_STRING"] = "drink=café" r = CommonMiddleware(get_response_empty).process_request(request) + self.assertIsNone(r) + response = HttpResponseNotFound() + r = CommonMiddleware(get_response_empty).process_response(request, response) self.assertEqual(r.status_code, 301) def test_response_redirect_class(self):