make the query string uri-safe before redirects (bug 564820)

apps/addons/tests/test_views.py

@@ -376,3 +376,9 @@ def test_button_caching():
     fmt = '%a, %d %b %Y %H:%M:%S GMT'
     expires = datetime.strptime(response['Expires'], fmt)
     assert (expires - datetime.now()).days >= 365
+
+
+def test_unicode_redirect():
+    url = '/en-US/firefox/addon/2848?xx=\xc2\xbcwhscheck\xc2\xbe'
+    response = test.Client().get(url)
+    eq_(response.status_code, 301)

apps/amo/middleware.py

@@ -3,12 +3,13 @@ Borrowed from: http://code.google.com/p/django-localeurl
 
 Note: didn't make sense to use localeurl since we need to capture app as well
 """
+import contextlib
 import urllib
 
 from django.contrib.sessions.middleware import SessionMiddleware
-from django.http import HttpResponsePermanentRedirect, QueryDict
+from django.http import HttpResponsePermanentRedirect
 from django.middleware import common
-from django.utils.encoding import smart_str
+from django.utils.encoding import iri_to_uri, smart_str
 
 import tower
 
@@ -102,7 +103,31 @@ class RemoveSlashMiddleware(object):
             # Use request.path because we munged app/locale in path_info.
             newurl = request.path[:-1]
             if request.GET:
-                newurl += '?' + request.META['QUERY_STRING']
+                with safe_query_string(request):
+                    newurl += '?' + request.META['QUERY_STRING']
             return HttpResponsePermanentRedirect(newurl)
         else:
             return response
+
+
+@contextlib.contextmanager
+def safe_query_string(request):
+    """
+    Turn the QUERY_STRING into a unicode- and ascii-safe string.
+
+    We need unicode so it can be combined with a reversed URL, but it has to be
+    ascii to go in a Location header.  iri_to_uri seems like a good compromise.
+    """
+    qs = request.META['QUERY_STRING']
+    try:
+        request.META['QUERY_STRING'] = iri_to_uri(qs)
+        yield
+    finally:
+        request.META['QUERY_STRING'] = qs
+
+
+class CommonMiddleware(common.CommonMiddleware):
+
+    def process_request(self, request):
+        with safe_query_string(request):
+            return super(CommonMiddleware, self).process_request(request)

apps/amo/tests/test_middleware.py

@@ -4,8 +4,6 @@ from nose.tools import eq_
 
 
 def test_no_vary_cookie():
-    c = test.Client()
-
     # We don't break good usage of Vary.
     response = test.Client().get('/')
     eq_(response['Vary'], 'Accept-Language')
@@ -21,6 +19,6 @@ def test_redirect_with_unicode_get():
 
 
 def test_trailing_slash_middleware():
-    response = test.Client().get('/en-US/firefox/about/')
+    response = test.Client().get(u'/en-US/firefox/about/?xxx=\xc3')
     eq_(response.status_code, 301)
-    assert response['Location'].endswith('/en-US/firefox/about')
+    assert response['Location'].endswith('/en-US/firefox/about?xxx=%C3%83')

settings.py

@@ -171,7 +171,7 @@ MIDDLEWARE_CLASSES = (
     'commonware.middleware.SetRemoteAddrFromForwardedFor',
     'commonware.log.ThreadRequestMiddleware',
 
-    'django.middleware.common.CommonMiddleware',
+    'amo.middleware.CommonMiddleware',
     'amo.middleware.NoVarySessionMiddleware',
     'django.contrib.messages.middleware.MessageMiddleware',
     'django.contrib.auth.middleware.AuthenticationMiddleware',