Fix cron-sent emails rendering dates in en_US instead of configured locale (#4535)

* Fix cron-sent emails rendering dates in en_US instead of configured locale

In CLI/cron context there is no Accept-Language header, so
Locale::acceptFromHttp() fails and Locale::lookup() with an empty
detected locale returns the first enabled locale (en_US) instead of
null, overriding the configured i18n.locale default. Guard
getBrowserLocale() against empty/unparsable headers so it falls back
to the configured default, and stop queueing empty locale cookies.

Also plumb an explicit per-recipient locale through email rendering:
TwigFactory::createEmailEnvironment(), System render helpers, and
Email::sendTemplate() now honor the recipient's language
(client lang, per staff member on broadcasts), outranking browser
auto-detection, with Massmailer rendering per-client too.

Fixes #4524

* Fix tests: simulate header-less requests, update subject render expectation

Symfony's Request::create() seeds a default Accept-Language header,
so strip it to reproduce the CLI/cron condition in the new
getActiveLocale fallback tests. Update the existing
renderEmailSubjectString test for the new locale parameter.
This commit is contained in:
Adam Daley authored and GitHub committed 2026-10-16 00:02:51 +01:00
1 parent a49cf8fd5f
commit 1a469d09cf
8 files changed
+361 -28

No files matched your search

+29 -5
View File
@@ -29,6 +29,7 @@ use FOSSBilling\Url;
use FOSSBilling\Version;
use Symfony\Component\Filesystem\Path;
use Symfony\Component\Intl\Currencies;
use Symfony\Component\Intl\Locales;
use Twig\Environment;
use Twig\Extension\AttributeExtension;
use Twig\Extension\CoreExtension;
@@ -179,21 +180,27 @@ class TwigFactory
* Used for database-stored templates (email templates, mass mailer).
*
* The env is built per-call (not memoized) so each email can be rendered
* with the recipient's timezone.
* with the recipient's timezone and locale.
*
* @param string|null $timezone IANA timezone to use for date formatting;
* pass the recipient's timezone so emails
* render in their local time. Defaults to
* the active user's timezone / config.
* @param string|null $locale ICU locale to use for date formatting;
* pass the recipient's language so emails
* render in their language. An explicit
* recipient language outranks browser
* auto-detection. Defaults to the active
* locale / config.
*/
public function createEmailEnvironment(?string $timezone = null): Environment
public function createEmailEnvironment(?string $timezone = null, ?string $locale = null): Environment
{
$timezone ??= $this->resolveTimezoneForActiveUser();
// Create Twig environment with ArrayLoader
$loader = new ArrayLoader();
$twig = new Environment($loader, $this->baseConfig);
$this->configureRendering($twig, $timezone);
$this->configureRendering($twig, $timezone, $locale);
// Add only essential extensions for email templates
$twig->addExtension(new StringLoaderExtension());
@@ -418,7 +425,7 @@ class TwigFactory
return 2;
}
private function configureRendering(Environment $twig, string $timezone): void
private function configureRendering(Environment $twig, string $timezone, ?string $locale = null): void
{
$twig->getExtension(CoreExtension::class)->setNumberFormat($this->getDefaultCurrencyFractionDigits(), '.', '');
$twig->getExtension(CoreExtension::class)->setTimezone($timezone);
@@ -426,7 +433,7 @@ class TwigFactory
$dateFormat = strtoupper((string) Config::getProperty('i18n.date_format', 'MEDIUM'));
$timeFormat = strtoupper((string) Config::getProperty('i18n.time_format', 'SHORT'));
$twig->addExtension(new IntlExtension(new \IntlDateFormatter(
i18n::getActiveLocale($this->di['request'], true, $this->di['cookie_queue']),
$this->resolveLocale($locale),
constant("\IntlDateFormatter::$dateFormat"),
constant("\IntlDateFormatter::$timeFormat"),
$timezone,
@@ -435,6 +442,23 @@ class TwigFactory
)));
}
/**
* Resolve the locale date filters should render in.
*
* An explicit recipient language outranks browser auto-detection. Any
* valid ICU locale is accepted (date formatting needs no FOSSBilling
* translation files); anything else falls back to the active locale,
* which itself falls back to the configured `i18n.locale` default.
*/
private function resolveLocale(?string $locale): string
{
if (!empty($locale) && Locales::exists($locale)) {
return $locale;
}
return i18n::getActiveLocale($this->di['request'], true, $this->di['cookie_queue']);
}
/**
* Set the CSRF cookie for the double-submit pattern.
* Should only be called in browser-facing contexts (admin/client page loads).
+19 -6
View File
@@ -93,23 +93,34 @@ class i18n
return null;
}
$header = $request->headers->get('Accept-Language', '');
$header = trim((string) $request->headers->get('Accept-Language', ''));
// CLI/cron requests and header-less clients send no Accept-Language.
// Without this guard, Locale::lookup() with an empty detected locale
// returns the first enabled locale (e.g. en_US) instead of null,
// overriding the configured i18n.locale default.
if ($header === '') {
return null;
}
try {
$detectedLocale = @\Locale::acceptFromHttp($header);
$detectedLocale = @\Locale::canonicalize($detectedLocale . '.utf8');
$acceptedLocale = @\Locale::acceptFromHttp($header);
if (empty($acceptedLocale)) {
return null;
}
$detectedLocale = @\Locale::canonicalize($acceptedLocale . '.utf8');
} catch (\Throwable) {
$detectedLocale = '';
return null;
}
if (empty($detectedLocale)) {
$detectedLocale = '';
return null;
}
try {
$matchingLocale = \Locale::lookup(self::getLocales(), $detectedLocale, false, null);
} catch (\Throwable) {
$matchingLocale = null;
return null;
}
/* The system was unable to match the browser locale to one of our local ones.
@@ -128,6 +139,8 @@ class i18n
return $locale;
}
}
return null;
}
$cookies?->queue(CookieNames::LOCALE, (string) $matchingLocale, strtotime('+1 month'), '/');
+33 -10
View File
@@ -324,11 +324,12 @@ class Service implements \FOSSBilling\InjectionAwareInterface
}
$systemService = $this->di['mod_service']('system');
// Pick a timezone for the date filters. Client-bound and individual
// staff-bound emails render in the recipient's timezone; broadcasts and
// misc fall through to the config default. Staff broadcasts parse per
// recipient in the send loop below; the value computed here is unused
// for that branch.
// Pick a timezone and locale for the date filters. Client-bound and
// individual staff-bound emails render in the recipient's timezone
// and language; broadcasts and misc fall through to the config
// defaults. An explicit recipient language outranks browser
// auto-detection. Staff broadcasts parse per recipient in the send
// loop below; the values computed here are unused for that branch.
$recipientTimezone = null;
if (isset($customer) && !empty($customer['timezone'])) {
$recipientTimezone = (string) $customer['timezone'];
@@ -336,7 +337,14 @@ class Service implements \FOSSBilling\InjectionAwareInterface
$recipientTimezone = (string) $oneStaff['timezone'];
}
[$subject, $content] = $this->_parse($template, $vars, $recipientTimezone);
$recipientLocale = null;
if (isset($customer)) {
$recipientLocale = $this->resolveRecipientLocale($customer);
} elseif (isset($oneStaff)) {
$recipientLocale = $this->resolveRecipientLocale($oneStaff);
}
[$subject, $content] = $this->_parse($template, $vars, $recipientTimezone, $recipientLocale);
$emailMod = $this->di['mod']('email');
$emailSettings = $emailMod->getConfig();
@@ -361,7 +369,7 @@ class Service implements \FOSSBilling\InjectionAwareInterface
$staffVars = $vars;
$staffVars['staff'] = $this->safeStaffTemplateVars($staff);
$staffTimezone = !empty($staff['timezone'] ?? null) ? (string) $staff['timezone'] : null;
[$subject, $content] = $this->_parse($template, $staffVars, $staffTimezone);
[$subject, $content] = $this->_parse($template, $staffVars, $staffTimezone, $this->resolveRecipientLocale($staff));
$to = $staff['email'];
$to_name = $staff['name'];
@@ -695,14 +703,29 @@ class Service implements \FOSSBilling\InjectionAwareInterface
return $str . '{% endapply %}';
}
private function _parse(EmailTemplate $template, array $vars, ?string $timezone = null): array
/**
* Resolve the locale an email should be rendered in for the given recipient.
*
* Returns the recipient's language (client `lang`, staff `lang` when
* present) when set, otherwise null to fall back to the active locale /
* config default. Validity is checked where the locale is consumed
* (TwigFactory), so this stays a pure extraction helper.
*/
private function resolveRecipientLocale(array $recipient): ?string
{
$lang = $recipient['lang'] ?? null;
return is_string($lang) && $lang !== '' ? $lang : null;
}
private function _parse(EmailTemplate $template, array $vars, ?string $timezone = null, ?string $locale = null): array
{
$systemService = $this->di['mod_service']('System');
[$subjectTemplate, $contentTemplate] = $this->getEffectiveTemplateParts($template);
try {
$pc = $systemService->renderEmailTplString($contentTemplate, $vars, $timezone);
$ps = $systemService->renderEmailSubjectString($subjectTemplate, $vars, $timezone);
$pc = $systemService->renderEmailTplString($contentTemplate, $vars, $timezone, $locale);
$ps = $systemService->renderEmailSubjectString($subjectTemplate, $vars, $timezone, $locale);
if ($template->hasError()) {
$template->clearError();
@@ -737,6 +737,238 @@ test('sendTemplate handles to_staff and to_client options', function (array $dat
expect($persistedQueue->getRecipient())->toBe($expectedRecipient);
})->with('sendTemplateExistsStaffProvider');
test('sendTemplate renders client-bound email in the client language', function (): void {
$service = new Box\Mod\Email\Service();
$di = container();
$emailTemplate = emailTemplate(data: ['enabled' => true]);
$templateRepo = Mockery::mock(Box\Mod\Email\Repository\EmailTemplateRepository::class);
$templateRepo->shouldReceive('findOneByActionCode')->andReturn($emailTemplate);
$em = emailBuildEm(null, $templateRepo);
$em->shouldReceive('persist')->atLeast()->once();
$em->shouldReceive('flush')->atLeast()->once();
$seenLocales = [];
$captureLocale = function (string $tpl, array $vars, ?string $timezone = null, ?string $locale = null) use (&$seenLocales): string {
$seenLocales[] = $locale;
return 'value';
};
$system = Mockery::mock(Box\Mod\System\Service::class);
$system->shouldReceive('getParamValue')->atLeast()->once()->andReturn('value');
$system->shouldReceive('renderEmailTplString')->atLeast()->once()->andReturnUsing($captureLocale);
$system->shouldReceive('renderEmailSubjectString')->atLeast()->once()->andReturnUsing($captureLocale);
$clientServiceMock = Mockery::mock(Box\Mod\Client\Service::class);
$clientServiceMock->shouldReceive('get')->atLeast()->once()->andReturn(createEntity(Box\Mod\Client\Entity\Client::class));
$clientServiceMock->shouldReceive('toApiArray')->atLeast()->once()->andReturn([
'id' => 1,
'email' => 'client@example.com',
'first_name' => 'John',
'last_name' => 'Smith',
'lang' => 'es_ES',
'timezone' => 'UTC',
]);
$staffServiceMock = Mockery::mock(Box\Mod\Staff\Service::class);
$cryptMock = Mockery::mock(FOSSBilling\Crypt::class);
$cryptMock->shouldReceive('encrypt')->atLeast()->once();
$modMock = Mockery::mock(FOSSBilling\Module::class)->makePartial();
$modMock->shouldReceive('getConfig')->atLeast()->once()->andReturn([
'from_name' => 'Test',
'from_email' => 'test@test.com',
]);
$di['api_admin'] = function () use ($di) {
$api = new FOSSBilling\Api\Proxy(\Tests\Helpers\admin());
$api->setDi($di);
return $api;
};
$validatorMock = Mockery::mock(FOSSBilling\Validate::class);
$validatorMock->shouldReceive('checkRequiredParamsForArray')->byDefault();
$di['validator'] = $validatorMock;
$di['em'] = $em;
$di['crypt'] = $cryptMock;
$di['mod'] = $di->protect(fn () => $modMock);
$di['mod_service'] = $di->protect(moduleService([
'staff' => $staffServiceMock,
'system' => $system,
'client' => $clientServiceMock,
]));
$di['tools'] = new FOSSBilling\Tools();
$service->setDi($di);
$result = $service->sendTemplate(['code' => 'mod_email_test', 'to_client' => 1]);
expect($result)->toBeTrue();
expect($seenLocales)->not->toBeEmpty();
foreach ($seenLocales as $locale) {
expect($locale)->toBe('es_ES');
}
});
test('sendTemplate falls back to the default locale when the client has no language', function (): void {
$service = new Box\Mod\Email\Service();
$di = container();
$emailTemplate = emailTemplate(data: ['enabled' => true]);
$templateRepo = Mockery::mock(Box\Mod\Email\Repository\EmailTemplateRepository::class);
$templateRepo->shouldReceive('findOneByActionCode')->andReturn($emailTemplate);
$em = emailBuildEm(null, $templateRepo);
$em->shouldReceive('persist')->atLeast()->once();
$em->shouldReceive('flush')->atLeast()->once();
$seenLocales = [];
$captureLocale = function (string $tpl, array $vars, ?string $timezone = null, ?string $locale = null) use (&$seenLocales): string {
$seenLocales[] = $locale;
return 'value';
};
$system = Mockery::mock(Box\Mod\System\Service::class);
$system->shouldReceive('getParamValue')->atLeast()->once()->andReturn('value');
$system->shouldReceive('renderEmailTplString')->atLeast()->once()->andReturnUsing($captureLocale);
$system->shouldReceive('renderEmailSubjectString')->atLeast()->once()->andReturnUsing($captureLocale);
$clientServiceMock = Mockery::mock(Box\Mod\Client\Service::class);
$clientServiceMock->shouldReceive('get')->atLeast()->once()->andReturn(createEntity(Box\Mod\Client\Entity\Client::class));
$clientServiceMock->shouldReceive('toApiArray')->atLeast()->once()->andReturn([
'id' => 1,
'email' => 'client@example.com',
'first_name' => 'John',
'last_name' => 'Smith',
'timezone' => 'UTC',
]);
$staffServiceMock = Mockery::mock(Box\Mod\Staff\Service::class);
$cryptMock = Mockery::mock(FOSSBilling\Crypt::class);
$cryptMock->shouldReceive('encrypt')->atLeast()->once();
$modMock = Mockery::mock(FOSSBilling\Module::class)->makePartial();
$modMock->shouldReceive('getConfig')->atLeast()->once()->andReturn([
'from_name' => 'Test',
'from_email' => 'test@test.com',
]);
$di['api_admin'] = function () use ($di) {
$api = new FOSSBilling\Api\Proxy(\Tests\Helpers\admin());
$api->setDi($di);
return $api;
};
$validatorMock = Mockery::mock(FOSSBilling\Validate::class);
$validatorMock->shouldReceive('checkRequiredParamsForArray')->byDefault();
$di['validator'] = $validatorMock;
$di['em'] = $em;
$di['crypt'] = $cryptMock;
$di['mod'] = $di->protect(fn () => $modMock);
$di['mod_service'] = $di->protect(moduleService([
'staff' => $staffServiceMock,
'system' => $system,
'client' => $clientServiceMock,
]));
$di['tools'] = new FOSSBilling\Tools();
$service->setDi($di);
$result = $service->sendTemplate(['code' => 'mod_email_test', 'to_client' => 1]);
expect($result)->toBeTrue();
expect($seenLocales)->not->toBeEmpty();
foreach ($seenLocales as $locale) {
expect($locale)->toBeNull();
}
});
test('sendTemplate renders staff broadcast emails in each recipient language', function (): void {
$service = new Box\Mod\Email\Service();
$di = container();
$emailTemplate = emailTemplate(data: ['enabled' => true]);
$templateRepo = Mockery::mock(Box\Mod\Email\Repository\EmailTemplateRepository::class);
$templateRepo->shouldReceive('findOneByActionCode')->andReturn($emailTemplate);
$templateGroupRepo = Mockery::mock(Box\Mod\Email\Repository\EmailTemplateGroupRepository::class);
$templateGroupRepo->shouldReceive('getGroupIdsForTemplate')->andReturn([3]);
$em = emailBuildEm(null, $templateRepo, null, true, $templateGroupRepo);
$em->shouldReceive('persist')->atLeast()->once();
$em->shouldReceive('flush')->atLeast()->once();
$seenLocales = [];
$captureLocale = function (string $tpl, array $vars, ?string $timezone = null, ?string $locale = null) use (&$seenLocales): string {
$seenLocales[] = $locale;
return 'value';
};
$system = Mockery::mock(Box\Mod\System\Service::class);
$system->shouldReceive('getParamValue')->atLeast()->once()->andReturn('value');
$system->shouldReceive('renderEmailTplString')->atLeast()->once()->andReturnUsing($captureLocale);
$system->shouldReceive('renderEmailSubjectString')->atLeast()->once()->andReturnUsing($captureLocale);
$groupMemberRepo = Mockery::mock(Box\Mod\Staff\Repository\AdminGroupMemberRepository::class);
$groupMemberRepo->shouldReceive('getActiveStaffInGroups')->atLeast()->once()->with([3])->andReturn([
[
'id' => 1,
'email' => 'staff@example.com',
'name' => 'George',
'signature' => '',
'timezone' => null,
'lang' => 'de_DE',
],
]);
$staffServiceMock = Mockery::mock(Box\Mod\Staff\Service::class);
$staffServiceMock->shouldReceive('getAdminGroupMemberRepository')->atLeast()->once()->andReturn($groupMemberRepo);
$clientServiceMock = Mockery::mock(Box\Mod\Client\Service::class);
$cryptMock = Mockery::mock(FOSSBilling\Crypt::class);
$cryptMock->shouldReceive('encrypt')->atLeast()->once();
$modMock = Mockery::mock(FOSSBilling\Module::class)->makePartial();
$modMock->shouldReceive('getConfig')->atLeast()->once()->andReturn([
'from_name' => 'Test',
'from_email' => 'test@test.com',
]);
$di['api_admin'] = function () use ($di) {
$api = new FOSSBilling\Api\Proxy(\Tests\Helpers\admin());
$api->setDi($di);
return $api;
};
$validatorMock = Mockery::mock(FOSSBilling\Validate::class);
$validatorMock->shouldReceive('checkRequiredParamsForArray')->byDefault();
$di['validator'] = $validatorMock;
$di['em'] = $em;
$di['crypt'] = $cryptMock;
$di['mod'] = $di->protect(fn () => $modMock);
$di['mod_service'] = $di->protect(moduleService([
'staff' => $staffServiceMock,
'system' => $system,
'client' => $clientServiceMock,
]));
$di['tools'] = new FOSSBilling\Tools();
$service->setDi($di);
$result = $service->sendTemplate(['code' => 'mod_email_test', 'to_staff' => true]);
expect($result)->toBeTrue();
// The pre-loop generic parse uses the default locale; the per-recipient
// re-parse must use the staff member's language.
expect($seenLocales)->toContain('de_DE');
});
dataset('sendTemplateClientBillingProvider', fn (): array => [
'validated billing override' => [
['client_billing_email' => 'billing@example.com'],
+4 -2
View File
@@ -225,8 +225,10 @@ class Service implements \FOSSBilling\InjectionAwareInterface
$clientArr = $clientService->toApiArray($client, true, null);
$vars = ['c' => $clientArr];
$ps = $systemService->renderEmailSubjectString($model->getSubject(), $vars);
$pc = $systemService->renderEmailTplString($model->getContent(), $vars);
$clientTimezone = !empty($clientArr['timezone']) ? (string) $clientArr['timezone'] : null;
$clientLocale = !empty($clientArr['lang']) ? (string) $clientArr['lang'] : null;
$ps = $systemService->renderEmailSubjectString($model->getSubject(), $vars, $clientTimezone, $clientLocale);
$pc = $systemService->renderEmailTplString($model->getContent(), $vars, $clientTimezone, $clientLocale);
return [$ps, $pc];
}
+9 -4
View File
@@ -588,15 +588,20 @@ class Service
* pass the recipient's so emails render in
* their local time. Falls back to the active
* user's timezone, then the config.
* @param string|null $locale Optional ICU locale for date formatting;
* pass the recipient's language so emails
* render in their language. Outranks browser
* auto-detection. Falls back to the active
* locale, then the config.
*
* @return string The rendered template
*
* @throws \FOSSBilling\InformationException If template violates sandbox policy or has syntax errors
*/
public function renderEmailTplString(string $tpl, array $vars, ?string $timezone = null): string
public function renderEmailTplString(string $tpl, array $vars, ?string $timezone = null, ?string $locale = null): string
{
$twigFactory = $this->di['twig_factory'];
$twig = $twigFactory->createEmailEnvironment($timezone);
$twig = $twigFactory->createEmailEnvironment($timezone, $locale);
return SandboxedStringRenderer::render(
$twig,
@@ -616,9 +621,9 @@ class Service
* HTML-autoescaping email environment, decoding once to restore it as
* typed. Never use this for the HTML body.
*/
public function renderEmailSubjectString(string $tpl, array $vars, ?string $timezone = null): string
public function renderEmailSubjectString(string $tpl, array $vars, ?string $timezone = null, ?string $locale = null): string
{
$rendered = $this->renderEmailTplString($tpl, $vars, $timezone);
$rendered = $this->renderEmailTplString($tpl, $vars, $timezone, $locale);
return html_entity_decode($rendered, ENT_QUOTES | ENT_HTML5, 'UTF-8');
}
@@ -228,7 +228,7 @@ test('renderEmailSubjectString decodes the HTML autoescape pass', function (): v
$service = Mockery::mock(Service::class)->makePartial();
$service->shouldReceive('renderEmailTplString')
->once()
->with('[A & B Ltd] Invoice', [], null)
->with('[A & B Ltd] Invoice', [], null, null)
->andReturn('[A & B Ltd] Invoice');
expect($service->renderEmailSubjectString('[A & B Ltd] Invoice', []))->toBe('[A & B Ltd] Invoice');
+34
View File
@@ -210,6 +210,40 @@ test('getActiveLocale returns the configured default when no cookie and no Accep
expect(i18n::getActiveLocale(Request::create('/'), false))->toBe('en_US');
});
test('getActiveLocale falls back to config default with auto-detect and no Accept-Language header (cron/CLI)', function (): void {
Config::setProperty('i18n.locale', 'de_DE');
// Symfony's Request::create() seeds a default Accept-Language header,
// so strip it to simulate a header-less CLI/cron request.
$request = Request::create('/');
$request->headers->remove('Accept-Language');
expect(i18n::getActiveLocale($request, true))->toBe('de_DE');
});
test('getActiveLocale falls back to config default with auto-detect and an unparsable Accept-Language header', function (): void {
Config::setProperty('i18n.locale', 'de_DE');
$request = Request::create('/', 'GET', [], [], [], ['HTTP_ACCEPT_LANGUAGE' => '*']);
expect(i18n::getActiveLocale($request, true))->toBe('de_DE');
});
test('getActiveLocale with auto-detect and no header does not queue a locale cookie', function (): void {
Config::setProperty('i18n.locale', 'de_DE');
$cookies = new CookieQueue();
$request = Request::create('/');
$request->headers->remove('Accept-Language');
expect(i18n::getActiveLocale($request, true, $cookies))->toBe('de_DE');
$response = new Response();
$cookies->applyToResponse($response);
expect($response->headers->getCookies())->toBe([]);
});
test('getActiveTimezone migrates and expires the legacy timezone cookie', function (): void {
$request = requestWithTimezoneCookie('Europe/Berlin', CookieNames::LEGACY_TIMEZONE);
$cookies = new CookieQueue();