Merge pull request #1679 from mjec/issue-1678

Fix issue #1678

wagtail/wagtailredirects/models.py

@@ -54,11 +54,19 @@ class Redirect(models.Model):
         if path.endswith('/'):
             path = path[:-1]
 
+        # Parameters must be sorted alphabetically
+        parameters = url_parsed[3]
+        parameters_components = parameters.split(';')
+        parameters = ';'.join(sorted(parameters_components))
+
         # Query string components must be sorted alphabetically
         query_string = url_parsed[4]
         query_string_components = query_string.split('&')
         query_string = '&'.join(sorted(query_string_components))
 
+        if parameters:
+            path = path + ';' + parameters
+
         # Add query string to path
         if query_string:
             path = path + '?' + query_string

wagtail/wagtailredirects/tests.py

@@ -11,25 +11,31 @@ class TestRedirects(TestCase):
         normalise_path = models.Redirect.normalise_path
 
         # Create a path
-        path = normalise_path('/Hello/world.html?foo=Bar&Baz=quux2')
+        path = normalise_path('/Hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2')
 
         # Test against equivalant paths
-        self.assertEqual(path, normalise_path('/Hello/world.html?foo=Bar&Baz=quux2')) # The exact same URL
-        self.assertEqual(path, normalise_path('http://mywebsite.com:8000/Hello/world.html?foo=Bar&Baz=quux2')) # Scheme, hostname and port ignored
-        self.assertEqual(path, normalise_path('Hello/world.html?foo=Bar&Baz=quux2')) # Leading slash can be omitted
-        self.assertEqual(path, normalise_path('Hello/world.html/?foo=Bar&Baz=quux2')) # Trailing slashes are ignored
-        self.assertEqual(path, normalise_path('/Hello/world.html?foo=Bar&Baz=quux2#cool')) # Fragments are ignored
-        self.assertEqual(path, normalise_path('/Hello/world.html?Baz=quux2&foo=Bar')) # Order of query string parameters are ignored
-        self.assertEqual(path, normalise_path('  /Hello/world.html?foo=Bar&Baz=quux2')) # Leading whitespace
-        self.assertEqual(path, normalise_path('/Hello/world.html?foo=Bar&Baz=quux2  ')) # Trailing whitespace
+        self.assertEqual(path, normalise_path('/Hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # The exact same URL
+        self.assertEqual(path, normalise_path('http://mywebsite.com:8000/Hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # Scheme, hostname and port ignored
+        self.assertEqual(path, normalise_path('Hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # Leading slash can be omitted
+        self.assertEqual(path, normalise_path('Hello/world.html/;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # Trailing slashes are ignored
+        self.assertEqual(path, normalise_path('/Hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2#cool')) # Fragments are ignored
+        self.assertEqual(path, normalise_path('/Hello/world.html;fizz=three;buzz=five?Baz=quux2&foo=Bar')) # Order of query string parameters is ignored
+        self.assertEqual(path, normalise_path('  /Hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # Leading whitespace
+        self.assertEqual(path, normalise_path('/Hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2  ')) # Trailing whitespace
+
+        # This assertion should be changed if parameter order is important
+        self.assertEqual(path, normalise_path('/Hello/world.html;buzz=five;fizz=three?foo=Bar&Baz=quux2')) # Order of parameters is ignored
 
         # Test against different paths
-        self.assertNotEqual(path, normalise_path('/hello/world.html?foo=Bar&Baz=quux2')) # 'hello' is lowercase
-        self.assertNotEqual(path, normalise_path('/Hello/world?foo=Bar&Baz=quux2')) # No '.html'
-        self.assertNotEqual(path, normalise_path('/Hello/world.html?foo=bar&Baz=Quux2')) # Query string parameters have wrong case
-        self.assertNotEqual(path, normalise_path('/Hello/world.html?foo=Bar&baz=quux2')) # ditto
-        self.assertNotEqual(path, normalise_path('/Hello/WORLD.html?foo=Bar&Baz=quux2')) # 'WORLD' is uppercase
-        self.assertNotEqual(path, normalise_path('/Hello/world.htm?foo=Bar&Baz=quux2')) # '.htm' is not the same as '.html'
+        self.assertNotEqual(path, normalise_path('/hello/world.html;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # 'hello' is lowercase
+        self.assertNotEqual(path, normalise_path('/Hello/world;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # No '.html'
+        self.assertNotEqual(path, normalise_path('/Hello/world.html;fizz=three;buzz=five?foo=bar&Baz=Quux2')) # Query string parameter value has wrong case
+        self.assertNotEqual(path, normalise_path('/Hello/world.html;fizz=three;buzz=five?foo=Bar&baz=quux2')) # Query string parameter name has wrong case
+        self.assertNotEqual(path, normalise_path('/Hello/world.html;fizz=three;buzz=Five?foo=Bar&Baz=quux2')) # Parameter value has wrong case
+        self.assertNotEqual(path, normalise_path('/Hello/world.html;Fizz=three;buzz=five?foo=Bar&Baz=quux2')) # Parameter name has wrong case
+        self.assertNotEqual(path, normalise_path('/Hello/world.html?foo=Bar&Baz=quux2')) # Missing params
+        self.assertNotEqual(path, normalise_path('/Hello/WORLD.html;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # 'WORLD' is uppercase
+        self.assertNotEqual(path, normalise_path('/Hello/world.htm;fizz=three;buzz=five?foo=Bar&Baz=quux2')) # '.htm' is not the same as '.html'
 
         # Normalise some rubbish to make sure it doesn't crash
         normalise_path('This is not a URL')