diff --git a/superset-frontend/src/preamble.test.ts b/superset-frontend/src/preamble.test.ts index 579b0c97f71f..dfbe1e7fd6b1 100644 --- a/superset-frontend/src/preamble.test.ts +++ b/superset-frontend/src/preamble.test.ts @@ -17,6 +17,8 @@ * under the License. */ +import type { LanguagePack } from '@apache-superset/core/translation'; + const mockConfigure = jest.fn(); const mockInitFeatureFlags = jest.fn(); const mockMakeApi = jest.fn(() => jest.fn()); @@ -111,3 +113,83 @@ test('falls back to en when passing locale to setupFormatters', async () => { expect(mockSetupFormatters).toHaveBeenCalledWith({}, {}, 'en'); }); + +// --- language pack loading semantics (issues #35330, PR #41780) ----------- +// English: nothing is loaded at all. Non-English: the pack is expected to +// already be on window (set by the versioned classic + {% endif %} + {# + Operators can still supply a pack directly in the bootstrap payload + via COMMON_BOOTSTRAP_OVERRIDES_FUNC (the historical #35330 + workaround). When one is present, stash it on window here instead of + emitting the versioned script tag, so early chunks see the override. + #} + {% if language_pack_inline %} + + })(); + + {% endif %} {% if entry %} {{ js_bundle(assets_prefix, entry) }} {% endif %} diff --git a/superset/translations/utils.py b/superset/translations/utils.py index 36be0331a4d7..79bbe872604f 100644 --- a/superset/translations/utils.py +++ b/superset/translations/utils.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import hashlib import json import logging import os @@ -24,9 +25,41 @@ # Global caching for JSON language packs ALL_LANGUAGE_PACKS: dict[str, dict[str, Any]] = {"en": {}} +# Global caching for language pack content hashes, used to build +# content-addressed (cache-busting) asset URLs +ALL_LANGUAGE_PACK_VERSIONS: dict[str, str] = {} + DIR = os.path.dirname(os.path.abspath(__file__)) +def get_language_pack_filename(locale: str) -> str: + """Resolve the on-disk JSON pack for a locale (empty pack for English)""" + if not locale or locale == "en": + # Forcing a dummy, quasi-empty language pack for English since the file + # in the en directory contains data with empty mappings + return DIR + "/empty_language_pack.json" + return DIR + f"/{locale}/LC_MESSAGES/messages.json" + + +def get_language_pack_version(locale: str) -> Optional[str]: + """Get/cache a short content hash of the language pack file + + The hash is embedded in the pack's asset URL so browsers can cache it + as immutable and pick up a fresh copy whenever translations change. + Returns None when the pack file cannot be read. + """ + version = ALL_LANGUAGE_PACK_VERSIONS.get(locale) + if not version: + try: + with open(get_language_pack_filename(locale), "rb") as f: + version = hashlib.sha256(f.read()).hexdigest()[:12] + ALL_LANGUAGE_PACK_VERSIONS[locale] = version + except OSError: + logger.warning("No language pack file to version for locale %s", locale) + return None + return version + + def get_language_pack(locale: str) -> Optional[dict[str, Any]]: """Get/cache a language pack @@ -37,11 +70,7 @@ def get_language_pack(locale: str) -> Optional[dict[str, Any]]: """ pack = ALL_LANGUAGE_PACKS.get(locale) if not pack: - filename = DIR + f"/{locale}/LC_MESSAGES/messages.json" - if not locale or locale == "en": - # Forcing a dummy, quasy-empty language pack for English since the file - # in the en directory is contains data with empty mappings - filename = DIR + "/empty_language_pack.json" + filename = get_language_pack_filename(locale) try: with open(filename, encoding="utf8") as f: pack = json.load(f) diff --git a/superset/views/base.py b/superset/views/base.py index 7bd76ef58242..a2dd17789682 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -65,7 +65,7 @@ from superset.themes.utils import ( is_valid_theme, ) -from superset.translations.utils import get_language_pack +from superset.translations.utils import get_language_pack_version from superset.utils import core as utils, json from superset.utils.filters import get_dataset_access_filters from superset.utils.version import get_version_metadata, visible_version_metadata @@ -578,25 +578,50 @@ def common_bootstrap_payload() -> dict[str, Any]: # Convert locale to string for proper cache key hashing locale_str = str(locale) if locale else None payload = dict(cached_common_bootstrap_data(utils.get_user_id(), locale_str)) - # Inject the Jed language pack outside the per-user memoize so the cached - # payload stays small and the pack is shared across users for the same - # locale. The frontend uses it to configure the translator synchronously, - # before any code-split chunk evaluates a module-level `const X = t('...')` - # (upstream issue #35330). - language = payload.get("locale") - if language and language != "en": - # Respect a pack already provided via COMMON_BOOTSTRAP_OVERRIDES_FUNC - # (the workaround in #35330 does exactly that), otherwise load the - # shared one. `get_language_pack` returns the empty English pack on a - # miss, which is the right result (English) when no translation file - # exists. - pack = payload.get("language_pack") or get_language_pack(language) - else: - pack = None - payload["language_pack"] = pack + # The language pack itself is NOT embedded in the payload: spa.html loads + # it through the content-addressed /language_pack///script.js + # tag before the entry bundle, keeping HTML small while still configuring + # the translator synchronously (upstream issue #35330). A pack provided via + # COMMON_BOOTSTRAP_OVERRIDES_FUNC (the historical workaround) is respected + # and takes precedence over the script tag. + payload.setdefault("language_pack", None) return payload +def get_language_pack_template_context(common: dict[str, Any]) -> dict[str, Any]: + """Template vars controlling how spa.html delivers the language pack. + + ``common`` is the already-built common bootstrap payload for the request + (passed in rather than re-derived, so callers that assembled or mocked + their own payload stay consistent with what the template sees). + + Three mutually exclusive outcomes: + - English (or no locale): no script tag, nothing to stash. + - Operator supplied a pack via COMMON_BOOTSTRAP_OVERRIDES_FUNC: no script + tag; spa.html stashes the bootstrap pack on window instead. + - Otherwise: emit the content-addressed script URL so the browser can + cache the pack as immutable and cache-bust on translation changes. + """ + language = common.get("locale") + if not language or language == "en": + return {"language_pack_src": None, "language_pack_inline": False} + if common.get("language_pack"): + return {"language_pack_src": None, "language_pack_inline": True} + version = get_language_pack_version(language) + return { + "language_pack_src": ( + url_for( + "Superset.language_pack_script", + lang=language, + version=version, + ) + if version + else None + ), + "language_pack_inline": False, + } + + def get_spa_payload(extra_data: dict[str, Any] | None = None) -> dict[str, Any]: """Generate standardized payload for spa.html template rendering. @@ -695,6 +720,7 @@ def get_spa_template_context( "dark_theme_bg": dark_theme_bg, "spinner_svg": spinner_svg, "default_title": default_title, + **get_language_pack_template_context(payload.get("common") or {}), **template_kwargs, } diff --git a/superset/views/core.py b/superset/views/core.py index 75bf6d3e48c6..e9f18d3a7f50 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -82,6 +82,7 @@ FlaskResponse, ) from superset.tasks.utils import get_current_user +from superset.translations.utils import get_language_pack, get_language_pack_version from superset.utils import core as utils, json from superset.utils.cache import etag_cache from superset.utils.core import ( @@ -917,6 +918,48 @@ def language_pack(self, lang: str) -> FlaskResponse: "Language pack doesn't exist on the server", status=404 ) + @expose("/language_pack///script.js") + def language_pack_script(self, lang: str, version: str) -> FlaskResponse: + """Serve the language pack as a content-addressed classic script. + + spa.html loads this BEFORE the entry bundle so translations are + configured synchronously (no race with code-split chunks), while the + versioned URL lets browsers cache the pack as immutable and pick up a + fresh copy whenever translations change. + + Deliberately unauthenticated: translation catalogs are static, public + content shipped in the Superset repo, contain no user or tenant data, + and must load for anonymous principals (login page, embedded). + """ + # Only allow expected language formats like "en", "pt_BR", etc. + if not re.match(r"^[a-z]{2,3}(_[A-Z]{2})?$", lang): + abort(400, "Invalid language code") + if not re.match(r"^[0-9a-f]{12}$", version): + abort(400, "Invalid language pack version") + + current_version = get_language_pack_version(lang) + pack = get_language_pack(lang) + if current_version is None or pack is None: + return json_error_response( + "Language pack doesn't exist on the server", status=404 + ) + + response = Response( + f"window.__SUPERSET_LANGUAGE_PACK__ = {json.dumps(pack)};", + mimetype="application/javascript; charset=utf-8", + ) + if version == current_version: + # Content-addressed URL: safe to cache forever. + response.cache_control.public = True + response.cache_control.max_age = 31536000 + response.cache_control.immutable = True + else: + # Stale or unknown version (e.g. HTML rendered before an upgrade): + # serve the current pack but keep caches from pinning it under the + # wrong address. + response.cache_control.no_cache = True + return response + @event_logger.log_this @expose("/welcome/") def welcome(self) -> FlaskResponse: diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index c089bf4cf25e..66c8fe5fe5e2 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1668,6 +1668,10 @@ def test_views_are_secured(self): # Serves the PWA web app manifest unauthenticated (PWA install # fetches have no session); mirrors the RedirectView precedent. ["PwaManifestView", "manifest"], + # Serves translation catalogs (static public repo content, no + # user/tenant data) as content-addressed scripts; must load for + # anonymous principals (login page, embedded dashboards). + ["Superset", "language_pack_script"], ] unsecured_views = [] for view_class in appbuilder.baseviews: diff --git a/tests/unit_tests/translations/__init__.py b/tests/unit_tests/translations/__init__.py new file mode 100644 index 000000000000..13a83393a912 --- /dev/null +++ b/tests/unit_tests/translations/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/tests/unit_tests/translations/utils_test.py b/tests/unit_tests/translations/utils_test.py new file mode 100644 index 000000000000..66007d01aba8 --- /dev/null +++ b/tests/unit_tests/translations/utils_test.py @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import hashlib +from collections.abc import Iterator + +import pytest + +from superset.translations import utils as translations_utils +from superset.translations.utils import ( + get_language_pack_filename, + get_language_pack_version, +) + + +@pytest.fixture(autouse=True) +def _clear_version_cache() -> Iterator[None]: + translations_utils.ALL_LANGUAGE_PACK_VERSIONS.clear() + yield + translations_utils.ALL_LANGUAGE_PACK_VERSIONS.clear() + + +def test_language_pack_filename_resolution() -> None: + assert get_language_pack_filename("fr").endswith("/fr/LC_MESSAGES/messages.json") + assert get_language_pack_filename("en").endswith("/empty_language_pack.json") + assert get_language_pack_filename("").endswith("/empty_language_pack.json") + + +def test_version_is_short_content_hash(tmp_path, monkeypatch) -> None: + pack_file = tmp_path / "fr" / "LC_MESSAGES" / "messages.json" + pack_file.parent.mkdir(parents=True) + pack_file.write_bytes(b'{"domain": "superset"}') + monkeypatch.setattr(translations_utils, "DIR", str(tmp_path)) + + version = get_language_pack_version("fr") + + expected = hashlib.sha256(b'{"domain": "superset"}').hexdigest()[:12] + assert version == expected + + +def test_version_is_cached_and_changes_with_content(tmp_path, monkeypatch) -> None: + pack_file = tmp_path / "fr" / "LC_MESSAGES" / "messages.json" + pack_file.parent.mkdir(parents=True) + pack_file.write_bytes(b"{}") + monkeypatch.setattr(translations_utils, "DIR", str(tmp_path)) + + first = get_language_pack_version("fr") + pack_file.write_bytes(b'{"changed": true}') + # Cached within the process lifetime: same version until cache cleared. + assert get_language_pack_version("fr") == first + + translations_utils.ALL_LANGUAGE_PACK_VERSIONS.clear() + assert get_language_pack_version("fr") != first + + +def test_version_none_when_pack_missing(tmp_path, monkeypatch) -> None: + monkeypatch.setattr(translations_utils, "DIR", str(tmp_path)) + assert get_language_pack_version("xx") is None diff --git a/tests/unit_tests/views/test_bootstrap_auth.py b/tests/unit_tests/views/test_bootstrap_auth.py index a4f2ffe16005..f87fc0f2d207 100644 --- a/tests/unit_tests/views/test_bootstrap_auth.py +++ b/tests/unit_tests/views/test_bootstrap_auth.py @@ -135,71 +135,104 @@ def test_recaptcha_shown_for_non_federated_auth( assert payload["conf"]["RECAPTCHA_PUBLIC_KEY"] == "test-key" -# --- language_pack injection -------------------------------------------- +# --- language pack delivery ---------------------------------------------- # -# The Jed pack is injected by `common_bootstrap_payload` (outside the -# memoized `cached_common_bootstrap_data`) using the shared -# `superset.translations.utils.get_language_pack`. Tests here cover the -# wrapper to confirm the pack lands on the payload for non-English -# locales and is None for English. +# The pack is NOT embedded in the bootstrap payload; spa.html loads it via a +# content-addressed script tag whose URL comes from +# `get_language_pack_template_context`. A pack supplied through +# COMMON_BOOTSTRAP_OVERRIDES_FUNC still rides the payload and suppresses the +# script tag. -def test_common_bootstrap_payload_includes_language_pack_for_non_english( +def test_common_bootstrap_payload_does_not_embed_language_pack( app_context: None, ) -> None: - """common.language_pack carries the shared utility's pack for non-en.""" - fake_pack = {"domain": "superset", "locale_data": {"superset": {}}} + """The payload stays small: no full pack even for non-English locales.""" with ( patch( "superset.views.base.cached_common_bootstrap_data", return_value={"locale": "fr"}, ), - patch( - "superset.views.base.get_language_pack", - return_value=fake_pack, - ) as mock_get, patch("superset.views.base.utils.get_user_id", return_value=1), patch("superset.views.base.get_locale", return_value="fr"), ): payload = common_bootstrap_payload() - assert payload["language_pack"] == fake_pack - mock_get.assert_called_once_with("fr") + assert payload["language_pack"] is None -def test_common_bootstrap_payload_skips_pack_for_english( +def test_common_bootstrap_payload_preserves_override_pack( app_context: None, ) -> None: - """English short-circuits: pack is None and the utility is not called.""" + """A pack from COMMON_BOOTSTRAP_OVERRIDES_FUNC is passed through.""" + fake_pack = {"domain": "superset", "locale_data": {"superset": {}}} with ( patch( "superset.views.base.cached_common_bootstrap_data", - return_value={"locale": "en"}, + return_value={"locale": "fr", "language_pack": fake_pack}, ), - patch("superset.views.base.get_language_pack") as mock_get, patch("superset.views.base.utils.get_user_id", return_value=1), - patch("superset.views.base.get_locale", return_value="en"), + patch("superset.views.base.get_locale", return_value="fr"), ): payload = common_bootstrap_payload() - assert payload["language_pack"] is None - mock_get.assert_not_called() + assert payload["language_pack"] == fake_pack def test_common_bootstrap_payload_does_not_mutate_memoized_dict( app_context: None, ) -> None: - """Injecting language_pack must not write back into the memoize cache.""" + """Defaulting language_pack must not write back into the memoize cache.""" cached: dict[str, Any] = {"locale": "fr"} with ( patch( "superset.views.base.cached_common_bootstrap_data", return_value=cached, ), - patch("superset.views.base.get_language_pack", return_value={"x": 1}), patch("superset.views.base.utils.get_user_id", return_value=1), patch("superset.views.base.get_locale", return_value="fr"), ): common_bootstrap_payload() assert "language_pack" not in cached + + +def _language_pack_context(locale: str, payload_extra: dict[str, Any]) -> Any: + from superset.views.base import get_language_pack_template_context + + with ( + patch( + "superset.views.base.get_language_pack_version", + return_value="abc123def456", + ), + patch( + "superset.views.base.url_for", + return_value="/language_pack/fr/abc123def456/script.js", + ), + ): + return get_language_pack_template_context({"locale": locale, **payload_extra}) + + +def test_language_pack_template_context_versioned_src_for_non_english( + app_context: None, +) -> None: + context = _language_pack_context("fr", {}) + assert context == { + "language_pack_src": "/language_pack/fr/abc123def456/script.js", + "language_pack_inline": False, + } + + +def test_language_pack_template_context_none_for_english( + app_context: None, +) -> None: + context = _language_pack_context("en", {}) + assert context == {"language_pack_src": None, "language_pack_inline": False} + + +def test_language_pack_template_context_inline_for_override_pack( + app_context: None, +) -> None: + """An operator-supplied pack suppresses the script tag; spa.html inlines.""" + context = _language_pack_context("fr", {"language_pack": {"domain": "superset"}}) + assert context == {"language_pack_src": None, "language_pack_inline": True} diff --git a/tests/unit_tests/views/test_language_pack_script.py b/tests/unit_tests/views/test_language_pack_script.py new file mode 100644 index 000000000000..797bd6e13d50 --- /dev/null +++ b/tests/unit_tests/views/test_language_pack_script.py @@ -0,0 +1,115 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from pathlib import Path +from typing import Any +from unittest.mock import patch + +FAKE_PACK = {"domain": "superset", "locale_data": {"superset": {"": {}}}} +FAKE_VERSION = "abc123def456" + + +def test_script_served_immutable_when_version_matches(client: Any) -> None: + with ( + patch( + "superset.views.core.get_language_pack_version", + return_value=FAKE_VERSION, + ), + patch("superset.views.core.get_language_pack", return_value=FAKE_PACK), + ): + response = client.get(f"/language_pack/fr/{FAKE_VERSION}/script.js") + + assert response.status_code == 200 + assert response.mimetype == "application/javascript" + body = response.get_data(as_text=True) + assert body.startswith("window.__SUPERSET_LANGUAGE_PACK__ = ") + assert '"domain": "superset"' in body + cache_control = response.headers["Cache-Control"] + assert "immutable" in cache_control + assert "max-age=31536000" in cache_control + assert "public" in cache_control + + +def test_script_not_cacheable_when_version_stale(client: Any) -> None: + """A pre-upgrade HTML page may reference an old version: serve fresh + content, but do not let caches pin it under the stale address.""" + with ( + patch( + "superset.views.core.get_language_pack_version", + return_value=FAKE_VERSION, + ), + patch("superset.views.core.get_language_pack", return_value=FAKE_PACK), + ): + response = client.get("/language_pack/fr/000000000000/script.js") + + assert response.status_code == 200 + assert "no-cache" in response.headers["Cache-Control"] + assert "immutable" not in response.headers["Cache-Control"] + + +def test_script_404_when_pack_missing(client: Any) -> None: + with patch( + "superset.views.core.get_language_pack_version", + return_value=None, + ): + response = client.get(f"/language_pack/xx/{FAKE_VERSION}/script.js") + + assert response.status_code == 404 + + +def test_script_rejects_malformed_lang_and_version(client: Any) -> None: + assert client.get(f"/language_pack/../{FAKE_VERSION}/script.js").status_code in ( + 400, + 404, + ) + assert client.get("/language_pack/fr/not-a-hash!/script.js").status_code == 400 + + +def test_script_serves_only_the_requested_locale(client: Any) -> None: + """The endpoint resolves exactly one pack: the locale in the URL.""" + with ( + patch( + "superset.views.core.get_language_pack_version", + return_value=FAKE_VERSION, + ) as mock_version, + patch( + "superset.views.core.get_language_pack", return_value=FAKE_PACK + ) as mock_pack, + ): + client.get(f"/language_pack/pt_BR/{FAKE_VERSION}/script.js") + + mock_version.assert_called_once_with("pt_BR") + mock_pack.assert_called_once_with("pt_BR") + + +def test_spa_template_loads_pack_before_entry_bundle() -> None: + """Static guard on spa.html: the language pack script tag must precede + the entry bundle and stay a classic script (no async/defer). A deferred + or reordered tag would let code-split chunks evaluate module-level + `t('...')` calls before the translator is configured (issue #35330).""" + import superset + + template = ( + Path(superset.__file__).parent / "templates" / "superset" / "spa.html" + ).read_text() + + tag_start = template.index('