Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

try variants without query string when redirect lookup fails #5

Merged
merged 2 commits into from

3 participants

@noelbush

This is quite similar to the pull request at https://github.com/salvaorenick/django-cms-redirects/pull/4/files , except we don't look at PATH_INFO.

There are cases when requests arrive with additional junk in the query string which interferes with finding the redirect. For instance, we want to define a redirect "/some-old-address/" to point to another page or path, but we don't want this to fail when somebody clicks a link that has "/some-old-address/?google=analytics&tracking=junk&and=other&irrelevant=parameters". So here we first try the path as is, then do the test without a trailing slash (as currently), and then try slashed and slashless variants without the query string. There's also a test included.

Sorry for the weirdness with whitespace in the diff. I tried to squash some commits and something odd happened.

@andrewschoen

Looks good, thanks for this!

@andrewschoen andrewschoen merged commit 95584ff into salvaorenick:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 31, 2012
  1. @nsh-ableton

    try variants of a URL (without query string, with/without trailing sl…

    nsh-ableton authored
    …ash) when initial redirect lookup fails
  2. @nsh-ableton

    try variants of a URL (without query string, with/without trailing sl…

    nsh-ableton authored
    …ash) when initial redirect lookup fails
This page is out of date. Refresh to see the latest.
Showing with 56 additions and 20 deletions.
  1. +35 −11 cms_redirects/middleware.py
  2. +21 −9 cms_redirects/tests.py
View
46 cms_redirects/middleware.py
@@ -2,21 +2,45 @@
from django import http
from django.conf import settings
+
+def get_redirect(old_path):
+ try:
+ r = CMSRedirect.objects.get(site__id__exact=settings.SITE_ID,
+ old_path=old_path)
+ except CMSRedirect.DoesNotExist:
+ r = None
+ return r
+
+
+def remove_slash(path):
+ return path[:path.rfind('/')]+path[path.rfind('/')+1:]
+
+
+def remove_query(path):
+ return path.split('?', 1)[0]
+
+
class RedirectFallbackMiddleware(object):
def process_exception(self, request, exception):
if isinstance(exception, http.Http404):
+
+ # First try the whole path.
path = request.get_full_path()
- try:
- r = CMSRedirect.objects.get(site__id__exact=settings.SITE_ID, old_path=path)
- except CMSRedirect.DoesNotExist:
- r = None
+ r = get_redirect(path)
+
+ # It could be that we need to try without a trailing slash.
if r is None and settings.APPEND_SLASH:
- # Try removing the trailing slash.
- try:
- r = CMSRedirect.objects.get(site__id__exact=settings.SITE_ID,
- old_path=path[:path.rfind('/')]+path[path.rfind('/')+1:])
- except CMSRedirect.DoesNotExist:
- pass
+ r = get_redirect(remove_slash(path))
+
+ # It could be that the redirect is defined without a query string.
+ if r is None and path.count('?'):
+ r = get_redirect(remove_query(path))
+
+ # It could be that we need to try without query string and without a trailing slash.
+ if r is None and path.count('?') and settings.APPEND_SLASH:
+ r = get_redirect(remove_slash(remove_query(path)))
+
+
if r is not None:
if r.page:
if r.response_code == '302':
@@ -29,5 +53,5 @@ def process_exception(self, request, exception):
return http.HttpResponseRedirect(r.new_path)
else:
return http.HttpResponsePermanentRedirect(r.new_path)
-
+
View
30 cms_redirects/tests.py
@@ -10,7 +10,7 @@
class TestRedirects(unittest.TestCase):
def setUp(self):
settings.APPEND_SLASH = False
-
+
self.site = Site.objects.get_current()
page = Page()
@@ -22,21 +22,21 @@ def setUp(self):
title = Title(title="Hello world!")
title.page = page
title.language = u'en'
- title.save()
-
+ title.save()
+
def test_301_page_redirect(self):
r_301_page = CMSRedirect(site=self.site, page=self.page, old_path='/301_page.php')
r_301_page.save()
-
+
c = Client()
r = c.get('/301_page.php')
self.assertEqual(r.status_code, 301)
self.assertEqual(r._headers['location'][1], 'http://testserver/')
-
+
def test_302_page_redirect(self):
r_302_page = CMSRedirect(site=self.site, page=self.page, old_path='/302_page.php', response_code='302')
r_302_page.save()
-
+
c = Client()
r = c.get('/302_page.php')
self.assertEqual(r.status_code, 302)
@@ -45,7 +45,7 @@ def test_302_page_redirect(self):
def test_301_path_redirect(self):
r_301_path = CMSRedirect(site=self.site, new_path='/', old_path='/301_path.php')
r_301_path.save()
-
+
c = Client()
r = c.get('/301_path.php')
self.assertEqual(r.status_code, 301)
@@ -54,7 +54,7 @@ def test_301_path_redirect(self):
def test_302_path_redirect(self):
r_302_path = CMSRedirect(site=self.site, new_path='/', old_path='/302_path.php', response_code='302')
r_302_path.save()
-
+
c = Client()
r = c.get('/302_path.php')
self.assertEqual(r.status_code, 302)
@@ -63,8 +63,20 @@ def test_302_path_redirect(self):
def test_410_redirect(self):
r_410 = CMSRedirect(site=self.site, old_path='/410.php', response_code='302')
r_410.save()
-
+
c = Client()
r = c.get('/410.php')
self.assertEqual(r.status_code, 410)
+ def test_redirect_can_ignore_query_string(self):
+ """
+ Set up a redirect as in the generic 301 page case, but then try to get this page with
+ a query string appended. Succeed nonetheless.
+ """
+ r_301_page = CMSRedirect(site=self.site, page=self.page, old_path='/301_page.php')
+ r_301_page.save()
+
+ c = Client()
+ r = c.get('/301_page.php?this=is&a=query&string')
+ self.assertEqual(r.status_code, 301)
+ self.assertEqual(r._headers['location'][1], 'http://testserver')
Something went wrong with that request. Please try again.