From 196ac8f8b31302d026af6d79f764c8f146a8c78e Mon Sep 17 00:00:00 2001 From: Jannis Leidel Date: Wed, 20 Apr 2011 14:41:47 +0000 Subject: [PATCH] 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 | 95 ++++++++++++++++++++ django/contrib/flatpages/tests/views.py | 62 +++++++++++++ django/contrib/flatpages/views.py | 18 ++-- docs/ref/contrib/flatpages.txt | 12 +++ docs/releases/1.4.txt | 13 +++ 5 files changed, 195 insertions(+), 5 deletions(-) diff --git a/django/contrib/flatpages/tests/middleware.py b/django/contrib/flatpages/tests/middleware.py index e01ca33d6d140..01a9b355d7ddb 100644 --- a/django/contrib/flatpages/tests/middleware.py +++ b/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 @@ def test_fallback_authenticated_flatpage(self): response = self.client.get('/sekrit/') self.assertEqual(response.status_code, 200) self.assertContains(response, "

Isn't it sekrit!

") + + 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, "

Isn't it special!

") + + +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, "

Root

") + + diff --git a/django/contrib/flatpages/tests/views.py b/django/contrib/flatpages/tests/views.py index 4452745393fb8..54ab52ab7e5d0 100644 --- a/django/contrib/flatpages/tests/views.py +++ b/django/contrib/flatpages/tests/views.py @@ -73,3 +73,65 @@ def test_view_flatpage_special_chars(self): response = self.client.get('/flatpage_root/some.very_special~chars-here/') self.assertEqual(response.status_code, 200) self.assertContains(response, "

Isn't it special!

") + + +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) diff --git a/django/contrib/flatpages/views.py b/django/contrib/flatpages/views.py index 88ef4da65e226..ef7fda5d5c797 100644 --- a/django/contrib/flatpages/views.py +++ b/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 - f = get_object_or_404(FlatPage, url__exact=url, sites__id__exact=settings.SITE_ID) + url = '/' + url + 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 diff --git a/docs/ref/contrib/flatpages.txt b/docs/ref/contrib/flatpages.txt index 0411e8f93f60e..a99f75eb2d8f0 100644 --- a/docs/ref/contrib/flatpages.txt +++ b/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 diff --git a/docs/releases/1.4.txt b/docs/releases/1.4.txt index 56a59195055fd..9de896d33d232 100644 --- a/docs/releases/1.4.txt +++ b/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`.