Fixed #3228 -- Added new APPEND_SLASH handling behaviour in the commo…

…n middleware. Makes customisation a bit easier. Thanks, Mihai Preda and Andy Gayton.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@6852 bcc190cf-cafb-0310-a4f2-bffc1f526a37

AUTHORS

@@ -133,6 +133,7 @@ answer newbie questions, and generally made Django that much better:
     Jorge Gajon <gajon@gajon.org>
     gandalf@owca.info
     Marc Garcia <marc.garcia@accopensys.com>
+    Andy Gayton <andy-django@thecablelounge.com>
     Baishampayan Ghose
     Dimitris Glezos <dimitris@glezos.com>
     glin@seznam.cz
@@ -255,6 +256,7 @@ answer newbie questions, and generally made Django that much better:
     Gustavo Picon
     Luke Plant <http://lukeplant.me.uk/>
     plisk
+    Mihai Preda <mihai_preda@yahoo.com>
     Daniel Poelzleithner <http://poelzi.org/>
     polpak@yahoo.com
     Jyrki Pulliainen <jyrki.pulliainen@gmail.com>

django/middleware/common.py

@@ -5,6 +5,7 @@
 from django import http
 from django.core.mail import mail_managers
 from django.utils.http import urlquote
+from django.core import urlresolvers
 
 class CommonMiddleware(object):
     """
@@ -16,6 +17,12 @@ class CommonMiddleware(object):
           this middleware appends missing slashes and/or prepends missing
           "www."s.
 
+            - If APPEND_SLASH is set and the initial URL doesn't end with a
+              slash, and it is not found in urlpatterns, a new URL is formed by
+              appending a slash at the end. If this new URL is found in
+              urlpatterns, then an HTTP-redirect is returned to this new URL;
+              otherwise the initial URL is processed as usual. 
+
         - ETags: If the USE_ETAGS setting is set, ETags will be calculated from
           the entire page content and Not Modified responses will be returned
           appropriately.
@@ -33,27 +40,48 @@ def process_request(self, request):
                 if user_agent_regex.search(request.META['HTTP_USER_AGENT']):
                     return http.HttpResponseForbidden('<h1>Forbidden</h1>')
 
-        # Check for a redirect based on settings.APPEND_SLASH and settings.PREPEND_WWW
+        # Check for a redirect based on settings.APPEND_SLASH
+        # and settings.PREPEND_WWW
         host = request.get_host()
         old_url = [host, request.path]
         new_url = old_url[:]
-        if settings.PREPEND_WWW and old_url[0] and not old_url[0].startswith('www.'):
+
+        if (settings.PREPEND_WWW and old_url[0] and
+                not old_url[0].startswith('www.')):
             new_url[0] = 'www.' + old_url[0]
-        # Append a slash if append_slash is set and the URL doesn't have a
-        # trailing slash or a file extension.
-        if settings.APPEND_SLASH and (not old_url[1].endswith('/')) and ('.' not in old_url[1].split('/')[-1]):
-            new_url[1] = new_url[1] + '/'
-            if settings.DEBUG and request.method == 'POST':
-                raise RuntimeError, "You called this URL via POST, 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 POST data. Change your form to point to %s%s (note the trailing slash), or set APPEND_SLASH=False in your Django settings." % (new_url[0], new_url[1])
+
+        # 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('/')):
+            try:
+                urlresolvers.resolve(request.path)
+            except urlresolvers.Resolver404:
+                new_url[1] = new_url[1] + '/'
+                if settings.DEBUG and request.method == 'POST':
+                    raise RuntimeError, (""
+                    "You called this URL via POST, 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 POST data. "
+                    "Change your form to point to %s%s (note the trailing "
+                    "slash), or set APPEND_SLASH=False in your Django "
+                    "settings.") % (new_url[0], new_url[1])
+
         if new_url != old_url:
-            # Redirect
-            if new_url[0]:
-                newurl = "%s://%s%s" % (request.is_secure() and 'https' or 'http', new_url[0], urlquote(new_url[1]))
+            # Redirect if the target url exists
+            try:
+                urlresolvers.resolve(new_url[1])
+            except urlresolvers.Resolver404:
+                pass
             else:
-                newurl = urlquote(new_url[1])
-            if request.GET:
-                newurl += '?' + request.GET.urlencode()
-            return http.HttpResponsePermanentRedirect(newurl)
+                if new_url[0]:
+                    newurl = "%s://%s%s" % (
+                        request.is_secure() and 'https' or 'http',
+                        new_url[0], urlquote(new_url[1]))
+                else:
+                    newurl = urlquote(new_url[1])
+                if request.GET:
+                    newurl += '?' + request.GET.urlencode()
+                return http.HttpResponsePermanentRedirect(newurl)
 
         return None
 

docs/middleware.txt

@@ -58,11 +58,20 @@ Adds a few conveniences for perfectionists:
   which should be a list of strings.
 
 * Performs URL rewriting based on the ``APPEND_SLASH`` and ``PREPEND_WWW``
-  settings. If ``APPEND_SLASH`` is ``True``, URLs that lack a trailing
-  slash will be redirected to the same URL with a trailing slash, unless the
-  last component in the path contains a period. So ``foo.com/bar`` is
-  redirected to ``foo.com/bar/``, but ``foo.com/bar/file.txt`` is passed
-  through unchanged.
+  settings.
+
+  If ``APPEND_SLASH`` is ``True`` and the initial URL doesn't end with a slash,
+  and it is not found in urlpatterns, a new URL is formed by appending a slash
+  at the end. If this new URL is found in urlpatterns, then an HTTP-redirect is
+  returned to this new URL; otherwise the initial URL is processed as usual.
+
+  So ``foo.com/bar`` will be redirected to ``foo.com/bar/`` if you do not
+  have a valid urlpattern for ``foo.com/bar``, and do have a valid urlpattern
+  for ``foo.com/bar/``.
+
+  **New in Django development version:** The behaviour of ``APPEND_SLASH`` has
+  changed slightly in the development version (it didn't used to check to see
+  if the pattern was matched in the URL patterns).
 
   If ``PREPEND_WWW`` is ``True``, URLs that lack a leading "www." will be
   redirected to the same URL with a leading "www."

tests/regressiontests/middleware/tests.py

@@ -0,0 +1,93 @@
+# -*- coding: utf-8 -*-
+
+import unittest
+
+from django.test import TestCase 
+from django.http import HttpRequest
+from django.middleware.common import CommonMiddleware
+from django.conf import settings
+
+class CommonMiddlewareTest(TestCase):
+    def _get_request(self, path):
+        request = HttpRequest()
+        request.META = {
+            'SERVER_NAME': 'testserver',
+            'SERVER_PORT': 80,
+        }
+        request.path = "/middleware/%s" % path
+        return request
+
+    def test_append_slash_have_slash(self):
+        """
+        tests that urls with slashes go unmolested
+        """
+        settings.APPEND_SLASH = True
+        request = self._get_request('slash/')
+        self.assertEquals(CommonMiddleware().process_request(request), None)
+
+    def test_append_slash_slashless_resource(self):
+        """
+        tests that matches to explicit slashless urls go unmolested
+        """
+        settings.APPEND_SLASH = True
+        request = self._get_request('noslash')
+        self.assertEquals(CommonMiddleware().process_request(request), None)
+
+    def test_append_slash_slashless_unknown(self):
+        """
+        tests that APPEND_SLASH doesn't redirect to unknown resources
+        """
+        settings.APPEND_SLASH = True
+        request = self._get_request('unknown')
+        self.assertEquals(CommonMiddleware().process_request(request), None)
+
+    def test_append_slash_redirect(self):
+        """
+        tests that APPEND_SLASH redirects slashless urls to a valid pattern
+        """
+        settings.APPEND_SLASH = True
+        request = self._get_request('slash')
+        r = CommonMiddleware().process_request(request)
+        self.assertEquals(r.status_code, 301)
+        self.assertEquals(r['Location'], 'http://testserver/middleware/slash/')
+
+    def test_append_slash_no_redirect_on_POST_in_DEBUG(self):
+        """
+        tests that while in debug mode, an exception is raised with a warning
+        when a failed attempt is made to POST to an url which would normally be
+        redirected to a slashed version
+        """
+        settings.APPEND_SLASH = True
+        settings.DEBUG = True
+        request = self._get_request('slash')
+        request.method = 'POST'
+        self.assertRaises(
+            RuntimeError,
+            CommonMiddleware().process_request,
+            request)
+        try:
+            CommonMiddleware().process_request(request)
+        except RuntimeError, e:
+            self.assertTrue('end in a slash' in str(e))
+
+    def test_append_slash_disabled(self):
+        """
+        tests disabling append slash functionality
+        """
+        settings.APPEND_SLASH = False
+        request = self._get_request('slash')
+        self.assertEquals(CommonMiddleware().process_request(request), None)
+
+    def test_append_slash_quoted(self):
+        """
+        tests that urls which require quoting are redirected to their slash
+        version ok
+        """
+        settings.APPEND_SLASH = True
+        request = self._get_request('needsquoting#')
+        r = CommonMiddleware().process_request(request)
+        self.assertEquals(r.status_code, 301)
+        self.assertEquals(
+            r['Location'],
+            'http://testserver/middleware/needsquoting%23/')
+

tests/regressiontests/middleware/urls.py

@@ -0,0 +1,7 @@
+from django.conf.urls.defaults import patterns
+
+urlpatterns = patterns('',
+    (r'^noslash$', 'view'),
+    (r'^slash/$', 'view'),
+    (r'^needsquoting#/$', 'view'),
+)

tests/urls.py

@@ -14,4 +14,7 @@
 
     # django built-in views
     (r'^views/', include('regressiontests.views.urls')),
+
+    # test urlconf for middleware tests
+    (r'^middleware/', include('regressiontests.middleware.urls')),
 )