diff --git a/src/cockpit/packages.py b/src/cockpit/packages.py index 28fffcd853ac..0d648db3fe13 100644 --- a/src/cockpit/packages.py +++ b/src/cockpit/packages.py @@ -62,28 +62,55 @@ logger = logging.getLogger(__name__) -def parse_accept_language(headers: JsonObject) -> List[str]: +# In practice, this is going to get called over and over again with exactly the +# same list. Let's try to cache the result. +@functools.lru_cache() +def parse_accept_language(accept_language: str) -> Sequence[str]: """Parse the Accept-Language header, if it exists. - Returns an ordered list of languages. + Returns an ordered list of languages, with fallbacks inserted, and + truncated to the position where 'en' would have otherwise appeared, if + applicable. https://tools.ietf.org/html/rfc7231#section-5.3.5 + https://datatracker.ietf.org/doc/html/rfc4647#section-3.4 """ - locales = [] - for language in get_str(headers, 'Accept-Language', '').split(','): - language = language.strip() - locale, _, weightstr = language.partition(';q=') - weight = float(weightstr or 1) - - # Skip possible empty locales - if not locale: - continue - - # Locales are case-insensitive and we store our list in lowercase - locales.append((locale.lower(), weight)) - - return [locale for locale, _ in sorted(locales, key=lambda k: k[1], reverse=True)] + logger.debug('parse_accept_language(%r)', accept_language) + locales_with_q = [] + for entry in accept_language.split(','): + entry = entry.strip().lower() + logger.debug(' entry %r', entry) + locale, _, qstr = entry.partition(';q=') + try: + q = float(qstr or 1.0) + except ValueError: + continue # ignore malformed entry + + while locale: + logger.debug(' adding %r q=%r', locale, q) + locales_with_q.append((locale, q)) + # strip off '-detail' suffixes until there's nothing left + locale, _, _region = locale.rpartition('-') + + # Sort the list by highest q value. Otherwise, this is a stable sort. + locales_with_q.sort(key=lambda pair: pair[1], reverse=True) + logger.debug(' sorted list is %r', locales_with_q) + + # If we have 'en' anywhere in our list, ignore it and all items after it. + # This will result in us getting an untranslated (ie: English) version if + # none of the more-preferred languages are found, which is what we want. + # We also take the chance to drop duplicate items. Note: both of these + # things need to happen after sorting. + results = [] + for locale, _q in locales_with_q: + if locale == 'en': + break + if locale not in results: + results.append(locale) + + logger.debug(' results list is %r', results) + return tuple(results) def sortify_version(version: str) -> str: @@ -280,22 +307,15 @@ def load_file(self, filename: str) -> Document: return Document(path.open('rb'), content_type, content_encoding, content_security_policy) - def load_translation(self, path: str, locales: List[str]) -> Document: + def load_translation(self, path: str, locales: Sequence[str]) -> Document: self.ensure_scanned() assert self.translations is not None - # First check the locales that the user sent + # First match wins for locale in locales: with contextlib.suppress(KeyError): return self.load_file(self.translations[path][locale]) - # Next, check the language-only versions of variant-specified locales - for locale in locales: - language, _, region = locale.partition('-') - if region: - with contextlib.suppress(KeyError): - return self.load_file(self.translations[path][language]) - # We prefer to return an empty document than 404 in order to avoid # errors in the console when a translation can't be found return Document(io.BytesIO(), 'text/javascript') @@ -306,7 +326,7 @@ def load_path(self, path: str, headers: JsonObject) -> Document: assert self.translations is not None if path in self.translations: - locales = parse_accept_language(headers) + locales = parse_accept_language(get_str(headers, 'Accept-Language', '')) return self.load_translation(path, locales) else: return self.load_file(self.files[path]) @@ -500,7 +520,7 @@ def load_manifests_js(self, headers: JsonObject) -> Document: chunks: List[bytes] = [] # Send the translations required for the manifest files, from each package - locales = parse_accept_language(headers) + locales = parse_accept_language(get_str(headers, 'Accept-Language', '')) for name, package in self.packages.items(): if name in ['static', 'base1']: continue diff --git a/test/pytest/test_packages.py b/test/pytest/test_packages.py index 8f3a8f1cd64d..e6d7c75c7f65 100644 --- a/test/pytest/test_packages.py +++ b/test/pytest/test_packages.py @@ -16,7 +16,6 @@ # along with this program. If not, see . import json -from typing import List import pytest @@ -24,12 +23,26 @@ @pytest.mark.parametrize(("test_input", "expected"), [ - ('de-at, zh-CH, en,', ['de-at', 'zh-ch', 'en']), - ('es-es, nl;q=0.8, fr;q=0.9', ['es-es', 'fr', 'nl']), - ('fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5', ['fr-ch', 'fr', 'en', 'de', '*']) + # correct handles empty values + ('', ()), + (' ', ()), + (' , ', ()), + (' , ,xx', ('xx',)), + # english → empty list + ('en', ()), + (' , en', ()), + # invalid q values get ignored + ('aa;q===,bb;q=abc,cc;q=.,zz', ('zz',)), + # variant-peeling works + ('aa-bb-cc-dd,ee-ff-gg-hh', ('aa-bb-cc-dd', 'aa-bb-cc', 'aa-bb', 'aa', 'ee-ff-gg-hh', 'ee-ff-gg', 'ee-ff', 'ee')), + # sorting and english-truncation are working + ('fr-ch;q=0.8,es-mx;q=1.0,en-ca;q=0.9', ('es-mx', 'es', 'en-ca')), + ('de-at, zh-CN, en,', ('de-at', 'de', 'zh-cn', 'zh')), + ('es-es, nl;q=0.8, fr;q=0.9', ('es-es', 'es', 'fr', 'nl')), + ('fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5', ('fr-ch', 'fr')) ]) -def test_parse_accept_language(test_input: str, expected: List[str]) -> None: - assert parse_accept_language({'Accept-Language': test_input}) == expected +def test_parse_accept_language(test_input: str, expected: 'tuple[str]') -> None: + assert parse_accept_language(test_input) == expected @pytest.fixture @@ -160,6 +173,44 @@ def test_condition_hides_priority(pkgdir): assert packages.packages['basic'].priority == 1 +def test_english_translation(pkgdir): + make_package(pkgdir, 'one') + (pkgdir / 'one' / 'po.de.js').write_text('eins') + + packages = Packages() + + # make sure we get German + document = packages.load_path('/one/po.js', {'Accept-Language': 'de'}) + assert '/javascript' in document.content_type + assert document.data.read() == b'eins' + + # make sure we get German here (higher q-value) even with English first + document = packages.load_path('/one/po.js', {'Accept-Language': 'en;q=0.9, de-ch'}) + assert '/javascript' in document.content_type + assert document.data.read() == b'eins' + + # make sure we get the empty ("English") translation, and not German + document = packages.load_path('/one/po.js', {'Accept-Language': 'en, de'}) + assert '/javascript' in document.content_type + assert document.data.read() == b'' + + document = packages.load_path('/one/po.js', {'Accept-Language': 'de;q=0.9, fr;q=0.7, en'}) + assert '/javascript' in document.content_type + assert document.data.read() == b'' + + document = packages.load_path('/one/po.js', {'Accept-Language': 'de;q=0.9, fr, en-ca'}) + assert '/javascript' in document.content_type + assert document.data.read() == b'' + + document = packages.load_path('/one/po.js', {'Accept-Language': ''}) + assert '/javascript' in document.content_type + assert document.data.read() == b'' + + document = packages.load_path('/one/po.js', {}) + assert '/javascript' in document.content_type + assert document.data.read() == b'' + + def test_translation(pkgdir): # old style: make sure po.de.js is served as fallback for manifest translations make_package(pkgdir, 'one')