fix: don't cache the 'en' language fallback on anonymous pages (#41618)

[DeepDiver1975]

Problem

Anonymous pages — the login page and the password-protected public-share page — always render in English, even when default_language is configured or the browser sends a matching Accept-Language header. Authenticated pages render in the correct language. Fixes #41618.

Root cause

OC\L10N\Factory is a per-request singleton. findLanguage() short-circuits on a non-empty $requestLanguage (and languageExists(_, 'en') is always true). When the first no-app findLanguage() runs without a usable Accept-Language match — e.g. the header isn't forwarded by the web server, as in the official Docker/Apache image — setLanguageFromRequest() caches the 'en' last-resort fallback into $requestLanguage:

if ($app === null && !$this->requestLanguage) {
    $this->requestLanguage = 'en';
}
return 'en';

Every later lookup on that request then returns 'en' before default_language (step 3) or Accept-Language (step 4) is ever consulted. Authenticated requests are unaffected because they seed $requestLanguage from the stored user language first.

The original triage hypothesis (guest layout not consulting findLanguage()) was refuted while tracing the code — layout.guest.php / TemplateLayout do call findLanguage(); the defect is the poisoned cache.

Fix

Stop caching the 'en' fallback in setLanguageFromRequest(). default_language and Accept-Language are then re-evaluated on subsequent lookups, so anonymous pages honor both. Single-file behavioral change in lib/private/L10N/Factory.php.

Tests

tests/lib/L10N/FactoryTest.php: the dataSetLanguageFromRequest rows that previously enshrined the 'en' caching (no-app, no available match) now assert the fallback is returned ('en') but not stored (requestLanguage stays '') — which is exactly the regression guard for this bug.

Verified locally: php -l clean on both files. Full suite runs in CI (Test (Linux)).

🤖 Generated with Claude Code

[DeepDiver1975]

Code Review

Overview

Fixes #41618 where anonymous pages (login, public-share password page) always render in English despite a configured default_language or a matching Accept-Language header. Root cause is well-diagnosed: Factory is a per-request singleton, and setLanguageFromRequest() cached the 'en' last-resort fallback into $this->requestLanguage. Since findLanguage() short-circuits on a non-empty requestLanguage (and languageExists(_, 'en') is always true), every later lookup returned 'en' before default_language (step 3) or Accept-Language (step 4) could be consulted. The fix removes that one caching block.

Correctness — sound

Traced the full findLanguage() flow against the change:

• The return 'en' value is preserved, so get() and the setUserValue(..., $lang) call behave identically. Only the singleton's cached state changes.
• The two legitimate cache writes for real Accept-Language matches are untouched, so a genuine browser-language preference is still cached and honored — surgical.
• Ordering holds: default_language is evaluated before setLanguageFromRequest(). Because requestLanguage now only ever caches a real match (never the 'en' fallback), repeated lookups stay consistent.
• requestLanguage has no consumers outside Factory.php, so no external code relies on the old poisoning behavior.

Style / conventions — good

• Changelog follows repo convention (numeric filename, Bugfix: prefix, issue URL footer).
• Inline comment captures the non-obvious why — worth keeping.

Test coverage — one gap worth closing

The revised dataSetLanguageFromRequest row [null, 'de', null, ['ru'], 'en', ''] correctly asserts return 'en' but don't store it — a solid regression guard for the mechanism.

However, no test reproduces the actual reported bug end-to-end: two sequential findLanguage() calls on an anonymous request where the second call must now honor default_language after the first fell through to the 'en' fallback. All existing findLanguage tests pre-seed requestLanguage and make a single call.

Recommend adding a testFindLanguage... case with empty requestLanguage, no user lang, first call yielding 'en', then a default_language set and a second findLanguage() asserting it is returned. That guards the regression at the layer the bug manifested.

Performance — negligible

findAvailableLanguages() is memoized, so re-running setLanguageFromRequest() on repeated fallback lookups is trivial.

Security — none

No auth, input-handling, or output-encoding surface touched.

Verdict

Clean, minimal, correctly-reasoned fix. Approve, with the recommended end-to-end findLanguage() regression test.

[DeepDiver1975]

Re-review — after applying the recommended test

Pushed 928906020f adding the end-to-end regression test I recommended above.

What was added

testFindLanguageDoesNotCacheFallbackAcrossAnonymousLookups reproduces the exact #41618 scenario at the findLanguage() layer (not just the isolated setLanguageFromRequest() unit):

1. Anonymous request (getUser() returns null), installed = true, empty requestLanguage, no Accept-Language header (the not-forwarded case from the official Docker/Apache image).
2. First findLanguage() → no user lang, no default_language, no header match → returns the 'en' last-resort fallback.
3. Asserts requestLanguage is still '' — the fallback was not cached.
4. default_language is then set to 'de'; the second findLanguage() returns 'de' instead of a poisoned cached 'en'.

This locks in the fix at the layer where the bug actually manifested. On the pre-fix code, step 4 would return 'en', so the test fails without the fix — a true regression guard.

Verification

• php -l clean.
• Full FactoryTest suite green locally: OK (49 tests, 134 assertions), including the new case (7 assertions).

The earlier test-coverage gap is now closed. No other findings — the fix and its tests look complete. Approve.

[dj4oC]

Fixes #41618 — anonymous pages (login / public-share password page) now honor default_language and Accept-Language instead of always rendering in English, by no longer caching the 'en' last-resort fallback in the per-request L10N\Factory singleton.

Branch target

oC11 / master — in scope. ✓

Review passes

• Security: no findings. Pure i18n language-selection logic; no user input reaches a sink and there is no authz / path-traversal / injection surface. (OWASP/CWE: n/a.)
• Stability: no findings. Removing the $this->requestLanguage = 'en' assignment leaves requestLanguage empty on the last-resort path, so a later findLanguage() re-evaluates default_language (step 3) then Accept-Language (step 4). The method still returns 'en', so callers that don't re-query are unaffected — no BC break. PHP 8.3-clean (no 7.4-only idioms reintroduced).
• Performance: no finding of concern. Anonymous requests with no matchable language now re-run the (in-memory, DB-free) resolution on each lookup rather than short-circuiting on a cached 'en'. Negligible; introduces no new queries.
• Test coverage: the changed line is directly exercised — new regression test testFindLanguageDoesNotCacheFallbackAcrossAnonymousLookups() plus the updated dataSetLanguageFromRequest rows assert the fallback is returned ('en') but not cached (requestLanguage stays ''). Verified independently rather than from the PR text: PHP Unit is green on sqlite / mariadb 10.6 & 10.11 / mysql 8.0 / postgres.

Quality gates

• Security: no findings
• Stability: no findings
• Performance: no findings
• Test coverage: PHPUnit regression added; changed lines covered; PHP Unit CI green on all 5 DB engines
• TODOs found: none
• Dependency touched: no
• CI status: all green
• Stale review: no

Note (non-blocking)

This changes a user-visible frontend behavior (the rendered language of the login and public-share pages). Unit coverage of the defect itself is solid, but there is no end-to-end / acceptance guard that an anonymous page actually renders in default_language. A follow-up acceptance-level check would close that gap — not blocking, given the regression unit test and green CI.

Verdict

Commenting — no blocking findings. Requires one human approval before merge per repo policy.

🤖 Automated review by Claude Code (security · stability · performance · coverage)

---

Generated by Claude Code