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
9390da7fb6 negated the performance fix
of commit 434d309ef6 (#24720).
Meanwhile, the logging issue #26293 that it targeted was subsequently
fixed more fully by commit 40b69607c7
(#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 <andersk@mit.edu>
This commit is contained in:
Anders Kaseorg 2022-06-02 12:34:41 +02:00 committed by Carlton Gibson
parent 798b6c23ee
commit fbac2a4dd8
2 changed files with 20 additions and 11 deletions

View File

@ -46,19 +46,18 @@ class CommonMiddleware(MiddlewareMixin):
# Check for a redirect based on settings.PREPEND_WWW # Check for a redirect based on settings.PREPEND_WWW
host = request.get_host() 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 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): if self.should_redirect_with_slash(request):
path = self.get_full_path_with_slash(request) path = self.get_full_path_with_slash(request)
else: else:
path = request.get_full_path() path = request.get_full_path()
# Return a redirect if necessary return self.response_redirect_class(f"{request.scheme}://www.{host}{path}")
if redirect_url or path != request.get_full_path():
redirect_url += path
return self.response_redirect_class(redirect_url)
def should_redirect_with_slash(self, request): def should_redirect_with_slash(self, request):
""" """

View File

@ -80,7 +80,11 @@ class CommonMiddlewareTest(SimpleTestCase):
""" """
request = self.rf.get("/slash") request = self.rf.get("/slash")
r = CommonMiddleware(get_response_empty).process_request(request) 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.status_code, 301)
self.assertEqual(r.url, "/slash/")
@override_settings(APPEND_SLASH=True) @override_settings(APPEND_SLASH=True)
def test_append_slash_redirect_querystring(self): def test_append_slash_redirect_querystring(self):
@ -164,6 +168,9 @@ class CommonMiddlewareTest(SimpleTestCase):
# Use 4 slashes because of RequestFactory behavior. # Use 4 slashes because of RequestFactory behavior.
request = self.rf.get("////evil.com/security") request = self.rf.get("////evil.com/security")
r = CommonMiddleware(get_response_404).process_request(request) 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.status_code, 301)
self.assertEqual(r.url, "/%2Fevil.com/security/") self.assertEqual(r.url, "/%2Fevil.com/security/")
r = CommonMiddleware(get_response_404)(request) r = CommonMiddleware(get_response_404)(request)
@ -354,6 +361,9 @@ class CommonMiddlewareTest(SimpleTestCase):
request = self.rf.get("/slash") request = self.rf.get("/slash")
request.META["QUERY_STRING"] = "drink=café" request.META["QUERY_STRING"] = "drink=café"
r = CommonMiddleware(get_response_empty).process_request(request) 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.status_code, 301)
def test_response_redirect_class(self): def test_response_redirect_class(self):