Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes #17866: Vary: Accept-Language header when language prefix used #796

Merged
merged 1 commit into from

2 participants

@ambv

Tests pass on 2.7 and 3.3.

Remarks:

  • we cannot restrict including "Accept-Language" to only the case when it was used in page rendering. The language might be taken from the session or a client cookie. In that case the user gets that specific language but it doesn't mean the server doesn't vary content depending on Accept-Language (e.g. if the session would be empty and the cookie not set). Such a thing would break caching proxies. Taking that into account, I leave out "Vary: Accept-Language" on i18n_patterns URLs only.
  • the patch reuses supported languages across requests to speed up computation, trans_null had to learn the supportedargument toget_language_from_path()(already intrans_real`).
  • now LocaleMiddleware.is_language_prefix_pattern_used() would be computed in every response so I moved this computation to __init__ since it's not going to change between restarts
  • on a similar note, django.utils.translation.trans_real.check_for_language is needlessly computed for every get_language_from_path(). It won't change between restarts of the application, shouldn't we memoize it?
@HonzaKral HonzaKral merged commit c2a0451 into django:master
@ambv ambv deleted the ambv:vary branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
26 django/middleware/locale.py
@@ -17,6 +17,14 @@ class LocaleMiddleware(object):
is available, of course).
"""
+ def __init__(self):
+ self._supported_languages = dict(settings.LANGUAGES)
+ self._is_language_prefix_patterns_used = False
+ for url_pattern in get_resolver(None).url_patterns:
+ if isinstance(url_pattern, LocaleRegexURLResolver):
+ self._is_language_prefix_patterns_used = True
+ break
+
def process_request(self, request):
check_path = self.is_language_prefix_patterns_used()
language = translation.get_language_from_request(
@@ -26,9 +34,11 @@ def process_request(self, request):
def process_response(self, request, response):
language = translation.get_language()
- if (response.status_code == 404 and
- not translation.get_language_from_path(request.path_info)
- and self.is_language_prefix_patterns_used()):
+ language_from_path = translation.get_language_from_path(
+ request.path_info, supported=self._supported_languages
+ )
+ if (response.status_code == 404 and not language_from_path
+ and self.is_language_prefix_patterns_used()):
urlconf = getattr(request, 'urlconf', None)
language_path = '/%s%s' % (language, request.path_info)
path_valid = is_valid_path(language_path, urlconf)
@@ -42,8 +52,9 @@ def process_response(self, request, response):
request.get_host(), language, request.get_full_path())
return HttpResponseRedirect(language_url)
translation.deactivate()
-
- patch_vary_headers(response, ('Accept-Language',))
+ if not (self.is_language_prefix_patterns_used()
+ and language_from_path):
+ patch_vary_headers(response, ('Accept-Language',))
if 'Content-Language' not in response:
response['Content-Language'] = language
return response
@@ -53,7 +64,4 @@ def is_language_prefix_patterns_used(self):
Returns `True` if the `LocaleRegexURLResolver` is used
at root level of the urlpatterns, else it returns `False`.
"""
- for url_pattern in get_resolver(None).url_patterns:
- if isinstance(url_pattern, LocaleRegexURLResolver):
- return True
- return False
+ return self._is_language_prefix_patterns_used
View
4 django/utils/translation/__init__.py
@@ -165,8 +165,8 @@ def to_locale(language):
def get_language_from_request(request, check_path=False):
return _trans.get_language_from_request(request, check_path)
-def get_language_from_path(path):
- return _trans.get_language_from_path(path)
+def get_language_from_path(path, supported=None):
+ return _trans.get_language_from_path(path, supported=supported)
def templatize(src, origin=None):
return _trans.templatize(src, origin)
View
2  django/utils/translation/trans_null.py
@@ -58,6 +58,6 @@ def to_locale(language):
def get_language_from_request(request, check_path=False):
return settings.LANGUAGE_CODE
-def get_language_from_path(request):
+def get_language_from_path(request, supported=None):
return None
View
20 tests/regressiontests/i18n/patterns/tests.py
@@ -172,6 +172,26 @@ def test_pt_br_redirect(self):
self.assertEqual(response.status_code, 200)
+class URLVaryAcceptLanguageTests(URLTestCaseBase):
+ """
+ Tests that 'Accept-Language' is not added to the Vary header when using
+ prefixed URLs.
+ """
+ def test_no_prefix_response(self):
+ response = self.client.get('/not-prefixed/')
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.get('Vary'), 'Accept-Language')
+
+ def test_en_redirect(self):
+ response = self.client.get('/account/register/', HTTP_ACCEPT_LANGUAGE='en')
+ self.assertRedirects(response, '/en/account/register/')
+ self.assertFalse(response.get('Vary'))
+
+ response = self.client.get(response['location'])
+ self.assertEqual(response.status_code, 200)
+ self.assertFalse(response.get('Vary'))
+
+
class URLRedirectWithoutTrailingSlashTests(URLTestCaseBase):
"""
Tests the redirect when the requested URL doesn't end with a slash
View
3  tests/regressiontests/i18n/tests.py
@@ -48,7 +48,8 @@
from .patterns.tests import (URLRedirectWithoutTrailingSlashTests,
URLTranslationTests, URLDisabledTests, URLTagTests, URLTestCaseBase,
URLRedirectWithoutTrailingSlashSettingTests, URLNamespaceTests,
- URLPrefixTests, URLResponseTests, URLRedirectTests, PathUnusedTests)
+ URLPrefixTests, URLResponseTests, URLRedirectTests, PathUnusedTests,
+ URLVaryAcceptLanguageTests)
here = os.path.dirname(os.path.abspath(upath(__file__)))
Something went wrong with that request. Please try again.