From 9128762f1608f9633005f16c95270058a50ada2b Mon Sep 17 00:00:00 2001 From: Bas Peschier Date: Sun, 22 Mar 2015 20:04:31 +0100 Subject: [PATCH] Fixed #19910 -- Added slash to i18n redirect if APPEND_SLASH is set. This introduces a force_append_slash argument for request.get_full_path() which is used by RedirectFallbackMiddleware and CommonMiddleware when handling redirects for settings.APPEND_SLASH. --- django/contrib/redirects/middleware.py | 10 +++++----- django/http/request.py | 5 +++-- django/middleware/common.py | 22 ++++------------------ django/middleware/locale.py | 13 ++++++++----- tests/i18n/patterns/tests.py | 4 ++-- 5 files changed, 22 insertions(+), 32 deletions(-) diff --git a/django/contrib/redirects/middleware.py b/django/contrib/redirects/middleware.py index 903eb11abf..dc8d94bc96 100644 --- a/django/contrib/redirects/middleware.py +++ b/django/contrib/redirects/middleware.py @@ -34,12 +34,12 @@ class RedirectFallbackMiddleware(object): r = Redirect.objects.get(site=current_site, old_path=full_path) except Redirect.DoesNotExist: pass - if settings.APPEND_SLASH and not request.path.endswith('/'): - # Try appending a trailing slash. - path_len = len(request.path) - full_path = full_path[:path_len] + '/' + full_path[path_len:] + if r is None and settings.APPEND_SLASH and not request.path.endswith('/'): try: - r = Redirect.objects.get(site=current_site, old_path=full_path) + r = Redirect.objects.get( + site=current_site, + old_path=request.get_full_path(force_append_slash=True), + ) except Redirect.DoesNotExist: pass if r is not None: diff --git a/django/http/request.py b/django/http/request.py index 7989d064b6..97592b444a 100644 --- a/django/http/request.py +++ b/django/http/request.py @@ -99,11 +99,12 @@ class HttpRequest(object): msg += " The domain name provided is not valid according to RFC 1034/1035." raise DisallowedHost(msg) - def get_full_path(self): + def get_full_path(self, force_append_slash=False): # RFC 3986 requires query string arguments to be in the ASCII range. # Rather than crash if this doesn't happen, we encode defensively. - return '%s%s' % ( + return '%s%s%s' % ( escape_uri_path(self.path), + '/' if force_append_slash and not self.path.endswith('/') else '', ('?' + iri_to_uri(self.META.get('QUERY_STRING', ''))) if self.META.get('QUERY_STRING', '') else '' ) diff --git a/django/middleware/common.py b/django/middleware/common.py index 6c5843220f..dad1e6b2eb 100644 --- a/django/middleware/common.py +++ b/django/middleware/common.py @@ -6,9 +6,7 @@ from django import http from django.conf import settings from django.core import urlresolvers from django.core.mail import mail_managers -from django.utils import six from django.utils.encoding import force_text -from django.utils.http import urlquote logger = logging.getLogger('django.request') @@ -60,7 +58,7 @@ class CommonMiddleware(object): # Check for a redirect based on settings.APPEND_SLASH # and settings.PREPEND_WWW host = request.get_host() - old_url = [host, request.path] + old_url = [host, request.get_full_path()] new_url = old_url[:] if (settings.PREPEND_WWW and old_url[0] and @@ -73,7 +71,7 @@ class CommonMiddleware(object): urlconf = getattr(request, 'urlconf', None) if (not urlresolvers.is_valid_path(request.path_info, urlconf) and urlresolvers.is_valid_path("%s/" % request.path_info, urlconf)): - new_url[1] = new_url[1] + '/' + new_url[1] = request.get_full_path(force_append_slash=True) if settings.DEBUG and request.method in ('POST', 'PUT', 'PATCH'): raise RuntimeError(("" "You called this URL via %(method)s, but the URL doesn't end " @@ -89,21 +87,9 @@ class CommonMiddleware(object): if new_url[0] != old_url[0]: newurl = "%s://%s%s" % ( request.scheme, - new_url[0], urlquote(new_url[1])) + new_url[0], new_url[1]) else: - newurl = urlquote(new_url[1]) - if request.META.get('QUERY_STRING', ''): - if six.PY3: - newurl += '?' + request.META['QUERY_STRING'] - else: - # `query_string` is a bytestring. Appending it to the unicode - # string `newurl` will fail if it isn't ASCII-only. This isn't - # allowed; only broken software generates such query strings. - # Better drop the invalid query string than crash (#15152). - try: - newurl += '?' + request.META['QUERY_STRING'].decode() - except UnicodeDecodeError: - pass + newurl = new_url[1] return self.response_redirect_class(newurl) def process_response(self, request, response): diff --git a/django/middleware/locale.py b/django/middleware/locale.py index 1a5b086148..271a43cbfb 100644 --- a/django/middleware/locale.py +++ b/django/middleware/locale.py @@ -34,15 +34,18 @@ class LocaleMiddleware(object): urlconf = getattr(request, 'urlconf', None) language_path = '/%s%s' % (language, request.path_info) path_valid = is_valid_path(language_path, urlconf) - if (not path_valid and settings.APPEND_SLASH - and not language_path.endswith('/')): - path_valid = is_valid_path("%s/" % language_path, urlconf) + path_needs_slash = ( + not path_valid and ( + settings.APPEND_SLASH and not language_path.endswith('/') + and is_valid_path('%s/' % language_path, urlconf) + ) + ) - if path_valid: + if path_valid or path_needs_slash: script_prefix = get_script_prefix() # Insert language after the script prefix and before the # rest of the URL - language_url = request.get_full_path().replace( + language_url = request.get_full_path(force_append_slash=path_needs_slash).replace( script_prefix, '%s%s/' % (script_prefix, language), 1 diff --git a/tests/i18n/patterns/tests.py b/tests/i18n/patterns/tests.py index f2949ed596..eb020fbdaf 100644 --- a/tests/i18n/patterns/tests.py +++ b/tests/i18n/patterns/tests.py @@ -247,8 +247,8 @@ class URLRedirectWithoutTrailingSlashTests(URLTestCaseBase): def test_en_redirect(self): response = self.client.get('/account/register', HTTP_ACCEPT_LANGUAGE='en', follow=True) - # target status code of 301 because of CommonMiddleware redirecting - self.assertIn(('/en/account/register/', 301), response.redirect_chain) + # We only want one redirect, bypassing CommonMiddleware + self.assertListEqual(response.redirect_chain, [('/en/account/register/', 302)]) self.assertRedirects(response, '/en/account/register/', 302) response = self.client.get('/prefixed.xml', HTTP_ACCEPT_LANGUAGE='en', follow=True)