[1.x] fix: resolve translation references in implicitly-loaded fallback catalogues - #5024
Conversation
…alogues
Backport of the 2.x fix to 1.x.
Flarum's Translator::getCatalogue() only ran `=>`-reference resolution
(parseCatalogue()) when the requested locale was not yet present in
$this->catalogues. But Symfony's Translator::loadFallbackCatalogues()
pre-stores the *original* fallback catalogue in $this->catalogues while
attaching only a fresh copy to the requesting locale's catalogue.
So loading e.g. 'de' first stored a raw, never-parsed 'en' original;
a later getCatalogue('en') found the locale already set, skipped
parsing, and returned it with unresolved references. Consumers that
snapshot the catalogue - most damagingly Frontend\AddTranslations,
which compiles the locale JS assets - then emitted literal
"=> core.admin.dashboard.title" strings, and the poisoned compiled
asset persisted until a cache flush (revisions gate recompiles).
Track parsed state per catalogue *object* (SplObjectStorage, since 1.x
must stay PHP 7.3-compatible) instead of per locale name, and walk the
fallback chain on every access. The raw stored original and its parsed
copy share a locale name, so a locale-keyed flag cannot distinguish
them; per-object tracking parses each catalogue object exactly once.
parseCatalogue() is unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntract Review follow-up. No behaviour change (src/ diff vs the fix commit is comments only). - Formatter: justify leaving storage/formatter/* by parity with core's own runtime refresh (Formatter::flush() and the Formatter extender only forget the cache entry) rather than by the production log line, which is an E_WARNING that also occurs on healthy parse-only requests. The renderer class is autoloaded DURING unserialize(), so the deletion window is microseconds. Document the residuals: same-hash rebuilds rewrite the live class file in place, and concurrent cache misses each rebuild it. - Views: core does delete storage/views/* on the acting pod for extensions that register views; the reason other pods can leave them is that compiled views are keyed by source path and expire by source mtime. - OPcache: drop the unsupported "classes fail during the swap" rationale; the targeted invalidation is belt-and-braces (Symfony re-invalidates on rewrite; no-op from the CLI). Note that the per-SAPI double apply now buys little. - DISTRIBUTED_CACHE.md: the manifest race is reduced (fewer writers), not eliminated, and core's single flush is itself a dozen sequential writes; attribute the racing-refill half of the residual to this extension's settings layer; state the real per-apply cost, including that Saved triggers a full apply core does not need; point operators at the core 1.x fallback-catalogue bug (flarum/framework#5024) that propagation cannot fix. README FAQ updated. - Test: always seed rev-manifest.json with a revision for the asset sentinel (a re-added flush is a no-op against an empty manifest, which is what a second run inherited), name the sentinel forum.js, assert flarum.formatter is forgotten, clean seeded files up in tearDown only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Adding context on why I think this qualifies under the 1.x critical-fixes-only policy, with what we've learned running it in production since the PR was opened. It's user-visible breakage that doesn't heal itself. Once the raw It's not a multi-instance edge case. The trigger is simply: a non-English catalogue gets compiled before the English one is touched, in the same request. That happens on a single-instance install whenever the site's default locale isn't There is no admin-level workaround. The only mitigation short of this patch is site-level PHP: a middleware that calls The fix is already accepted on 2.x (#5023, merged 2026-09-02). This backport is the same logic with Happy to adjust anything for 1.x conventions. If a 1.8.20 is on the table at all, I'd argue this belongs in it: deterministic, silent, persists until manual intervention, and every non-English-default forum is affected. |
…#36) * fix: make the epoch apply non-destructive to in-flight requests v1.1.7 reused core's full-reset recipe (`cache:clear`) as a *runtime* recipe: every settings save and extension toggle now ran the same deletions on every pod, in the request path, while visitors were mid-page. The recipe didn't change; the context did — and deletion's inherent hazard (removing a file a running request is about to use) went from theoretical to daily. Observed in production as intermittent 500s and stale shared assets after admin actions. An apply now forgets cache entries rather than deleting the files those entries reference: - Shared compiled assets are no longer flushed. Core already flushes them once, on the pod handling the admin action. Flushing again from every pod's apply meant up to 2N concurrent writers on the non-atomic read-modify-write of rev-manifest.json; lost updates left a revision pointing at stale content, which browsers and CDNs then cache until the next admin action. (Seen as: a disable not reflected on the forum, a newly enabled extension's settings page empty, mixed revisions where admin-de.js advanced while admin.js stayed put.) - storage/formatter/* is no longer deleted. The generated renderer class names are content hashes, so a rebuilt formatter simply writes new files and superseded ones are inert. Deleting them mid-traffic breaks requests that are unserializing the cached formatter: the class file vanishes, the object comes back incomplete, and calling a method on it throws \Error — which is not \Exception, so it escapes core's render guard in BasicPostSerializer and surfaces as a 500. Core's own runtime refresh, Formatter::flush(), likewise only forgets the cache entry; cache:clear sweeps the orphans. - storage/views/* is no longer deleted. Blade recompiles by source mtime, and toggles or settings saves never change template sources (deployments do, and those run cache:clear). - The global opcache_reset() is replaced by targeted opcache_invalidate() on the locale catalogue files we do delete. A global reset restarts the whole php-fpm pool mid-traffic — a recompile storm, with lazily autoloaded classes able to fail during the swap — which is far more disruptive than the staleness it guarded against. What an apply still does, unchanged: flush the file cache, delete and invalidate the compiled locale catalogues (the only cache here whose filenames are not content-derived, so deletion is the only way to force a rebuild), drop the shared settings cache and the resolved repository instance, and record the applied epoch per pod and SAPI under an atomic claim. Tests pin the new contract: an apply clears the locale catalogues but leaves formatter class files, compiled views, the shared assets and rev-manifest.json untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs/tests: state the apply's residuals honestly and pin the asset contract Review follow-up. No behaviour change (src/ diff vs the fix commit is comments only). - Formatter: justify leaving storage/formatter/* by parity with core's own runtime refresh (Formatter::flush() and the Formatter extender only forget the cache entry) rather than by the production log line, which is an E_WARNING that also occurs on healthy parse-only requests. The renderer class is autoloaded DURING unserialize(), so the deletion window is microseconds. Document the residuals: same-hash rebuilds rewrite the live class file in place, and concurrent cache misses each rebuild it. - Views: core does delete storage/views/* on the acting pod for extensions that register views; the reason other pods can leave them is that compiled views are keyed by source path and expire by source mtime. - OPcache: drop the unsupported "classes fail during the swap" rationale; the targeted invalidation is belt-and-braces (Symfony re-invalidates on rewrite; no-op from the CLI). Note that the per-SAPI double apply now buys little. - DISTRIBUTED_CACHE.md: the manifest race is reduced (fewer writers), not eliminated, and core's single flush is itself a dozen sequential writes; attribute the racing-refill half of the residual to this extension's settings layer; state the real per-apply cost, including that Saved triggers a full apply core does not need; point operators at the core 1.x fallback-catalogue bug (flarum/framework#5024) that propagation cannot fix. README FAQ updated. - Test: always seed rev-manifest.json with a revision for the asset sentinel (a re-added flush is a no-op against an empty manifest, which is what a second run inherited), name the sentinel forum.js, assert flarum.formatter is forgotten, clean seeded files up in tearDown only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Backport of #5023 to
1.x. See that PR for the full write-up; summary below.Symptom
Compiled locale JS assets (e.g.
admin-en.js) sometimes contain unresolved translation references — literal values like:so the admin UI displays raw
=> core.admin.…strings. Only the fallback locale (en) is affected; the poisoned compiled asset persists until a cache flush (its revision exists, so it is never recompiled). Found in production on Flarum 1.8.19 (community.sbb.ch) and confirmed end-to-end there: touchinggetCatalogue('de')before recompiling the English admin locale asset deterministically produces the broken asset; removing that single line produces a clean one.Root cause
Translator::getCatalogue()gated=>-reference resolution on a locale-keyed flag (! isset($this->catalogues[$locale])). But Symfony'sTranslator::loadFallbackCatalogues()(symfony/translation v5.4), when loading any other locale, pre-stores the originalencatalogue in$this->catalogues['en']while attaching only a copy to the requesting catalogue. Loadingdefirst therefore stores a raw, never-parsedenoriginal; a latergetCatalogue('en')finds the locale already set, skips parsing, and returns it raw.Flarum\Frontend\AddTranslationsthen compiles the literal=> …strings into the locale JS asset.Deterministic repro (locale
de, fallback['en'], non-debug, cache dir — mirroringLocaleServiceProvider):Fix
Track parsed state per catalogue object instead of per locale name, and walk the fallback chain on every
getCatalogue()call — the raw stored original and its parsed copy share a locale name, so a locale-keyed flag cannot distinguish them. Per-object tracking parses each catalogue object exactly once;parseCatalogue()is unchanged.Differences from the 2.x fix, for PHP 7.3 compatibility:
SplObjectStorage(contains()/attach()) instead ofWeakMap, untyped private property.Tests
New
TranslatorReferenceResolutionTest(PHPUnit 9 style), wired likeLocaleServiceProvider:enloaded directly → resolved (regression guard).enfirst loaded implicitly asde's fallback, then requested directly → resolved (failed before this fix with'=> bar'vs'Resolved').de's attached fallback copy resolved, viagetFallbackCatalogue()andtrans()fallthrough (regression guard).Full unit suite and PHPStan pass on
1.x.Context
Surfaced while working on FriendsOfFlarum/redis#34 — multi-instance cache invalidation increases recompile frequency, raising exposure — but reproducible on a bare single-instance install.
🤖 Generated with Claude Code