Fixed #6213 -- Updated the flatpages app to only append a slash if the flatpage actually exist.

The FlatpageFallbackMiddleware (and the view) now only add a trailing slash and redirect if the resulting URL refers to an existing flatpage. Previously requesting /notaflatpageoravalidurl would redirect to /notaflatpageoravalidurl/, which would then raise a 404. Requesting /notaflatpageoravalidurl now will immediately raise a 404. Also, Redirects returned by flatpages are now permanent (301 status code) to match the behaviour of the CommonMiddleware.

Thanks to Steve Losh for the initial work on the patch.

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

django/contrib/flatpages/tests/middleware.py

@@ -1,6 +1,7 @@
 import os
 from django.conf import settings
 from django.contrib.auth.models import User
+from django.contrib.flatpages.models import FlatPage
 from django.test import TestCase
 
 class FlatpageMiddlewareTests(TestCase):
@@ -68,3 +69,97 @@ class FlatpageMiddlewareTests(TestCase):
         response = self.client.get('/sekrit/')
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "<p>Isn't it sekrit!</p>")
+
+    def test_fallback_flatpage_special_chars(self):
+        "A flatpage with special chars in the URL can be served by the fallback middleware"
+        fp = FlatPage.objects.create(
+            url="/some.very_special~chars-here/",
+            title="A very special page",
+            content="Isn't it special!",
+            enable_comments=False,
+            registration_required=False,
+        )
+        fp.sites.add(1)
+
+        response = self.client.get('/some.very_special~chars-here/')
+        self.assertEquals(response.status_code, 200)
+        self.assertContains(response, "<p>Isn't it special!</p>")
+
+
+class FlatpageMiddlewareAppendSlashTests(TestCase):
+    fixtures = ['sample_flatpages']
+    urls = 'django.contrib.flatpages.tests.urls'
+
+    def setUp(self):
+        self.old_MIDDLEWARE_CLASSES = settings.MIDDLEWARE_CLASSES
+        flatpage_middleware_class = 'django.contrib.flatpages.middleware.FlatpageFallbackMiddleware'
+        if flatpage_middleware_class not in settings.MIDDLEWARE_CLASSES:
+            settings.MIDDLEWARE_CLASSES += (flatpage_middleware_class,)
+        self.old_TEMPLATE_DIRS = settings.TEMPLATE_DIRS
+        settings.TEMPLATE_DIRS = (
+            os.path.join(
+                os.path.dirname(__file__),
+                'templates'
+            ),
+        )
+        self.old_LOGIN_URL = settings.LOGIN_URL
+        settings.LOGIN_URL = '/accounts/login/'
+        self.old_APPEND_SLASH = settings.APPEND_SLASH
+        settings.APPEND_SLASH = True
+
+    def tearDown(self):
+        settings.MIDDLEWARE_CLASSES = self.old_MIDDLEWARE_CLASSES
+        settings.TEMPLATE_DIRS = self.old_TEMPLATE_DIRS
+        settings.LOGIN_URL = self.old_LOGIN_URL
+        settings.APPEND_SLASH = self.old_APPEND_SLASH
+
+    def test_redirect_view_flatpage(self):
+        "A flatpage can be served through a view and should add a slash"
+        response = self.client.get('/flatpage_root/flatpage')
+        self.assertRedirects(response, '/flatpage_root/flatpage/', status_code=301)
+
+    def test_redirect_view_non_existent_flatpage(self):
+        "A non-existent flatpage raises 404 when served through a view and should not add a slash"
+        response = self.client.get('/flatpage_root/no_such_flatpage')
+        self.assertEquals(response.status_code, 404)
+
+    def test_redirect_fallback_flatpage(self):
+        "A flatpage can be served by the fallback middlware and should add a slash"
+        response = self.client.get('/flatpage')
+        self.assertRedirects(response, '/flatpage/', status_code=301)
+
+    def test_redirect_fallback_non_existent_flatpage(self):
+        "A non-existent flatpage raises a 404 when served by the fallback middlware and should not add a slash"
+        response = self.client.get('/no_such_flatpage')
+        self.assertEquals(response.status_code, 404)
+
+    def test_redirect_fallback_flatpage_special_chars(self):
+        "A flatpage with special chars in the URL can be served by the fallback middleware and should add a slash"
+        fp = FlatPage.objects.create(
+            url="/some.very_special~chars-here/",
+            title="A very special page",
+            content="Isn't it special!",
+            enable_comments=False,
+            registration_required=False,
+        )
+        fp.sites.add(1)
+
+        response = self.client.get('/some.very_special~chars-here')
+        self.assertRedirects(response, '/some.very_special~chars-here/', status_code=301)
+
+    def test_redirect_fallback_flatpage_root(self):
+        "A flatpage at / should not cause a redirect loop when APPEND_SLASH is set"
+        fp = FlatPage.objects.create(
+            url="/",
+            title="Root",
+            content="Root",
+            enable_comments=False,
+            registration_required=False,
+        )
+        fp.sites.add(1)
+
+        response = self.client.get('/')
+        self.assertEquals(response.status_code, 200)
+        self.assertContains(response, "<p>Root</p>")
+
+

django/contrib/flatpages/tests/views.py

@@ -73,3 +73,65 @@ class FlatpageViewTests(TestCase):
         response = self.client.get('/flatpage_root/some.very_special~chars-here/')
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "<p>Isn't it special!</p>")
+
+
+class FlatpageViewAppendSlashTests(TestCase):
+    fixtures = ['sample_flatpages']
+    urls = 'django.contrib.flatpages.tests.urls'
+
+    def setUp(self):
+        self.old_MIDDLEWARE_CLASSES = settings.MIDDLEWARE_CLASSES
+        flatpage_middleware_class = 'django.contrib.flatpages.middleware.FlatpageFallbackMiddleware'
+        if flatpage_middleware_class in settings.MIDDLEWARE_CLASSES:
+            settings.MIDDLEWARE_CLASSES = tuple(m for m in settings.MIDDLEWARE_CLASSES if m != flatpage_middleware_class)
+        self.old_TEMPLATE_DIRS = settings.TEMPLATE_DIRS
+        settings.TEMPLATE_DIRS = (
+            os.path.join(
+                os.path.dirname(__file__),
+                'templates'
+            ),
+        )
+        self.old_LOGIN_URL = settings.LOGIN_URL
+        settings.LOGIN_URL = '/accounts/login/'
+        self.old_APPEND_SLASH = settings.APPEND_SLASH
+        settings.APPEND_SLASH = True
+
+    def tearDown(self):
+        settings.MIDDLEWARE_CLASSES = self.old_MIDDLEWARE_CLASSES
+        settings.TEMPLATE_DIRS = self.old_TEMPLATE_DIRS
+        settings.LOGIN_URL = self.old_LOGIN_URL
+        settings.APPEND_SLASH = self.old_APPEND_SLASH
+
+    def test_redirect_view_flatpage(self):
+        "A flatpage can be served through a view and should add a slash"
+        response = self.client.get('/flatpage_root/flatpage')
+        self.assertRedirects(response, '/flatpage_root/flatpage/', status_code=301)
+
+    def test_redirect_view_non_existent_flatpage(self):
+        "A non-existent flatpage raises 404 when served through a view and should not add a slash"
+        response = self.client.get('/flatpage_root/no_such_flatpage')
+        self.assertEquals(response.status_code, 404)
+
+    def test_redirect_fallback_flatpage(self):
+        "A fallback flatpage won't be served if the middleware is disabled and should not add a slash"
+        response = self.client.get('/flatpage')
+        self.assertEquals(response.status_code, 404)
+
+    def test_redirect_fallback_non_existent_flatpage(self):
+        "A non-existent flatpage won't be served if the fallback middlware is disabled and should not add a slash"
+        response = self.client.get('/no_such_flatpage')
+        self.assertEquals(response.status_code, 404)
+
+    def test_redirect_view_flatpage_special_chars(self):
+        "A flatpage with special chars in the URL can be served through a view and should add a slash"
+        fp = FlatPage.objects.create(
+            url="/some.very_special~chars-here/",
+            title="A very special page",
+            content="Isn't it special!",
+            enable_comments=False,
+            registration_required=False,
+        )
+        fp.sites.add(1)
+
+        response = self.client.get('/flatpage_root/some.very_special~chars-here')
+        self.assertRedirects(response, '/flatpage_root/some.very_special~chars-here/', status_code=301)

django/contrib/flatpages/views.py

@@ -1,7 +1,7 @@
 from django.contrib.flatpages.models import FlatPage
 from django.template import loader, RequestContext
 from django.shortcuts import get_object_or_404
-from django.http import HttpResponse, HttpResponseRedirect
+from django.http import Http404, HttpResponse, HttpResponsePermanentRedirect
 from django.conf import settings
 from django.core.xheaders import populate_xheaders
 from django.utils.safestring import mark_safe
@@ -28,11 +28,19 @@ def flatpage(request, url):
         flatpage
             `flatpages.flatpages` object
     """
-    if not url.endswith('/') and settings.APPEND_SLASH:
-        return HttpResponseRedirect("%s/" % request.path)
     if not url.startswith('/'):
-        url = "/" + url
+        url = '/' + url
-    f = get_object_or_404(FlatPage, url__exact=url, sites__id__exact=settings.SITE_ID)
+    try:
+        f = get_object_or_404(FlatPage,
+            url__exact=url, sites__id__exact=settings.SITE_ID)
+    except Http404:
+        if not url.endswith('/') and settings.APPEND_SLASH:
+            url += '/'
+            f = get_object_or_404(FlatPage,
+                url__exact=url, sites__id__exact=settings.SITE_ID)
+            return HttpResponsePermanentRedirect('%s/' % request.path)
+        else:
+            raise
     return render_flatpage(request, f)
 
 @csrf_protect

docs/ref/contrib/flatpages.txt

@@ -77,6 +77,18 @@ does all of the work.
           :class:`~django.template.RequestContext` in rendering the
           template.
 
+    .. versionchanged:: 1.4
+       The middleware will only add a trailing slash and redirect (by looking
+       at the :setting:`APPEND_SLASH` setting) if the resulting URL refers to
+       a valid flatpage. Previously requesting a non-existent flatpage
+       would redirect to the same URL with an apppended slash first and
+       subsequently raise a 404.
+
+    .. versionchanged:: 1.4
+       Redirects by the middlware are permanent (301 status code) instead of
+       temporary (302) to match behaviour of the
+       :class:`~django.middleware.common.CommonMiddleware`.
+
     If it doesn't find a match, the request continues to be processed as usual.
 
     The middleware only gets activated for 404s -- not for 500s or responses

docs/releases/1.4.txt

@@ -78,3 +78,16 @@ Django instance, and try to submit it to the upgraded Django instance:
 
   * time period: the amount of time you expect user to take filling out
     such forms.
+
+django.contrib.flatpages
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Starting in the 1.4 release the
+:class:`~django.contrib.flatpages.middleware.FlatpageFallbackMiddleware` only
+adds a trailing slash and redirects if the resulting URL refers to an existing
+flatpage. For example, requesting ``/notaflatpageoravalidurl`` in a previous
+version would redirect to ``/notaflatpageoravalidurl/``, which would
+subsequently raise a 404. Requesting ``/notaflatpageoravalidurl`` now will
+immediately raise a 404. Additionally redirects returned by flatpages are now
+permanent (301 status code) to match the behaviour of the
+:class:`~django.middleware.common.CommonMiddleware`.