From 746987f916bfb24a0671429d9993ad727dc7c8bc Mon Sep 17 00:00:00 2001 From: Jannis Leidel Date: Fri, 2 Mar 2012 11:07:36 +0000 Subject: [PATCH] =?UTF-8?q?Fixed=20#17734=20--=20Made=20sure=20to=20only?= =?UTF-8?q?=20redirect=20translated=20URLs=20if=20they=20can=20actually=20?= =?UTF-8?q?be=20resolved=20to=20prevent=20unwanted=20redirects.=20Many=20t?= =?UTF-8?q?hanks=20to=20Orne=20Brocaar=20and=20Anssi=20K=C3=A4=C3=A4ri?= =?UTF-8?q?=C3=A4inen=20for=20input.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git-svn-id: http://code.djangoproject.com/svn/django/trunk@17621 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/core/urlresolvers.py | 14 ++++++++ django/middleware/common.py | 19 ++-------- django/middleware/locale.py | 17 ++++++--- tests/regressiontests/i18n/patterns/tests.py | 35 ++++++------------- .../i18n/patterns/urls/namespace.py | 1 + 5 files changed, 39 insertions(+), 47 deletions(-) diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index b634b56685..1497d43e91 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -518,3 +518,17 @@ def get_urlconf(default=None): changed from the default one. """ return getattr(_urlconfs, "value", default) + +def is_valid_path(path, urlconf=None): + """ + Returns True if the given path resolves against the default URL resolver, + False otherwise. + + This is a convenience method to make working with "is this a match?" cases + easier, avoiding unnecessarily indented try...except blocks. + """ + try: + resolve(path, urlconf) + return True + except Resolver404: + return False diff --git a/django/middleware/common.py b/django/middleware/common.py index aaa094d15a..d894ec832f 100644 --- a/django/middleware/common.py +++ b/django/middleware/common.py @@ -64,8 +64,8 @@ class CommonMiddleware(object): # trailing slash and there is no pattern for the current path if settings.APPEND_SLASH and (not old_url[1].endswith('/')): urlconf = getattr(request, 'urlconf', None) - if (not _is_valid_path(request.path_info, urlconf) and - _is_valid_path("%s/" % request.path_info, urlconf)): + 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] + '/' if settings.DEBUG and request.method == 'POST': raise RuntimeError(("" @@ -151,18 +151,3 @@ def _is_internal_request(domain, referer): """ # Different subdomains are treated as different domains. return referer is not None and re.match("^https?://%s/" % re.escape(domain), referer) - -def _is_valid_path(path, urlconf=None): - """ - Returns True if the given path resolves against the default URL resolver, - False otherwise. - - This is a convenience method to make working with "is this a match?" cases - easier, avoiding unnecessarily indented try...except blocks. - """ - try: - urlresolvers.resolve(path, urlconf) - return True - except urlresolvers.Resolver404: - return False - diff --git a/django/middleware/locale.py b/django/middleware/locale.py index 2a71f3d150..77113e2987 100644 --- a/django/middleware/locale.py +++ b/django/middleware/locale.py @@ -1,10 +1,13 @@ "This is the locale selecting middleware that will look at accept headers" -from django.core.urlresolvers import get_resolver, LocaleRegexURLResolver +from django.conf import settings +from django.core.urlresolvers import (is_valid_path, get_resolver, + LocaleRegexURLResolver) from django.http import HttpResponseRedirect from django.utils.cache import patch_vary_headers from django.utils import translation + class LocaleMiddleware(object): """ This is a very simple middleware that parses a request @@ -23,13 +26,17 @@ class LocaleMiddleware(object): def process_response(self, request, response): language = translation.get_language() - translation.deactivate() - if (response.status_code == 404 and not translation.get_language_from_path(request.path_info) and self.is_language_prefix_patterns_used()): - return HttpResponseRedirect( - '/%s%s' % (language, request.get_full_path())) + urlconf = getattr(request, 'urlconf', None) + language_path = '/%s%s' % (language, request.path_info) + if settings.APPEND_SLASH and not language_path.endswith('/'): + language_path = language_path + '/' + if is_valid_path(language_path, urlconf): + return HttpResponseRedirect( + '/%s%s' % (language, request.get_full_path())) + translation.deactivate() patch_vary_headers(response, ('Accept-Language',)) if 'Content-Language' not in response: diff --git a/tests/regressiontests/i18n/patterns/tests.py b/tests/regressiontests/i18n/patterns/tests.py index 1cc4520358..1216d0bde9 100644 --- a/tests/regressiontests/i18n/patterns/tests.py +++ b/tests/regressiontests/i18n/patterns/tests.py @@ -144,37 +144,29 @@ class URLRedirectTests(URLTestCaseBase): def test_en_redirect(self): response = self.client.get('/account/register/', HTTP_ACCEPT_LANGUAGE='en') - self.assertRedirects(response, 'http://testserver/en/account/register/') + self.assertRedirects(response, '/en/account/register/') response = self.client.get(response['location']) self.assertEqual(response.status_code, 200) def test_en_redirect_wrong_url(self): response = self.client.get('/profiel/registeren/', HTTP_ACCEPT_LANGUAGE='en') - self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/en/profiel/registeren/') - - response = self.client.get(response['location']) self.assertEqual(response.status_code, 404) def test_nl_redirect(self): response = self.client.get('/profiel/registeren/', HTTP_ACCEPT_LANGUAGE='nl') - self.assertRedirects(response, 'http://testserver/nl/profiel/registeren/') + self.assertRedirects(response, '/nl/profiel/registeren/') response = self.client.get(response['location']) self.assertEqual(response.status_code, 200) def test_nl_redirect_wrong_url(self): response = self.client.get('/account/register/', HTTP_ACCEPT_LANGUAGE='nl') - self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/nl/account/register/') - - response = self.client.get(response['location']) self.assertEqual(response.status_code, 404) def test_pt_br_redirect(self): response = self.client.get('/conta/registre-se/', HTTP_ACCEPT_LANGUAGE='pt-br') - self.assertRedirects(response, 'http://testserver/pt-br/conta/registre-se/') + self.assertRedirects(response, '/pt-br/conta/registre-se/') response = self.client.get(response['location']) self.assertEqual(response.status_code, 200) @@ -187,17 +179,15 @@ class URLRedirectWithoutTrailingSlashTests(URLTestCaseBase): """ def test_not_prefixed_redirect(self): response = self.client.get('/not-prefixed', HTTP_ACCEPT_LANGUAGE='en') - self.assertEqual(response.status_code, 301) - self.assertEqual(response['location'], 'http://testserver/not-prefixed/') + self.assertRedirects(response, '/not-prefixed/', 301) def test_en_redirect(self): response = self.client.get('/account/register', HTTP_ACCEPT_LANGUAGE='en') - self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/en/account/register') + # target status code of 301 because of CommonMiddleware redirecting + self.assertRedirects(response, '/en/account/register', 302, target_status_code=301) response = self.client.get(response['location']) - self.assertEqual(response.status_code, 301) - self.assertEqual(response['location'], 'http://testserver/en/account/register/') + self.assertRedirects(response, '/en/account/register/', 301) class URLRedirectWithoutTrailingSlashSettingTests(URLTestCaseBase): @@ -208,20 +198,15 @@ class URLRedirectWithoutTrailingSlashSettingTests(URLTestCaseBase): @override_settings(APPEND_SLASH=False) def test_not_prefixed_redirect(self): response = self.client.get('/not-prefixed', HTTP_ACCEPT_LANGUAGE='en') - self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/en/not-prefixed') - - response = self.client.get(response['location']) self.assertEqual(response.status_code, 404) @override_settings(APPEND_SLASH=False) def test_en_redirect(self): - response = self.client.get('/account/register', HTTP_ACCEPT_LANGUAGE='en') - self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/en/account/register') + response = self.client.get('/account/register-without-slash', HTTP_ACCEPT_LANGUAGE='en') + self.assertRedirects(response, '/en/account/register-without-slash', 302) response = self.client.get(response['location']) - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 200) class URLResponseTests(URLTestCaseBase): diff --git a/tests/regressiontests/i18n/patterns/urls/namespace.py b/tests/regressiontests/i18n/patterns/urls/namespace.py index bf43949961..3a2467b7b5 100644 --- a/tests/regressiontests/i18n/patterns/urls/namespace.py +++ b/tests/regressiontests/i18n/patterns/urls/namespace.py @@ -7,4 +7,5 @@ view = TemplateView.as_view(template_name='dummy.html') urlpatterns = patterns('', url(_(r'^register/$'), view, name='register'), + url(_(r'^register-without-slash$'), view, name='register-without-slash'), )