-
Notifications
You must be signed in to change notification settings - Fork 50
FEATURE: Localization fallbacks (server-side) #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: localization-system-pre
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -155,6 +155,8 @@ def set_locale | |
| else | ||
| SiteSetting.default_locale | ||
| end | ||
|
|
||
| I18n.fallbacks.ensure_loaded! | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — corrupt fallback YAML → 500 on every request, no circuit breaker.
# Wrap the preload so a corrupt file degrades gracefully:
I18n.fallbacks.ensure_loaded! rescue nil
# or add a dedicated rescue_from for I18n::InvalidLocaleData / Psych::SyntaxErrorThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — fallbacks only wired into the web request path; jobs /
# Option A — preload in jobs too (app/jobs/base.rb after I18n.locale = ...):
I18n.fallbacks.ensure_loaded!
# Option B — have translate load the whole chain, not just the primary:
# translate_accelerator.rb translate():
I18n.fallbacks[config.locale].each { |l| load_locale(l) unless @loaded_locales.include?(l) }There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — fallbacks are wired into the web request path only.
|
||
| end | ||
|
|
||
| def store_preloaded(key, json) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # order: after 02-freedom_patches.rb | ||
|
|
||
| # Include pluralization module | ||
| require 'i18n/backend/pluralization' | ||
| I18n::Backend::Simple.send(:include, I18n::Backend::Pluralization) | ||
|
|
||
| # Include fallbacks module | ||
| require 'i18n/backend/fallbacks' | ||
| I18n.backend.class.send(:include, I18n::Backend::Fallbacks) | ||
|
|
||
| # Configure custom fallback order | ||
| class FallbackLocaleList < Hash | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — Pre-PR, class FallbackLocaleList < I18n::Locale::Fallbacks
def [](locale)
[locale.to_sym, SiteSetting.default_locale.to_sym, :en].uniq.compact
end
def ensure_loaded!; self[I18n.locale].each { |l| I18n.ensure_loaded! l }; end
end |
||
| def [](locale) | ||
| # user locale, site locale, english | ||
| # TODO - this can be extended to be per-language for a better user experience | ||
| # (e.g. fallback zh_TW to zh_CN / vice versa) | ||
| [locale, SiteSetting.default_locale.to_sym, :en].uniq.compact | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking — string/symbol mismatch defeats the
Downstream, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking — multisite cache poisoning: cross-site translation leakage. The fallback chain is now per-site ( Result: site A (default Pre-PR this was safe because # translate_accelerator.rb — include the site discriminator:
k = "#{key}#{config.locale}#{config.backend.object_id}#{SiteSetting.default_locale}"Or make |
||
| end | ||
|
|
||
| def ensure_loaded! | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — no test coverage for the new fallback feature.
Note: |
||
| self[I18n.locale].each { |l| I18n.ensure_loaded! l } | ||
| end | ||
| end | ||
| I18n.fallbacks = FallbackLocaleList.new | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — In i18n 0.7.0 the default # Subclass and override only [] to preserve the extension API:
class FallbackLocaleList < I18n::Locale::Fallbacks
def [](locale)
[locale.to_sym, SiteSetting.default_locale.to_sym, :en].uniq.compact
end
# ...ensure_loaded!...
endThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — no test coverage for the new fallback feature.
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,11 @@ def load_locale(locale) | |
| end | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking — a missing/misnamed fallback locale file is silently marked loaded with zero translations, no observability. Inside |
||
| end | ||
|
|
||
| def ensure_loaded!(locale) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — missing locale file silently marked loaded with zero translations.
# In load_locale, log when a locale resolves to no files:
files = I18n.load_path.grep(Regexp.new("\\.#{locale}\\.yml$"))
Rails.logger.warn("I18n: no translation files found for locale #{locale}") if files.empty?
I18n.backend.load_translations(files)There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Non-blocking — no circuit breaker: a corrupt fallback YAML 500s every request.
|
||
| @loaded_locales ||= [] | ||
| load_locale locale unless @loaded_locales.include?(locale) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking — fallbacks silently no-op (and can poison the shared cache) outside the HTTP request path. This new Fix: eager-load the fallback chain wherever (Note: fallbacks are not bypassed by the |
||
| end | ||
|
|
||
| def translate(key, *args) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking — multisite cache poisoning: cross-site translation leakage.
# translate_accelerator.rb — include the site discriminator in the key:
k = "#{key}#{config.locale}#{config.backend.object_id}#{SiteSetting.default_locale}"(Verified against i18n 0.7.0: |
||
| load_locale(config.locale) unless @loaded_locales.include?(config.locale) | ||
| return translate_no_cache(key, *args) if args.length > 0 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking — cache key omits the per-site fallback target, causing cross-site translation leakage on multisite. A few lines below this, the LRU key (line 72) is Concrete failure: Site A ( Fix: include the resolved fallback list (or |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking —
ensure_loaded!propagates YAML load errors with no graceful handling, turning a single corrupt fallback file into a 500 on every request.FallbackLocaleList#ensure_loaded!→I18n.ensure_loaded!→load_locale→I18n.backend.load_translations→load_file/load_yml, which re-raises malformed YAML asI18n::InvalidLocaleData(i18n 0.7.0 base.rb). None ofload_locale,I18n.ensure_loaded!, orFallbackLocaleList#ensure_loaded!rescue it, andApplicationControllerdeclares norescue_fromforI18n::InvalidLocaleData/Psych::SyntaxError. Because the fallback chain always includes:enandSiteSetting.default_locale, a single corrupt always-present fallback YAML (e.g.en.yml) takes down the entire site on every request — a broader blast radius than the previous lazy single-locale loading, which only loadedconfig.localeand only for the user's own locale. Consider skipping a broken fallback tier and continuing with the next (degrading gracefully) rather than failing the whole request, plus a log line.