From 3dc3d7e4331c4aa54c4c23f3f37f21abf96d7a8c Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 7 Nov 2024 21:55:20 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20newsletter=20not=20sendi?= =?UTF-8?q?ng=20if=20locale=20is=20invalid=20(#21573)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://github.com/moment/luxon/blob/master/docs/intl.md - We noticed the following error trace: RangeError: Incorrect locale information provided at BatchSendingService.retryDb (/home/ghost/node_modules/@tryghost/email-service/lib/BatchSendingService.js:639:32) at new DateTimeFormat () at getCachedDTF (/home/ghost/node_modules/luxon/build/node/luxon.js:621:11) at new PolyDateFormatter (/home/ghost/node_modules/luxon/build/node/luxon.js:842:16) at Locale.dtFormatter (/home/ghost/node_modules/luxon/build/node/luxon.js:1066:12) at Formatter.dtFormatter (/home/ghost/node_modules/luxon/build/node/luxon.js:2274:21) at Formatter.formatDateTime (/home/ghost/node_modules/luxon/build/node/luxon.js:2280:17) at DateTime.toLocaleString (/home/ghost/node_modules/luxon/build/node/luxon.js:6893:78) at formatDateLong (/home/ghost/node_modules/@tryghost/email-service/lib/EmailRenderer.js:45:74) at Object.getValue (/home/ghost/node_modules/@tryghost/email-service/lib/EmailRenderer.js:683:47) at /home/ghost/node_modules/@tryghost/email-service/lib/SendingService.js:158:36 at Array.map () at /home/ghost/node_modules/@tryghost/email-service/lib/SendingService.js:154:54 at Array.map () at SendingService.buildRecipients (/home/ghost/node_modules/@tryghost/email-service/lib/SendingService.js:151:24) at SendingService.send (/home/ghost/node_modules/@tryghost/email-service/lib/SendingService.js:127:33) at response.retryDb (/home/ghost/node_modules/@tryghost/email-service/lib/BatchSendingService.js:451:51) - This is due to the locale being user-input - it can be set to any string. - In our email sending code we pass the string to luxon to format dates, which errors if the locale is not valid according it Intl. - This fix ensures that the locale is valid before passing it to luxon, falling back to en-gb if the locale is not valid --- ghost/email-service/lib/EmailRenderer.js | 40 +++++++++--- .../email-service/test/email-renderer.test.js | 65 ++++++++++++++++++- 2 files changed, 95 insertions(+), 10 deletions(-) diff --git a/ghost/email-service/lib/EmailRenderer.js b/ghost/email-service/lib/EmailRenderer.js index cd2c3ef292..8cf928b973 100644 --- a/ghost/email-service/lib/EmailRenderer.js +++ b/ghost/email-service/lib/EmailRenderer.js @@ -12,8 +12,10 @@ const {EmailAddressParser} = require('@tryghost/email-addresses'); const {registerHelpers} = require('./helpers/register-helpers'); const crypto = require('crypto'); +const DEFAULT_LOCALE = 'en-gb'; + // Wrapper function so that i18next-parser can find these strings -const t = (x) => { +const t = (x) => { return x; }; @@ -38,10 +40,17 @@ function escapeHtml(unsafe) { .replace(/'/g, '''); } -function formatDateLong(date, timezone, locale = 'en-gb') { - if (locale === 'en') { - locale = 'en-gb'; +function isValidLocale(locale) { + try { + // Attempt to create a DateTimeFormat with the locale + new Intl.DateTimeFormat(locale); + return true; // No error means it's a valid locale + } catch (e) { + return false; // RangeError means invalid locale } +} + +function formatDateLong(date, timezone, locale = DEFAULT_LOCALE) { return DateTime.fromJSDate(date).setZone(timezone).setLocale(locale).toLocaleString({ year: 'numeric', month: 'long', @@ -185,7 +194,7 @@ class EmailRenderer { this.#models = models; this.#t = t; } - + getSubject(post, isTestEmail = false) { const subject = post.related('posts_meta')?.get('email_subject') || post.get('title'); return isTestEmail ? `[TEST] ${subject}` : subject; @@ -216,6 +225,21 @@ class EmailRenderer { }; } + // Locale is user-input, so we need to ensure it's valid + #getValidLocale() { + let locale = this.#settingsCache.get('locale') || DEFAULT_LOCALE; + + // Remove any trailing whitespace + locale = locale.trim(); + + // If the locale is just "en", or is not valid, revert to default + if (locale === 'en' || !isValidLocale(locale)) { + locale = DEFAULT_LOCALE; + } + + return locale; + } + getFromAddress(post, newsletter) { // Clean from address to ensure DMARC alignment const addresses = this.#emailAddressService.getAddress({ @@ -561,8 +585,8 @@ class EmailRenderer { * @returns {string} */ getMemberStatusText(member) { - const t = this.#t; - const locale = this.#settingsCache.get('locale'); + const t = this.#t; + const locale = this.#getValidLocale(); if (member.status === 'free') { // Not really used, but as a backup @@ -625,7 +649,7 @@ class EmailRenderer { */ buildReplacementDefinitions({html, newsletterUuid}) { const t = this.#t; // es-lint-disable-line no-shadow - const locale = this.#settingsCache.get('locale'); + const locale = this.#getValidLocale(); const baseDefinitions = [ { diff --git a/ghost/email-service/test/email-renderer.test.js b/ghost/email-service/test/email-renderer.test.js index 5dd0018ab7..1afb5457e4 100644 --- a/ghost/email-service/test/email-renderer.test.js +++ b/ghost/email-service/test/email-renderer.test.js @@ -130,7 +130,7 @@ describe('Email renderer', function () { email: 'test@example.com', createdAt: new Date(2023, 2, 13, 12, 0), status: 'free' - }; + }; }); it('returns the unsubscribe header replacement by default', function () { @@ -388,7 +388,7 @@ describe('Email renderer', function () { email: 'test@example.com', createdAt: new Date(2023, 2, 13, 12, 0), status: 'free' - }; + }; }); it('handles dates when the locale is en-gb (default)', function () { @@ -399,6 +399,7 @@ describe('Email renderer', function () { assert.equal(replacements[0].id, 'created_at'); assert.equal(replacements[0].getValue(member), '13 March 2023'); }); + it('handles dates when the locale is fr', function () { emailRenderer = new EmailRenderer({ urlUtils: { @@ -427,6 +428,7 @@ describe('Email renderer', function () { assert.equal(replacements[0].id, 'created_at'); assert.equal(replacements[0].getValue(member), '13 mars 2023'); }); + it('handles dates when the locale is en (US)', function () { emailRenderer = new EmailRenderer({ urlUtils: { @@ -455,6 +457,65 @@ describe('Email renderer', function () { assert.equal(replacements[0].id, 'created_at'); assert.equal(replacements[0].getValue(member), '13 March 2023'); }); + + it('handles dates when the locale has whitespace like "en "', function () { + emailRenderer = new EmailRenderer({ + urlUtils: { + urlFor: () => 'http://example.com/subdirectory/' + }, + labs: { + isSet: () => labsEnabled + }, + settingsCache: { + get: (key) => { + if (key === 'timezone') { + return 'UTC'; + } + if (key === 'locale') { + return 'en '; + } + } + }, + settingsHelpers: {getMembersValidationKey,createUnsubscribeUrl}, + t: t + }); + const html = '%%{created_at}%%'; + const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')}); + assert.equal(replacements.length, 2); + assert.equal(replacements[0].token.toString(), '/%%\\{created_at\\}%%/g'); + assert.equal(replacements[0].id, 'created_at'); + assert.equal(replacements[0].getValue(member), '13 March 2023'); + }); + + it('handles dates when the locale is invalid like "(en)"', function () { + emailRenderer = new EmailRenderer({ + urlUtils: { + urlFor: () => 'http://example.com/subdirectory/' + }, + labs: { + isSet: () => labsEnabled + }, + settingsCache: { + get: (key) => { + if (key === 'timezone') { + return 'UTC'; + } + if (key === 'locale') { + return '(en)'; + } + } + }, + settingsHelpers: {getMembersValidationKey,createUnsubscribeUrl}, + t: t + }); + + const html = '%%{created_at}%%'; + const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')}); + assert.equal(replacements.length, 2); + assert.equal(replacements[0].token.toString(), '/%%\\{created_at\\}%%/g'); + assert.equal(replacements[0].id, 'created_at'); + assert.equal(replacements[0].getValue(member), '13 March 2023'); + }); }); describe('isMemberTrialing', function () { let emailRenderer;