From 0fb7abae87408bab2dd9a9c1c22ab793e55430f3 Mon Sep 17 00:00:00 2001 From: Rishabh Garg Date: Tue, 28 Mar 2023 15:59:15 +0530 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20LinkReplacer=20bug=20cau?= =?UTF-8?q?sing=20broken=20links=20on=20published=20post/page=20(#16514)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs TryGhost/Team#2840 - moves the `entities.decode()` step to the `LinkReplacer` class so that it's applied to all links, not just the ones that are replaced in the email service - adds a test case to `LinkReplacer` to ensure that the `entities.decode()` step is applied to all links correctly, decoding any URLs with HTML entities in them --------- Co-authored-by: Chris Raible --- ghost/email-service/lib/email-renderer.js | 3 --- ghost/email-service/package.json | 1 - ghost/link-replacer/lib/LinkReplacer.js | 3 ++- ghost/link-replacer/package.json | 3 ++- ghost/link-replacer/test/LinkReplacer.test.js | 7 +++++++ yarn.lock | 10 +++++----- 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ghost/email-service/lib/email-renderer.js b/ghost/email-service/lib/email-renderer.js index 422b77519f..fde383874b 100644 --- a/ghost/email-service/lib/email-renderer.js +++ b/ghost/email-service/lib/email-renderer.js @@ -8,7 +8,6 @@ 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: { @@ -293,8 +292,6 @@ 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) { diff --git a/ghost/email-service/package.json b/ghost/email-service/package.json index 5c497041c9..8d53eb6b68 100644 --- a/ghost/email-service/package.json +++ b/ghost/email-service/package.json @@ -35,7 +35,6 @@ "@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" diff --git a/ghost/link-replacer/lib/LinkReplacer.js b/ghost/link-replacer/lib/LinkReplacer.js index ef31fe08a6..efcf444f2e 100644 --- a/ghost/link-replacer/lib/LinkReplacer.js +++ b/ghost/link-replacer/lib/LinkReplacer.js @@ -7,6 +7,7 @@ class LinkReplacer { */ async replace(html, replaceLink) { const cheerio = require('cheerio'); + const entities = require('entities'); try { const $ = cheerio.load(html, { xml: { @@ -22,7 +23,7 @@ class LinkReplacer { if (href) { let url; try { - url = new URL(href); + url = new URL(entities.decode(href)); } catch (e) { // Ignore invalid URLs } diff --git a/ghost/link-replacer/package.json b/ghost/link-replacer/package.json index 9722e54440..cc7a42bd83 100644 --- a/ghost/link-replacer/package.json +++ b/ghost/link-replacer/package.json @@ -24,6 +24,7 @@ "sinon": "15.0.2" }, "dependencies": { - "cheerio": "0.22.0" + "cheerio": "0.22.0", + "entities": "^4.4.0" } } diff --git a/ghost/link-replacer/test/LinkReplacer.test.js b/ghost/link-replacer/test/LinkReplacer.test.js index 13490b9628..0c0f78dbb8 100644 --- a/ghost/link-replacer/test/LinkReplacer.test.js +++ b/ghost/link-replacer/test/LinkReplacer.test.js @@ -36,6 +36,13 @@ describe('LinkReplacementService', function () { assert.equal(replaced, html); }); + it('Does escape HTML characters within a link\'s href', async function () { + const html = 'link'; + const expected = 'link'; + const replaced = await linkReplacer.replace(html, url => new URL(url)); + assert.equal(replaced, expected); + }); + it('Can replace to string', async function () { const html = 'link'; const expected = 'link'; diff --git a/yarn.lock b/yarn.lock index 7b3b297dbc..91af9bf6f7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13838,11 +13838,6 @@ 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" @@ -13853,6 +13848,11 @@ 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"