Fixed #19772 -- Handled APPEND_SLASH correctly in the redirects app.

django/contrib/redirects/middleware.py

@@ -1,3 +1,5 @@
+from __future__ import unicode_literals
+
 from django.contrib.redirects.models import Redirect
 from django.contrib.sites.models import get_current_site
 from django import http
@@ -7,17 +9,21 @@ class RedirectFallbackMiddleware(object):
     def process_response(self, request, response):
         if response.status_code != 404:
             return response # No need to check for a redirect for non-404 responses.
-        path = request.get_full_path()
+
+        full_path = request.get_full_path()
         current_site = get_current_site(request)
+
+        r = None
         try:
-            r = Redirect.objects.get(site__id__exact=current_site.id, old_path=path)
+            r = Redirect.objects.get(site=current_site, old_path=full_path)
         except Redirect.DoesNotExist:
-            r = None
-        if r is None and settings.APPEND_SLASH:
-            # Try removing the trailing slash.
+            pass
+        if settings.APPEND_SLASH and not request.path.endswith('/'):
+            # Try appending a trailing slash.
+            path_len = len(request.path)
+            full_path = full_path[:path_len] + '/' + full_path[path_len:]
             try:
-                r = Redirect.objects.get(site__id__exact=current_site.id,
-                    old_path=path[:path.rfind('/')]+path[path.rfind('/')+1:])
+                r = Redirect.objects.get(site=current_site, old_path=full_path)
             except Redirect.DoesNotExist:
                 pass
         if r is not None:

django/contrib/redirects/tests.py

@@ -8,10 +8,10 @@
 
 
 @override_settings(
-    SITE_ID=1,
-    APPEND_SLASH=True,
+    APPEND_SLASH=False,
     MIDDLEWARE_CLASSES=list(settings.MIDDLEWARE_CLASSES) +
         ['django.contrib.redirects.middleware.RedirectFallbackMiddleware'],
+    SITE_ID=1,
 )
 class RedirectTests(TestCase):
 
@@ -23,20 +23,32 @@ def test_model(self):
             site=self.site, old_path='/initial', new_path='/new_target')
         self.assertEqual(six.text_type(r1), "/initial ---> /new_target")
 
-    def test_redirect_middleware(self):
-        r1 = Redirect.objects.create(
+    def test_redirect(self):
+        Redirect.objects.create(
             site=self.site, old_path='/initial', new_path='/new_target')
         response = self.client.get('/initial')
         self.assertRedirects(response,
             '/new_target', status_code=301, target_status_code=404)
-        # Works also with trailing slash
-        response = self.client.get('/initial/')
+
+    @override_settings(APPEND_SLASH=True)
+    def test_redirect_with_append_slash(self):
+        Redirect.objects.create(
+            site=self.site, old_path='/initial/', new_path='/new_target/')
+        response = self.client.get('/initial')
         self.assertRedirects(response,
-            '/new_target', status_code=301, target_status_code=404)
+            '/new_target/', status_code=301, target_status_code=404)
+
+    @override_settings(APPEND_SLASH=True)
+    def test_redirect_with_append_slash_and_query_string(self):
+        Redirect.objects.create(
+            site=self.site, old_path='/initial/?foo', new_path='/new_target/')
+        response = self.client.get('/initial?foo')
+        self.assertRedirects(response,
+            '/new_target/', status_code=301, target_status_code=404)
 
     def test_response_gone(self):
         """When the redirect target is '', return a 410"""
-        r1 = Redirect.objects.create(
+        Redirect.objects.create(
             site=self.site, old_path='/initial', new_path='')
         response = self.client.get('/initial')
         self.assertEqual(response.status_code, 410)