Fixed #24720 -- Avoided resolving URLs that don't end in a slash twice in CommonMiddleware.

This speeds up affected requests by about 5%.

django/middleware/common.py

@@ -50,47 +50,68 @@ class CommonMiddleware(object):
                 if user_agent_regex.search(request.META['HTTP_USER_AGENT']):
                     raise PermissionDenied('Forbidden user agent')
 
-        # Check for a redirect based on settings.APPEND_SLASH
-        # and settings.PREPEND_WWW
+        # Check for a redirect based on settings.PREPEND_WWW
         host = request.get_host()
-        old_url = [host, request.get_full_path()]
-        new_url = old_url[:]
 
-        if (settings.PREPEND_WWW and old_url[0] and
-                not old_url[0].startswith('www.')):
-            new_url[0] = 'www.' + old_url[0]
+        if settings.PREPEND_WWW and host and not host.startswith('www.'):
+            host = 'www.' + host
 
-        # Append a slash if APPEND_SLASH is set and the URL doesn't have a
-        # trailing slash and there is no pattern for the current path
-        if settings.APPEND_SLASH and (not old_url[1].endswith('/')):
+            # Check if we also need to append a slash so we can do it all
+            # with a single redirect.
+            if self.should_redirect_with_slash(request):
+                path = self.get_full_path_with_slash(request)
+            else:
+                path = request.get_full_path()
+
+            return self.response_redirect_class('%s://%s%s' % (request.scheme, host, path))
+
+    def should_redirect_with_slash(self, request):
+        """
+        Return True if settings.APPEND_SLASH is True and appending a slash to
+        the request path turns an invalid path into a valid one.
+        """
+        if settings.APPEND_SLASH and not request.get_full_path().endswith('/'):
             urlconf = getattr(request, 'urlconf', None)
-            if (not urlresolvers.is_valid_path(request.path_info, urlconf) and
-                    urlresolvers.is_valid_path("%s/" % request.path_info, urlconf)):
-                new_url[1] = request.get_full_path(force_append_slash=True)
-                if settings.DEBUG and request.method in ('POST', 'PUT', 'PATCH'):
-                    raise RuntimeError((""
-                    "You called this URL via %(method)s, but the URL doesn't end "
-                    "in a slash and you have APPEND_SLASH set. Django can't "
-                    "redirect to the slash URL while maintaining %(method)s data. "
-                    "Change your form to point to %(url)s (note the trailing "
-                    "slash), or set APPEND_SLASH=False in your Django "
-                    "settings.") % {'method': request.method, 'url': ''.join(new_url)})
-
-        if new_url == old_url:
-            # No redirects required.
-            return
-        if new_url[0] != old_url[0]:
-            newurl = "%s://%s%s" % (
-                request.scheme,
-                new_url[0], new_url[1])
-        else:
-            newurl = new_url[1]
-        return self.response_redirect_class(newurl)
+            return (
+                not urlresolvers.is_valid_path(request.path_info, urlconf)
+                and urlresolvers.is_valid_path('%s/' % request.path_info, urlconf)
+            )
+        return False
+
+    def get_full_path_with_slash(self, request):
+        """
+        Return the full path of the request with a trailing slash appended.
+
+        Raise a RuntimeError if settings.DEBUG is True and request.method is
+        GET, PUT, or PATCH.
+        """
+        new_path = request.get_full_path(force_append_slash=True)
+        if settings.DEBUG and request.method in ('POST', 'PUT', 'PATCH'):
+            raise RuntimeError(
+                "You called this URL via %(method)s, but the URL doesn't end "
+                "in a slash and you have APPEND_SLASH set. Django can't "
+                "redirect to the slash URL while maintaining %(method)s data. "
+                "Change your form to point to %(url)s (note the trailing "
+                "slash), or set APPEND_SLASH=False in your Django settings." % {
+                    'method': request.method,
+                    'url': request.get_host() + new_path,
+                }
+            )
+        return new_path
 
     def process_response(self, request, response):
         """
         Calculate the ETag, if needed.
+
+        When the status code of the response is 404, it may redirect to a path
+        with an appended slash if should_redirect_with_slash() returns True.
         """
+        # If the given URL is "Not Found", then check if we should redirect to
+        # a path with a slash appended.
+        if response.status_code == 404:
+            if self.should_redirect_with_slash(request):
+                return self.response_redirect_class(self.get_full_path_with_slash(request))
+
         if settings.USE_ETAGS:
             if response.has_header('ETag'):
                 etag = response['ETag']

tests/middleware/tests.py

@@ -11,8 +11,8 @@ from django.conf import settings
 from django.core import mail
 from django.core.exceptions import PermissionDenied
 from django.http import (
-    FileResponse, HttpRequest, HttpResponse, HttpResponsePermanentRedirect,
-    HttpResponseRedirect, StreamingHttpResponse,
+    FileResponse, HttpRequest, HttpResponse, HttpResponseNotFound,
+    HttpResponsePermanentRedirect, HttpResponseRedirect, StreamingHttpResponse,
 )
 from django.middleware.clickjacking import XFrameOptionsMiddleware
 from django.middleware.common import (
@@ -39,6 +39,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         """
         request = self.rf.get('/slash/')
         self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponseNotFound()
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_slashless_resource(self):
@@ -47,6 +49,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         """
         request = self.rf.get('/noslash')
         self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponse("Here's the text of the Web page.")
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_slashless_unknown(self):
@@ -54,7 +58,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         APPEND_SLASH should not redirect to unknown resources.
         """
         request = self.rf.get('/unknown')
-        self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponseNotFound()
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_redirect(self):
@@ -62,7 +67,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         APPEND_SLASH should redirect slashless URLs to a valid pattern.
         """
         request = self.rf.get('/slash')
-        r = CommonMiddleware().process_request(request)
+        response = HttpResponseNotFound()
+        r = CommonMiddleware().process_response(request, response)
         self.assertEqual(r.status_code, 301)
         self.assertEqual(r.url, '/slash/')
 
@@ -72,7 +78,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         APPEND_SLASH should preserve querystrings when redirecting.
         """
         request = self.rf.get('/slash?test=1')
-        r = CommonMiddleware().process_request(request)
+        response = HttpResponseNotFound()
+        r = CommonMiddleware().process_response(request, response)
         self.assertEqual(r.url, '/slash/?test=1')
 
     @override_settings(APPEND_SLASH=True, DEBUG=True)
@@ -85,16 +92,17 @@ class CommonMiddlewareTest(SimpleTestCase):
         msg = "maintaining %s data. Change your form to point to testserver/slash/"
         request = self.rf.get('/slash')
         request.method = 'POST'
+        response = HttpResponseNotFound()
         with six.assertRaisesRegex(self, RuntimeError, msg % request.method):
-            CommonMiddleware().process_request(request)
+            CommonMiddleware().process_response(request, response)
         request = self.rf.get('/slash')
         request.method = 'PUT'
         with six.assertRaisesRegex(self, RuntimeError, msg % request.method):
-            CommonMiddleware().process_request(request)
+            CommonMiddleware().process_response(request, response)
         request = self.rf.get('/slash')
         request.method = 'PATCH'
         with six.assertRaisesRegex(self, RuntimeError, msg % request.method):
-            CommonMiddleware().process_request(request)
+            CommonMiddleware().process_response(request, response)
 
     @override_settings(APPEND_SLASH=False)
     def test_append_slash_disabled(self):
@@ -102,7 +110,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         Disabling append slash functionality should leave slashless URLs alone.
         """
         request = self.rf.get('/slash')
-        self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponseNotFound()
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_quoted(self):
@@ -110,7 +119,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         URLs which require quoting should be redirected to their slash version ok.
         """
         request = self.rf.get(quote('/needsquoting#'))
-        r = CommonMiddleware().process_request(request)
+        response = HttpResponseNotFound()
+        r = CommonMiddleware().process_response(request, response)
         self.assertEqual(r.status_code, 301)
         self.assertEqual(
             r.url,
@@ -152,6 +162,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         request = self.rf.get('/customurlconf/slash/')
         request.urlconf = 'middleware.extra_urls'
         self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponseNotFound()
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_slashless_resource_custom_urlconf(self):
@@ -161,6 +173,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         request = self.rf.get('/customurlconf/noslash')
         request.urlconf = 'middleware.extra_urls'
         self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponse("Here's the text of the Web page.")
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_slashless_unknown_custom_urlconf(self):
@@ -170,6 +184,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         request = self.rf.get('/customurlconf/unknown')
         request.urlconf = 'middleware.extra_urls'
         self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponseNotFound()
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_redirect_custom_urlconf(self):
@@ -178,7 +194,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         """
         request = self.rf.get('/customurlconf/slash')
         request.urlconf = 'middleware.extra_urls'
-        r = CommonMiddleware().process_request(request)
+        response = HttpResponseNotFound()
+        r = CommonMiddleware().process_response(request, response)
         self.assertIsNotNone(r,
             "CommonMiddlware failed to return APPEND_SLASH redirect using request.urlconf")
         self.assertEqual(r.status_code, 301)
@@ -194,8 +211,9 @@ class CommonMiddlewareTest(SimpleTestCase):
         request = self.rf.get('/customurlconf/slash')
         request.urlconf = 'middleware.extra_urls'
         request.method = 'POST'
+        response = HttpResponseNotFound()
         with six.assertRaisesRegex(self, RuntimeError, 'end in a slash'):
-            CommonMiddleware().process_request(request)
+            CommonMiddleware().process_response(request, response)
 
     @override_settings(APPEND_SLASH=False)
     def test_append_slash_disabled_custom_urlconf(self):
@@ -205,6 +223,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         request = self.rf.get('/customurlconf/slash')
         request.urlconf = 'middleware.extra_urls'
         self.assertEqual(CommonMiddleware().process_request(request), None)
+        response = HttpResponseNotFound()
+        self.assertEqual(CommonMiddleware().process_response(request, response), response)
 
     @override_settings(APPEND_SLASH=True)
     def test_append_slash_quoted_custom_urlconf(self):
@@ -213,7 +233,8 @@ class CommonMiddlewareTest(SimpleTestCase):
         """
         request = self.rf.get(quote('/customurlconf/needsquoting#'))
         request.urlconf = 'middleware.extra_urls'
-        r = CommonMiddleware().process_request(request)
+        response = HttpResponseNotFound()
+        r = CommonMiddleware().process_response(request, response)
         self.assertIsNotNone(r,
             "CommonMiddlware failed to return APPEND_SLASH redirect using request.urlconf")
         self.assertEqual(r.status_code, 301)
@@ -262,12 +283,16 @@ class CommonMiddlewareTest(SimpleTestCase):
         """Regression test for #15152"""
         request = self.rf.get('/slash')
         request.META['QUERY_STRING'] = force_str('drink=café')
-        response = CommonMiddleware().process_request(request)
-        self.assertEqual(response.status_code, 301)
+        r = CommonMiddleware().process_request(request)
+        self.assertIsNone(r)
+        response = HttpResponseNotFound()
+        r = CommonMiddleware().process_response(request, response)
+        self.assertEqual(r.status_code, 301)
 
     def test_response_redirect_class(self):
         request = self.rf.get('/slash')
-        r = CommonMiddleware().process_request(request)
+        response = HttpResponseNotFound()
+        r = CommonMiddleware().process_response(request, response)
         self.assertEqual(r.status_code, 301)
         self.assertEqual(r.url, '/slash/')
         self.assertIsInstance(r, HttpResponsePermanentRedirect)
@@ -277,7 +302,8 @@ class CommonMiddlewareTest(SimpleTestCase):
             response_redirect_class = HttpResponseRedirect
 
         request = self.rf.get('/slash')
-        r = MyCommonMiddleware().process_request(request)
+        response = HttpResponseNotFound()
+        r = MyCommonMiddleware().process_response(request, response)
         self.assertEqual(r.status_code, 302)
         self.assertEqual(r.url, '/slash/')
         self.assertIsInstance(r, HttpResponseRedirect)

tests/test_client_regress/views.py

@@ -66,7 +66,7 @@ def nested_view(request):
     """
     setup_test_environment()
     c = Client()
-    c.get("/no_template_view")
+    c.get("/no_template_view/")
     return render_to_response('base.html', {'nested': 'yes'})