🐛 Fixed replacements with fallback in plaintext newsletters (#16372)

fixes https://github.com/TryGhost/Team/issues/2683 When sending a newsletter with a replacement that has a fallback, the replacement only happens in the HTML version of the newsletter. The plaintext version isn't replaced. This commit fixes the issue and adds some tests to make sure it doesn't happen again. The cause of the issue was that we used the original matched Regex text to replace. But that was calculated on the HTML version, so double quotes were encoded. This change updates the generated 'token' regex to also match on both a double quote as the escaped double quote.
2025-01-20 22:42:53 -05:00 · 2023-03-07 15:34:43 +01:00 · 2023-03-07 15:34:43 +01:00 · 3db434736b
commit 3db434736b
parent d58f01bac7
9 changed files with 377 additions and 71 deletions
--- a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/emails.js
+++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/emails.js
@ -1,19 +1,22 @@
-const mega = require('../../../../../../services/mega');
 const labs = require('../../../../../../../shared/labs');
 const config = require('../../../../../../../shared/config');
+const emailService = require('../../../../../../services/email-service');

 module.exports = (model, frame) => {
    const jsonModel = model.toJSON ? model.toJSON(frame.options) : model;

    // Ensure we're not outputting unwanted replacement strings when viewing email contents
    // TODO: extract this to a utility, it's duplicated in the email-preview API controller
-    const replacements = mega.postEmailSerializer.parseReplacements(jsonModel);
-    replacements.forEach((replacement) => {
-        jsonModel[replacement.format] = jsonModel[replacement.format].replace(
-            replacement.regexp,
-            replacement.fallback || ''
-        );
-    });
+    if (jsonModel.html) {
+        const replacements = emailService.renderer.buildReplacementDefinitions({html: jsonModel.html, newsletterUuid: 'preview'});
+        const exampleMember = emailService.service.getDefaultExampleMember();
+
+        jsonModel.html = emailService.service.replaceDefinitions(jsonModel.html, replacements, exampleMember);
+
+        if (jsonModel.plaintext) {
+            jsonModel.plaintext = emailService.service.replaceDefinitions(jsonModel.plaintext, replacements, exampleMember);
+        }
+    }

    if (!labs.isSet('emailErrors') && !!(config.get('bulkEmail') && config.get('bulkEmail').mailgun)) {
        if (jsonModel.status === 'failed') {
--- a/ghost/core/core/server/services/email-service/wrapper.js
+++ b/ghost/core/core/server/services/email-service/wrapper.js
@ -94,6 +94,8 @@ class EmailServiceWrapper {
            sentry
        });

+        this.renderer = emailRenderer;
+
        this.service = new EmailService({
            batchSendingService,
            sendingService,
--- a/ghost/core/test/e2e-api/admin/snapshots/emails.test.js.snap
+++ b/ghost/core/test/e2e-api/admin/snapshots/emails.test.js.snap
@ -753,7 +753,7 @@ Object {
      "reply_to": null,
      "source": null,
      "source_type": "html",
-      "status": "pending",
+      "status": "failed",
      "subject": "You got mailed! Again!",
      "submitted_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
      "track_clicks": false,
@ -769,7 +769,55 @@ exports[`Emails API Can retry a failed email 2: [headers] 1`] = `
 Object {
  "access-control-allow-origin": "http://127.0.0.1:2369",
  "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "688",
+  "content-length": "687",
+  "content-type": "application/json; charset=utf-8",
+  "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
+  "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
+  "vary": "Accept-Version, Origin, Accept-Encoding",
+  "x-powered-by": "Express",
+}
+`;
+
+exports[`Emails API Does default replacements on the HTML body of an old email 1: [body] 1`] = `
+Object {
+  "emails": Array [
+    Object {
+      "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
+      "delivered_count": 0,
+      "email_count": 1,
+      "error": null,
+      "error_data": null,
+      "failed_count": 0,
+      "feedback_enabled": false,
+      "from": "support@example.com",
+      "html": "<p style=\\"margin: 0 0 1.5em 0; line-height: 1.6em;\\">Hey Jamie, Hey Jamie,</p><a href=\\"http://127.0.0.1:2369/unsubscribe/?uuid=example-uuid&newsletter=preview\\">Unsubscribe</a>",
+      "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
+      "newsletter_id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
+      "opened_count": 0,
+      "plaintext": "Hey Jamie, Hey Jamie
+Unsubscribe [http://127.0.0.1:2369/unsubscribe/?uuid=example-uuid&newsletter=preview]",
+      "post_id": "618ba1ffbe2896088840a6e3",
+      "recipient_filter": "all",
+      "reply_to": null,
+      "source": "{}",
+      "source_type": "lexical",
+      "status": "submitted",
+      "subject": "Test email",
+      "submitted_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
+      "track_clicks": false,
+      "track_opens": false,
+      "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
+      "uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
+    },
+  ],
+}
+`;
+
+exports[`Emails API Does default replacements on the HTML body of an old email 2: [headers] 1`] = `
+Object {
+  "access-control-allow-origin": "http://127.0.0.1:2369",
+  "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
+  "content-length": "923",
  "content-type": "application/json; charset=utf-8",
  "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
  "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
--- a/ghost/core/test/e2e-api/admin/emails.test.js
+++ b/ghost/core/test/e2e-api/admin/emails.test.js
@ -3,6 +3,8 @@ const {nullable, anything, anyContentVersion, anyEtag, anyObjectId, anyUuid, any
 const assert = require('assert');
 const sinon = require('sinon');
 const logging = require('@tryghost/logging');
+const jobManager = require('../../../core/server/services/jobs/job-service');
+const models = require('../../../core/server/models');

 const matchEmail = {
    id: anyObjectId,
@ -12,6 +14,11 @@ const matchEmail = {
    submitted_at: anyISODateTime
 };

+const matchEmailNewsletter = {
+    ...matchEmail,
+    newsletter_id: anyObjectId
+};
+
 const matchBatch = {
    id: anyObjectId,
    provider_id: anyString,
@ -30,13 +37,13 @@ describe('Emails API', function () {

    before(async function () {
        agent = await agentProvider.getAdminAPIAgent();
-        await fixtureManager.init('posts', 'members', 'members:emails:failed');
+        await fixtureManager.init('posts', 'newsletters', 'members', 'members:emails:failed');
        await agent.loginAsOwner();
    });

    beforeEach(function () {
        mockManager.mockEvents();
-        mockManager.mockLabsDisabled('emailStability');
+        mockManager.mockMailgun();
    });

    afterEach(function () {
@ -82,26 +89,10 @@ describe('Emails API', function () {
                etag: anyEtag
            });

+        await jobManager.allSettled();
        mockManager.assert.emittedEvent('email.edited');
    });

-    it('Errors when retrying an email that was successful', async function () {
-        const loggingStub = sinon.stub(logging, 'error');
-        await agent
-            .put(`emails/${fixtureManager.get('emails', 0).id}/retry`)
-            .expectStatus(400)
-            .matchBodySnapshot({
-                errors: [{
-                    id: anyErrorId
-                }]
-            })
-            .matchHeaderSnapshot({
-                'content-version': anyContentVersion,
-                etag: anyEtag
-            });
-        sinon.assert.calledOnce(loggingStub);
-    });
-
    it('Can browse email batches', async function () {
        await agent
            .get(`emails/${fixtureManager.get('emails', 0).id}/batches/`)
@ -195,4 +186,45 @@ describe('Emails API', function () {
                etag: anyEtag
            });
    });
+
+    // Older Ghost emails still have a html body and plaintext body set.
+    it('Does default replacements on the HTML body of an old email', async function () {
+        const html = '<p style="margin: 0 0 1.5em 0; line-height: 1.6em;">Hey %%{first_name, &quot;there&quot;}%%, Hey %%{first_name}%%,</p><a href="%%{unsubscribe_url}%%">Unsubscribe</a>';
+        const plaintext = 'Hey %%{first_name, "there"}%%, Hey %%{first_name}%%\nUnsubscribe [%%{unsubscribe_url}%%]';
+
+        // Create this email model in the database
+        const email = await models.Email.add({
+            post_id: fixtureManager.get('posts', 2).id,
+            newsletter_id: fixtureManager.get('newsletters', 0).id,
+            status: 'submitted',
+            submitted_at: new Date(),
+            track_opens: false,
+            track_clicks: false,
+            feedback_enabled: false,
+            recipient_filter: 'all',
+            subject: 'Test email',
+            from: 'support@example.com',
+            replyTo: null,
+            email_count: 1,
+            source: '{}',
+            source_type: 'lexical',
+            html,
+            plaintext
+        });
+
+        const {body} = await agent
+            .get(`emails/${email.id}/`)
+            .expectStatus(200)
+            .matchBodySnapshot({
+                emails: [matchEmailNewsletter]
+            })
+            .matchHeaderSnapshot({
+                'content-version': anyContentVersion,
+                etag: anyEtag
+            });
+
+        // Simple check that there are not %%{ leftover (in case the snapshots gets updated without noticing what this test is checking)
+        assert.equal(body.emails[0].html.includes('%%{'), false);
+        assert.equal(body.emails[0].plaintext.includes('%%{'), false);
+    });
 });
--- a/ghost/core/test/integration/services/email-service/batch-sending.test.js
+++ b/ghost/core/test/integration/services/email-service/batch-sending.test.js
@ -14,10 +14,12 @@ const mobileDocWithFreeMemberOnly = '{"version":"0.3.1","atoms":[],"cards":[["em
 const mobileDocWithPaidMemberOnly = '{"version":"0.3.1","atoms":[],"cards":[["email-cta",{"showButton":false,"showDividers":true,"segment":"status:-free","alignment":"left","html":"<p>This is for paid members only</p>"}]],"markups":[],"sections":[[1,"p",[[0,[],0,"Hello world"]]],[10,0],[1,"p",[[0,[],0,"Bye."]]]],"ghostVersion":"4.0"}';
 const mobileDocWithPaidAndFreeMemberOnly = '{"version":"0.3.1","atoms":[],"cards":[["email-cta",{"showButton":false,"showDividers":true,"segment":"status:free","alignment":"left","html":"<p>This is for free members only</p>"}],["email-cta",{"showButton":false,"showDividers":true,"segment":"status:-free","alignment":"left","html":"<p>This is for paid members only</p>"}]],"markups":[],"sections":[[1,"p",[[0,[],0,"Hello world"]]],[10,0],[10,1],[1,"p",[[0,[],0,"Bye."]]]],"ghostVersion":"4.0"}';
 const mobileDocWithFreeMemberOnlyAndPaywall = '{"version":"0.3.1","atoms":[],"cards":[["email-cta",{"showButton":false,"showDividers":true,"segment":"status:free","alignment":"left","html":"<p>This is for free members only</p>"}],["paywall",{}]],"markups":[],"sections":[[1,"p",[[0,[],0,"Hello world"]]],[10,0],[1,"p",[[0,[],0,"Bye."]]],[10,1],[1,"p",[[0,[],0,"This is after the paywall."]]]],"ghostVersion":"4.0"}';
+const mobileDocWithReplacements = '{"version":"0.3.1","atoms":[],"cards":[["email",{"html":"<p>Hey {first_name, \\"there\\"}, Hey {first_name},</p>"}]],"markups":[],"sections":[[1,"p",[[0,[],0,"Hello {first_name},"]]],[10,0]],"ghostVersion":"4.0"}';

 const configUtils = require('../../../utils/configUtils');
 const {settingsCache} = require('../../../../core/server/services/settings-helpers');
 const DomainEvents = require('@tryghost/domain-events');
+const emailService = require('../../../../core/server/services/email-service');

 let agent;
 let stubbedSend;
@ -183,17 +185,18 @@ describe('Batch sending tests', function () {
        });
        mockManager.mockMail();
        mockManager.mockMailgun(function () {
+            // Allows for setting stubbedSend during tests
            return stubbedSend.call(this, ...arguments);
        });
    });

    afterEach(async function () {
-        mockManager.restore();
        await configUtils.restore();
        await models.Settings.edit([{
            key: 'email_verification_required',
            value: false
        }], {context: {internal: true}});
+        mockManager.restore();
    });

    before(async function () {
@ -213,12 +216,12 @@ describe('Batch sending tests', function () {
    });

    after(async function () {
+        mockManager.restore();
        await ghostServer.stop();
    });

    it('Can send a scheduled post email', async function () {
        // Prepare a post and email model
-        const completedPromise = jobManager.awaitCompletion('batch-sending-service-job');
        const emailModel = await createPublishedPostEmail();

        assert.equal(emailModel.get('source_type'), 'mobiledoc');
@ -226,10 +229,10 @@ describe('Batch sending tests', function () {
        assert(emailModel.get('from'));

        // Await sending job
-        await completedPromise;
+        await jobManager.allSettled();

        await emailModel.refresh();
-        assert.equal(emailModel.get('status'), 'submitted', 'This email should be in submitted state: ' + (emailModel.get('error') ?? 'No error'));
+        assert.equal(emailModel.get('status'), 'submitted');
        assert.equal(emailModel.get('email_count'), 4);

        // Did we create batches?
@ -260,6 +263,33 @@ describe('Batch sending tests', function () {
        assert.equal(memberIds.length, _.uniq(memberIds).length);
    });

+    it('Protects the email job from being run multiple times at the same time', async function () {
+        // Prepare a post and email model
+        const emailModel = await createPublishedPostEmail();
+
+        assert.equal(emailModel.get('source_type'), 'mobiledoc');
+        assert(emailModel.get('subject'));
+        assert(emailModel.get('from'));
+
+        // Retry sending a couple of times
+        const promises = [];
+        for (let i = 0; i < 100; i++) {
+            promises.push(emailService.service.retryEmail(emailModel));
+        }
+        await Promise.all(promises);
+
+        // Await sending job
+        await jobManager.allSettled();
+
+        await emailModel.refresh();
+        assert.equal(emailModel.get('status'), 'submitted');
+        assert.equal(emailModel.get('email_count'), 4);
+
+        // Did we create batches?
+        const batches = await models.EmailBatch.findAll({filter: `email_id:${emailModel.id}`});
+        assert.equal(batches.models.length, 1);
+    });
+
    it('Doesn\'t include members created after the email in the batches', async function () {
        // If we create a new member (e.g. a member that was imported) after the email was created, they should not be included in the email
        const addStub = sinon.stub(models.Email, 'add');
@ -482,7 +512,6 @@ describe('Batch sending tests', function () {
        };

        // Prepare a post and email model
-        let completedPromise = jobManager.awaitCompletion('batch-sending-service-job');
        const emailModel = await createPublishedPostEmail();

        assert.equal(emailModel.get('source_type'), 'mobiledoc');
@ -490,7 +519,7 @@ describe('Batch sending tests', function () {
        assert(emailModel.get('from'));

        // Await sending job
-        await completedPromise;
+        await jobManager.allSettled();

        await emailModel.refresh();
        assert.equal(emailModel.get('status'), 'failed');
@ -545,9 +574,8 @@ describe('Batch sending tests', function () {
        let memberIds = emailRecipients.map(recipient => recipient.get('member_id'));
        assert.equal(memberIds.length, _.uniq(memberIds).length);

-        completedPromise = jobManager.awaitCompletion('batch-sending-service-job');
        await retryEmail(emailModel.id);
-        await completedPromise;
+        await jobManager.allSettled();

        await emailModel.refresh();
        batches = await models.EmailBatch.findAll({filter: `email_id:${emailModel.id}`});
@ -732,6 +760,67 @@ describe('Batch sending tests', function () {
        });
    });

+    describe('Replacements', function () {
+        it('Does replace with and without fallback in both plaintext and html for member without name', async function () {
+            // Create a new member without a first_name
+            await models.Member.add({
+                email: 'replacements-test-1@example.com',
+                labels: [{name: 'replacements-tests'}],
+                newsletters: [{
+                    id: fixtureManager.get('newsletters', 0).id
+                }]
+            });
+
+            const {html, plaintext} = await sendEmail({
+                mobiledoc: mobileDocWithReplacements
+            }, 'label:replacements-tests');
+
+            // Outside the email card, {first_name} is not replaced
+            assert.match(html, /Hello {first_name},/);
+
+            // Inside the email card with and without fallback, it is replaced
+            assert.match(html, /Hey there, Hey ,/);
+
+            // The unsubscribe link is replaced
+            assert.match(html, /<a href="http:\/\/127.0.0.1:2369\/unsubscribe\/\?uuid=[a-z0-9-]+&newsletter=[a-z0-9-]+"/, 'Unsubscribe link not found in html');
+
+            // Same for plaintext:
+            assert.match(plaintext, /Hello {first_name},/);
+            assert.match(plaintext, /Hey there, Hey ,/);
+            assert.match(plaintext, /\[http:\/\/127.0.0.1:2369\/unsubscribe\/\?uuid=[a-z0-9-]+&newsletter=[a-z0-9-]+\]/, 'Unsubscribe link not found in plaintext');
+        });
+
+        it('Does replace with and without fallback in both plaintext and html for member with name', async function () {
+            // Create a new member without a first_name
+            await models.Member.add({
+                name: 'Simon Tester',
+                email: 'replacements-test-2@example.com',
+                labels: [{name: 'replacements-tests-2'}],
+                newsletters: [{
+                    id: fixtureManager.get('newsletters', 0).id
+                }]
+            });
+
+            const {html, plaintext} = await sendEmail({
+                mobiledoc: mobileDocWithReplacements
+            }, 'label:replacements-tests-2');
+
+            // Outside the email card, {first_name} is not replaced
+            assert.match(html, /Hello {first_name},/);
+
+            // Inside the email card with and without fallback, it is replaced
+            assert.match(html, /Hey Simon, Hey Simon,/);
+
+            // The unsubscribe link is replaced
+            assert.match(html, /<a href="http:\/\/127.0.0.1:2369\/unsubscribe\/\?uuid=[a-z0-9-]+&newsletter=[a-z0-9-]+"/, 'Unsubscribe link not found in html');
+
+            // Same for plaintext:
+            assert.match(plaintext, /Hello {first_name},/);
+            assert.match(plaintext, /Hey Simon, Hey Simon,/);
+            assert.match(plaintext, /\[http:\/\/127.0.0.1:2369\/unsubscribe\/\?uuid=[a-z0-9-]+&newsletter=[a-z0-9-]+\]/, 'Unsubscribe link not found in plaintext');
+        });
+    });
+
    describe('HTML-content', function () {
        it('Does not HTML escape feature_image_caption', async function () {
            const {html, plaintext} = await sendEmail({
@ -745,6 +834,4 @@ describe('Batch sending tests', function () {
            assert.match(plaintext, /Testing feature image caption/);
        });
    });
-
-    // TODO: Replacement fallbacks
 });
--- a/ghost/email-service/lib/email-renderer.js
+++ b/ghost/email-service/lib/email-renderer.js
@ -294,7 +294,7 @@ class EmailRenderer {
        html = $.html(); // () Fix for vscode syntax highlighter

        // Replacement strings
-        const replacementDefinitions = this.buildReplacementDefinitions({html, newsletter});
+        const replacementDefinitions = this.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});

        // TODO: normalizeReplacementStrings (replace unsupported replacement strings)

@ -316,7 +316,6 @@ class EmailRenderer {
    }

    /**
-     * @private
     * createUnsubscribeUrl
     *
     * Takes a member and newsletter uuid. Returns the url that should be used to unsubscribe
@ -347,16 +346,15 @@ class EmailRenderer {
    }

    /**
-     * @private
     * Note that we only look in HTML because plaintext and HTML are essentially the same content
     * @returns {ReplacementDefinition[]}
     */
-    buildReplacementDefinitions({html, newsletter}) {
+    buildReplacementDefinitions({html, newsletterUuid}) {
        const baseDefinitions = [
            {
                id: 'unsubscribe_url',
                getValue: (member) => {
-                    return this.createUnsubscribeUrl(member.uuid, {newsletterUuid: newsletter.get('uuid')});
+                    return this.createUnsubscribeUrl(member.uuid, {newsletterUuid});
                }
            },
            {
@ -402,7 +400,7 @@ class EmailRenderer {
                    replacements.push({
                        id: replacementStr,
                        originalId: recipientProperty,
-                        token: new RegExp(escapeRegExp(replacementMatch), 'g'),
+                        token: new RegExp(escapeRegExp(replacementMatch).replace(/(?:"|&quot;)/g, '(?:"|&quot;)'), 'g'),
                        getValue: fallback ? (member => definition.getValue(member) || fallback) : definition.getValue
                    });
                }
--- a/ghost/email-service/lib/email-service.js
+++ b/ghost/email-service/lib/email-service.js
@ -161,6 +161,21 @@ class EmailService {
        return email;
    }

+    /**
+     * @return {import('./email-renderer').MemberLike}
+     */
+    getDefaultExampleMember() {
+        /**
+         * @type {import('./email-renderer').MemberLike}
+         */
+        return {
+            id: 'example-id',
+            uuid: 'example-uuid',
+            email: 'jamie@example.com',
+            name: 'Jamie Larson'
+        };
+    }
+
    /**
     * @private
     * @param {string} [email] (optional) Search for a member with this email address and use it as the example. If not found, defaults to the default but still uses the provided email address.
@ -170,12 +185,7 @@ class EmailService {
        /**
         * @type {import('./email-renderer').MemberLike}
         */
-        const exampleMember = {
-            id: 'example-id',
-            uuid: 'example-uuid',
-            email: 'jamie@example.com',
-            name: 'Jamie Larson'
-        };
+        const exampleMember = this.getDefaultExampleMember();

        // fetch any matching members so that replacements use expected values
        if (email) {
@ -194,6 +204,22 @@ class EmailService {
        return exampleMember;
    }

+    /**
+     * Do a manual replacement of tokens with values for a member (normally only used for previews)
+     *
+     * @param {string} htmlOrPlaintext
+     * @param {import('./email-renderer').ReplacementDefinition[]} replacements
+     * @param {import('./email-renderer').MemberLike} member
+     * @return {string}
+     */
+    replaceDefinitions(htmlOrPlaintext, replacements, member) {
+        // Do manual replacements with an example member
+        for (const replacement of replacements) {
+            htmlOrPlaintext = htmlOrPlaintext.replace(replacement.token, replacement.getValue(member));
+        }
+        return htmlOrPlaintext;
+    }
+
    /**
     *
     * @param {*} post
@ -207,16 +233,10 @@ class EmailService {
        const subject = this.#emailRenderer.getSubject(post);
        let {html, plaintext, replacements} = await this.#emailRenderer.renderBody(post, newsletter, segment, {clickTrackingEnabled: false});

-        // Do manual replacements with an example member
-        for (const replacement of replacements) {
-            html = html.replace(replacement.token, replacement.getValue(exampleMember));
-            plaintext = plaintext.replace(replacement.token, replacement.getValue(exampleMember));
-        }
-
        return {
            subject,
-            html,
-            plaintext
+            html: this.replaceDefinitions(html, replacements, exampleMember),
+            plaintext: this.replaceDefinitions(plaintext, replacements, exampleMember)
        };
    }

--- a/ghost/email-service/package.json
+++ b/ghost/email-service/package.json
@ -21,7 +21,8 @@
    "c8": "7.13.0",
    "mocha": "10.2.0",
    "should": "13.2.3",
-    "sinon": "15.0.1"
+    "sinon": "15.0.1",
+    "html-validate": "7.13.2"
  },
  "dependencies": {
    "@tryghost/color-utils": "0.1.22",
--- a/ghost/email-service/test/email-renderer.test.js
+++ b/ghost/email-service/test/email-renderer.test.js
@ -5,6 +5,60 @@ const {createModel} = require('./utils');
 const linkReplacer = require('@tryghost/link-replacer');
 const sinon = require('sinon');
 const logging = require('@tryghost/logging');
+const {HtmlValidate} = require('html-validate');
+
+function validateHtml(html) {
+    const htmlvalidate = new HtmlValidate({
+        extends: [
+            'html-validate:document',
+            'html-validate:standard'
+        ],
+        rules: {
+            // We need deprecated attrs for legacy tables in older email clients
+            'no-deprecated-attr': 'off',
+
+            // Don't care that the first <hx> isn't <h1>
+            'heading-level': 'off'
+        },
+        elements: [
+            'html5',
+            // By default, html-validate requires the 'lang' attribute on the <html> tag. We don't really want that for now.
+            {
+                html: {
+                    attributes: {
+                        lang: {
+                            required: false
+                        }
+                    }
+                }
+            }
+        ]
+    });
+    const report = htmlvalidate.validateString(html);
+
+    // Improve debugging and show a snippet of the invalid HTML instead of just the line number or a huge HTML-dump
+    const parsedErrors = [];
+
+    if (!report.valid) {
+        const lines = html.split('\n');
+        const messages = report.results[0].messages;
+
+        for (const item of messages) {
+            if (item.severity !== 2) {
+                // Ignore warnings
+                continue;
+            }
+            const start = Math.max(item.line - 4, 0);
+            const end = Math.min(item.line + 4, lines.length - 1);
+
+            const _html = lines.slice(start, end).map(l => l.trim()).join('\n');
+            parsedErrors.push(`${item.ruleId}: ${item.message}\n   At line ${item.line}, col ${item.column}\n   HTML-snippet:\n${_html}`);
+        }
+    }
+
+    // Fail if invalid HTML
+    assert.equal(report.valid, true, 'Expected valid HTML without warnings, got errors:\n' + parsedErrors.join('\n\n'));
+}

 describe('Email renderer', function () {
    let logStub;
@ -41,13 +95,13 @@ describe('Email renderer', function () {

        it('returns an empty list of replacements if nothing is used', function () {
            const html = 'Hello world';
-            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletter});
+            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});
            assert.equal(replacements.length, 0);
        });

        it('returns a replacement if it is used', function () {
            const html = 'Hello world %%{uuid}%%';
-            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletter});
+            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});
            assert.equal(replacements.length, 1);
            assert.equal(replacements[0].token.toString(), '/%%\\{uuid\\}%%/g');
            assert.equal(replacements[0].id, 'uuid');
@ -56,7 +110,7 @@ describe('Email renderer', function () {

        it('returns a replacement only once if used multiple times', function () {
            const html = 'Hello world %%{uuid}%% And %%{uuid}%%';
-            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletter});
+            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});
            assert.equal(replacements.length, 1);
            assert.equal(replacements[0].token.toString(), '/%%\\{uuid\\}%%/g');
            assert.equal(replacements[0].id, 'uuid');
@ -65,7 +119,7 @@ describe('Email renderer', function () {

        it('returns correct first name', function () {
            const html = 'Hello %%{first_name}%%,';
-            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletter});
+            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});
            assert.equal(replacements.length, 1);
            assert.equal(replacements[0].token.toString(), '/%%\\{first_name\\}%%/g');
            assert.equal(replacements[0].id, 'first_name');
@ -74,7 +128,7 @@ describe('Email renderer', function () {

        it('returns correct unsubscribe url', function () {
            const html = 'Hello %%{unsubscribe_url}%%,';
-            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletter});
+            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});
            assert.equal(replacements.length, 1);
            assert.equal(replacements[0].token.toString(), '/%%\\{unsubscribe_url\\}%%/g');
            assert.equal(replacements[0].id, 'unsubscribe_url');
@ -83,9 +137,9 @@ describe('Email renderer', function () {

        it('supports fallback values', function () {
            const html = 'Hey %%{first_name, "there"}%%,';
-            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletter});
+            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});
            assert.equal(replacements.length, 1);
-            assert.equal(replacements[0].token.toString(), '/%%\\{first_name, "there"\\}%%/g');
+            assert.equal(replacements[0].token.toString(), '/%%\\{first_name, (?:"|&quot;)there(?:"|&quot;)\\}%%/g');
            assert.equal(replacements[0].id, 'first_name_2');
            assert.equal(replacements[0].getValue(member), 'Test');

@ -95,16 +149,16 @@ describe('Email renderer', function () {

        it('supports combination of multiple fallback values', function () {
            const html = 'Hey %%{first_name, "there"}%%, %%{first_name, "member"}%% %%{first_name}%% %%{first_name, "there"}%%';
-            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletter});
+            const replacements = emailRenderer.buildReplacementDefinitions({html, newsletterUuid: newsletter.get('uuid')});
            assert.equal(replacements.length, 3);
-            assert.equal(replacements[0].token.toString(), '/%%\\{first_name, "there"\\}%%/g');
+            assert.equal(replacements[0].token.toString(), '/%%\\{first_name, (?:"|&quot;)there(?:"|&quot;)\\}%%/g');
            assert.equal(replacements[0].id, 'first_name_2');
            assert.equal(replacements[0].getValue(member), 'Test');

            // In case of empty name
            assert.equal(replacements[0].getValue({name: ''}), 'there');

-            assert.equal(replacements[1].token.toString(), '/%%\\{first_name, "member"\\}%%/g');
+            assert.equal(replacements[1].token.toString(), '/%%\\{first_name, (?:"|&quot;)member(?:"|&quot;)\\}%%/g');
            assert.equal(replacements[1].id, 'first_name_3');
            assert.equal(replacements[1].getValue(member), 'Test');

@ -341,6 +395,8 @@ describe('Email renderer', function () {

    describe('renderBody', function () {
        let renderedPost = '<p>Lexical Test</p>';
+        let postUrl = 'http://example.com';
+        let customSettings = {};
        let emailRenderer = new EmailRenderer({
            audienceFeedbackService: {
                buildLink: (_uuid, _postId, score) => {
@ -360,6 +416,9 @@ describe('Email renderer', function () {
            },
            settingsCache: {
                get: (key) => {
+                    if (customSettings[key]) {
+                        return customSettings[key];
+                    }
                    if (key === 'accent_color') {
                        return '#ffffff';
                    }
@ -375,7 +434,7 @@ describe('Email renderer', function () {
                }
            },
            getPostUrl: () => {
-                return 'http://example.com';
+                return postUrl;
            },
            renderers: {
                lexical: {
@ -430,6 +489,8 @@ describe('Email renderer', function () {
                }),
                loaded: ['posts_meta']
            };
+            postUrl = 'http://example.com';
+            customSettings = {};
        });

        it('returns feedback buttons and unsubcribe links', async function () {
@ -788,6 +849,60 @@ describe('Email renderer', function () {
            responsePaid.html.should.containEql('finishing part only for members');
            responsePaid.html.should.not.containEql('Become a paid member of Test Blog to get access to all');
        });
+
+        it('should output valid HTML and escape HTML characters in mobiledoc', async function () {
+            const post = createModel({
+                ...basePost,
+                title: 'This is\' a blog po"st test <3</body>',
+                excerpt: 'This is a blog post test <3</body>',
+                authors: [
+                    createModel({
+                        name: 'This is a blog post test <3</body>'
+                    })
+                ],
+                posts_meta: createModel({
+                    feature_image_alt: 'This is a blog post test <3</body>',
+                    feature_image_caption: 'This is escaped in the frontend'
+                })
+            });
+            postUrl = 'https://testpost.com/t&es<3t-post"</body>/';
+            customSettings = {
+                icon: 'icon2<3</body>'
+            };
+
+            const newsletter = createModel({
+                feedback_enabled: true,
+                name: 'My newsletter <3</body>',
+                header_image: 'https://testpost.com/test-post</body>/',
+                show_header_icon: true,
+                show_header_title: true,
+                show_feature_image: true,
+                title_font_category: 'sans-serif',
+                title_alignment: 'center',
+                body_font_category: 'serif',
+                show_badge: true,
+                show_header_name: true,
+                // Note: we don't need to check the footer content because this should contain valid HTML (not text)
+                footer_content: '<span>Footer content with valid HTML</span>'
+            });
+            const segment = null;
+            const options = {};
+
+            const response = await emailRenderer.renderBody(
+                post,
+                newsletter,
+                segment,
+                options
+            );
+
+            validateHtml(response.html);
+
+            // Check footer content is not escaped
+            assert.equal(response.html.includes('<span>Footer content with valid HTML</span>'), true, 'Should include footer content without escaping');
+
+            // Check doesn't contain the non escaped string '<3'
+            assert.equal(response.html.includes('<3'), false, 'Should escape HTML characters');
+        });
    });

    describe('getTemplateData', function () {