mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-03-11 02:12:21 -05:00
🐛 Fixed broken link tracking in newsletters (#16473)
refs https://github.com/TryGhost/Team/issues/2805 When we render mobiledoc to HTML, it automatically escapes HTML entities in the process, so a button or directly pasted link with href="https://example.com?code=test" will be rendered as href="https://example.com?code=test" as the url is encoded in the rendered HTML. Our link tracking was using the encoded URL as the redirect URL in newsletters, causing certain links to break. This change updates the link tracking to decode the URL with `entities.decode(url)` so we store the correct redirect URL in our DB and ensure link tracking redirects to the correct url from newsletters. --------- Co-authored-by: Rishabh <zrishabhgarg@gmail.com>
This commit is contained in:
parent
8856822092
commit
64c9e66b56
4 changed files with 152 additions and 82 deletions
|
@ -8,6 +8,7 @@ const {textColorForBackgroundColor, darkenToContrastThreshold} = require('@trygh
|
|||
const {DateTime} = require('luxon');
|
||||
const htmlToPlaintext = require('@tryghost/html-to-plaintext');
|
||||
const tpl = require('@tryghost/tpl');
|
||||
const entities = require('entities');
|
||||
|
||||
const messages = {
|
||||
subscriptionStatus: {
|
||||
|
@ -292,6 +293,8 @@ class EmailRenderer {
|
|||
// Link tracking
|
||||
if (options.clickTrackingEnabled) {
|
||||
html = await this.#linkReplacer.replace(html, async (url) => {
|
||||
// Decode any escaped entities in the url
|
||||
url = new URL(entities.decode(url.toString()));
|
||||
// We ignore all links that contain %%{uuid}%%
|
||||
// because otherwise we would add tracking to links that need to be replaced first
|
||||
if (url.toString().indexOf('%%{uuid}%%') !== -1) {
|
||||
|
|
|
@ -19,10 +19,10 @@
|
|||
],
|
||||
"devDependencies": {
|
||||
"c8": "7.13.0",
|
||||
"html-validate": "7.13.3",
|
||||
"mocha": "10.2.0",
|
||||
"should": "13.2.3",
|
||||
"sinon": "15.0.2",
|
||||
"html-validate": "7.13.3"
|
||||
"sinon": "15.0.2"
|
||||
},
|
||||
"dependencies": {
|
||||
"@tryghost/color-utils": "0.1.23",
|
||||
|
@ -35,6 +35,7 @@
|
|||
"@tryghost/validator": "^0.2.0",
|
||||
"bson-objectid": "2.0.4",
|
||||
"cheerio": "0.22.0",
|
||||
"entities": "4.4.0",
|
||||
"handlebars": "4.7.7",
|
||||
"juice": "8.1.0",
|
||||
"moment-timezone": "0.5.23"
|
||||
|
|
|
@ -858,82 +858,9 @@ describe('Email renderer', function () {
|
|||
let renderedPost = '<p>Lexical Test</p>';
|
||||
let postUrl = 'http://example.com';
|
||||
let customSettings = {};
|
||||
let emailRenderer = new EmailRenderer({
|
||||
audienceFeedbackService: {
|
||||
buildLink: (_uuid, _postId, score) => {
|
||||
return new URL('http://feedback-link.com/?score=' + encodeURIComponent(score) + '&uuid=' + encodeURIComponent(_uuid));
|
||||
}
|
||||
},
|
||||
urlUtils: {
|
||||
urlFor: (type) => {
|
||||
if (type === 'image') {
|
||||
return 'http://icon.example.com';
|
||||
}
|
||||
return 'http://example.com/subdirectory';
|
||||
},
|
||||
isSiteUrl: (u) => {
|
||||
return u.hostname === 'example.com';
|
||||
}
|
||||
},
|
||||
settingsCache: {
|
||||
get: (key) => {
|
||||
if (customSettings[key]) {
|
||||
return customSettings[key];
|
||||
}
|
||||
if (key === 'accent_color') {
|
||||
return '#ffffff';
|
||||
}
|
||||
if (key === 'timezone') {
|
||||
return 'Etc/UTC';
|
||||
}
|
||||
if (key === 'title') {
|
||||
return 'Test Blog';
|
||||
}
|
||||
if (key === 'icon') {
|
||||
return 'ICON';
|
||||
}
|
||||
}
|
||||
},
|
||||
getPostUrl: () => {
|
||||
return postUrl;
|
||||
},
|
||||
renderers: {
|
||||
lexical: {
|
||||
render: () => {
|
||||
return renderedPost;
|
||||
}
|
||||
},
|
||||
mobiledoc: {
|
||||
render: () => {
|
||||
return '<p> Mobiledoc Test</p>';
|
||||
}
|
||||
}
|
||||
},
|
||||
linkReplacer,
|
||||
memberAttributionService: {
|
||||
addPostAttributionTracking: (u) => {
|
||||
u.searchParams.append('post_tracking', 'added');
|
||||
return u;
|
||||
}
|
||||
},
|
||||
linkTracking: {
|
||||
service: {
|
||||
addTrackingToUrl: (u, _post, uuid) => {
|
||||
return new URL('http://tracked-link.com/?m=' + encodeURIComponent(uuid) + '&url=' + encodeURIComponent(u.href));
|
||||
}
|
||||
}
|
||||
},
|
||||
outboundLinkTagger: {
|
||||
addToUrl: (u, newsletter) => {
|
||||
u.searchParams.append('source_tracking', newsletter?.get('name') ?? 'site');
|
||||
return u;
|
||||
}
|
||||
},
|
||||
labs: {
|
||||
isSet: () => true
|
||||
}
|
||||
});
|
||||
let emailRenderer;
|
||||
let basePost;
|
||||
let addTrackingToUrlStub;
|
||||
|
||||
beforeEach(function () {
|
||||
basePost = {
|
||||
|
@ -955,6 +882,83 @@ describe('Email renderer', function () {
|
|||
};
|
||||
postUrl = 'http://example.com';
|
||||
customSettings = {};
|
||||
addTrackingToUrlStub = sinon.stub();
|
||||
addTrackingToUrlStub.callsFake((u, _post, uuid) => {
|
||||
return new URL('http://tracked-link.com/?m=' + encodeURIComponent(uuid) + '&url=' + encodeURIComponent(u.href));
|
||||
});
|
||||
emailRenderer = new EmailRenderer({
|
||||
audienceFeedbackService: {
|
||||
buildLink: (_uuid, _postId, score) => {
|
||||
return new URL('http://feedback-link.com/?score=' + encodeURIComponent(score) + '&uuid=' + encodeURIComponent(_uuid));
|
||||
}
|
||||
},
|
||||
urlUtils: {
|
||||
urlFor: (type) => {
|
||||
if (type === 'image') {
|
||||
return 'http://icon.example.com';
|
||||
}
|
||||
return 'http://example.com/subdirectory';
|
||||
},
|
||||
isSiteUrl: (u) => {
|
||||
return u.hostname === 'example.com';
|
||||
}
|
||||
},
|
||||
settingsCache: {
|
||||
get: (key) => {
|
||||
if (customSettings[key]) {
|
||||
return customSettings[key];
|
||||
}
|
||||
if (key === 'accent_color') {
|
||||
return '#ffffff';
|
||||
}
|
||||
if (key === 'timezone') {
|
||||
return 'Etc/UTC';
|
||||
}
|
||||
if (key === 'title') {
|
||||
return 'Test Blog';
|
||||
}
|
||||
if (key === 'icon') {
|
||||
return 'ICON';
|
||||
}
|
||||
}
|
||||
},
|
||||
getPostUrl: () => {
|
||||
return postUrl;
|
||||
},
|
||||
renderers: {
|
||||
lexical: {
|
||||
render: () => {
|
||||
return renderedPost;
|
||||
}
|
||||
},
|
||||
mobiledoc: {
|
||||
render: () => {
|
||||
return '<p> Mobiledoc Test</p>';
|
||||
}
|
||||
}
|
||||
},
|
||||
linkReplacer,
|
||||
memberAttributionService: {
|
||||
addPostAttributionTracking: (u) => {
|
||||
u.searchParams.append('post_tracking', 'added');
|
||||
return u;
|
||||
}
|
||||
},
|
||||
linkTracking: {
|
||||
service: {
|
||||
addTrackingToUrl: addTrackingToUrlStub
|
||||
}
|
||||
},
|
||||
outboundLinkTagger: {
|
||||
addToUrl: (u, newsletter) => {
|
||||
u.searchParams.append('source_tracking', newsletter?.get('name') ?? 'site');
|
||||
return u;
|
||||
}
|
||||
},
|
||||
labs: {
|
||||
isSet: () => true
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('returns feedback buttons and unsubcribe links', async function () {
|
||||
|
@ -1182,6 +1186,68 @@ describe('Email renderer', function () {
|
|||
clickTrackingEnabled: true
|
||||
};
|
||||
|
||||
renderedPost = '<p>Lexical Test</p><p><a href="https://external-domain.com/?ref=123">Hello</a><a href="https://encoded-link.com?code=test">Hello</a><a href="https://example.com/?ref=123"><img src="example" /></a></p>';
|
||||
|
||||
let response = await emailRenderer.renderBody(
|
||||
post,
|
||||
newsletter,
|
||||
segment,
|
||||
options
|
||||
);
|
||||
|
||||
// Check all links have domain tracked-link.com
|
||||
const $ = cheerio.load(response.html);
|
||||
const links = [];
|
||||
for (const link of $('a').toArray()) {
|
||||
const href = $(link).attr('href');
|
||||
links.push(href);
|
||||
if (href.includes('unsubscribe_url')) {
|
||||
href.should.eql('%%{unsubscribe_url}%%');
|
||||
} else if (href.includes('feedback-link.com')) {
|
||||
href.should.containEql('%%{uuid}%%');
|
||||
} else {
|
||||
href.should.containEql('tracked-link.com');
|
||||
href.should.containEql('m=%%{uuid}%%');
|
||||
}
|
||||
}
|
||||
|
||||
// Update the following array when you make changes to the email template, check if replacements are correct for each newly added link.
|
||||
assert.deepEqual(links, [
|
||||
`http://tracked-link.com/?m=%%{uuid}%%&url=http%3A%2F%2Fexample.com%2F%3Fsource_tracking%3DTest%2BNewsletter%26post_tracking%3Dadded`,
|
||||
`http://tracked-link.com/?m=%%{uuid}%%&url=http%3A%2F%2Fexample.com%2F%3Fsource_tracking%3DTest%2BNewsletter%26post_tracking%3Dadded`,
|
||||
`http://tracked-link.com/?m=%%{uuid}%%&url=https%3A%2F%2Fexternal-domain.com%2F%3Fref%3D123%26source_tracking%3Dsite`,
|
||||
`http://tracked-link.com/?m=%%{uuid}%%&url=https%3A%2F%2Fencoded-link.com%2F%3Fcode%3Dtest%26source_tracking%3Dsite`,
|
||||
`http://tracked-link.com/?m=%%{uuid}%%&url=https%3A%2F%2Fexample.com%2F%3Fref%3D123%26source_tracking%3DTest%2BNewsletter%26post_tracking%3Dadded`,
|
||||
`http://feedback-link.com/?score=1&uuid=%%{uuid}%%`,
|
||||
`http://feedback-link.com/?score=0&uuid=%%{uuid}%%`,
|
||||
`http://feedback-link.com/?score=1&uuid=%%{uuid}%%`,
|
||||
`http://feedback-link.com/?score=0&uuid=%%{uuid}%%`,
|
||||
`%%{unsubscribe_url}%%`,
|
||||
`http://tracked-link.com/?m=%%{uuid}%%&url=https%3A%2F%2Fghost.org%2F%3Fsource_tracking%3Dsite`
|
||||
]);
|
||||
|
||||
// Check uuid in replacements
|
||||
response.replacements.length.should.eql(2);
|
||||
response.replacements[0].id.should.eql('uuid');
|
||||
response.replacements[0].token.should.eql(/%%\{uuid\}%%/g);
|
||||
response.replacements[1].id.should.eql('unsubscribe_url');
|
||||
response.replacements[1].token.should.eql(/%%\{unsubscribe_url\}%%/g);
|
||||
});
|
||||
|
||||
it('handles encoded links', async function () {
|
||||
const post = createModel(basePost);
|
||||
const newsletter = createModel({
|
||||
header_image: null,
|
||||
name: 'Test Newsletter',
|
||||
show_badge: true,
|
||||
feedback_enabled: true,
|
||||
show_post_title_section: true
|
||||
});
|
||||
const segment = null;
|
||||
const options = {
|
||||
clickTrackingEnabled: true
|
||||
};
|
||||
|
||||
renderedPost = '<p>Lexical Test</p><p><a href="https://external-domain.com/?ref=123">Hello</a><a href="https://example.com/?ref=123"><img src="example" /></a></p>';
|
||||
|
||||
let response = await emailRenderer.renderBody(
|
||||
|
|
10
yarn.lock
10
yarn.lock
|
@ -13838,6 +13838,11 @@ ensure-posix-path@^1.0.0, ensure-posix-path@^1.0.1, ensure-posix-path@^1.0.2, en
|
|||
resolved "https://registry.yarnpkg.com/ensure-posix-path/-/ensure-posix-path-1.1.1.tgz#3c62bdb19fa4681544289edb2b382adc029179ce"
|
||||
integrity sha512-VWU0/zXzVbeJNXvME/5EmLuEj2TauvoaTz6aFYK1Z92JCBlDlZ3Gu0tuGR42kpW1754ywTs+QB0g5TP0oj9Zaw==
|
||||
|
||||
entities@4.4.0, entities@^4.2.0, entities@^4.3.0, entities@^4.4.0, entities@~4.4.0:
|
||||
version "4.4.0"
|
||||
resolved "https://registry.yarnpkg.com/entities/-/entities-4.4.0.tgz#97bdaba170339446495e653cfd2db78962900174"
|
||||
integrity sha512-oYp7156SP8LkeGD0GF85ad1X9Ai79WtRsZ2gxJqtBuzH+98YUV6jkHEKlZkMbcrjJjIVJNIDP/3WL9wQkoPbWA==
|
||||
|
||||
entities@^1.1.1, entities@~1.1.1:
|
||||
version "1.1.2"
|
||||
resolved "https://registry.yarnpkg.com/entities/-/entities-1.1.2.tgz#bdfa735299664dfafd34529ed4f8522a275fea56"
|
||||
|
@ -13848,11 +13853,6 @@ entities@^2.0.0:
|
|||
resolved "https://registry.yarnpkg.com/entities/-/entities-2.2.0.tgz#098dc90ebb83d8dffa089d55256b351d34c4da55"
|
||||
integrity sha512-p92if5Nz619I0w+akJrLZH0MX0Pb5DX39XOwQTtXSdQQOaYH03S1uIQp4mhOZtAXrxq4ViO67YTiLBo2638o9A==
|
||||
|
||||
entities@^4.2.0, entities@^4.3.0, entities@^4.4.0, entities@~4.4.0:
|
||||
version "4.4.0"
|
||||
resolved "https://registry.yarnpkg.com/entities/-/entities-4.4.0.tgz#97bdaba170339446495e653cfd2db78962900174"
|
||||
integrity sha512-oYp7156SP8LkeGD0GF85ad1X9Ai79WtRsZ2gxJqtBuzH+98YUV6jkHEKlZkMbcrjJjIVJNIDP/3WL9wQkoPbWA==
|
||||
|
||||
entities@~2.1.0:
|
||||
version "2.1.0"
|
||||
resolved "https://registry.yarnpkg.com/entities/-/entities-2.1.0.tgz#992d3129cf7df6870b96c57858c249a120f8b8b5"
|
||||
|
|
Loading…
Add table
Reference in a new issue