Commit a7f0ad21 authored by Robert Knight's avatar Robert Knight

Fix crash if link href and text cannot be compared

`replaceLinksWithEmbeds` attempts to compare the href and text of a link in a
percent-encoding agnostic way. This can be done by either percent-decoding the
href or percent-encoding the text. Both of these approaches can fail, but
`encodeURI` has fewer failure modes than `decodeURI`. Switch to using
`encodeURI` for this reason, handle failure, and add tests that would cover both
implementation approaches.

Fixes #4405
parent a1e52d60
...@@ -346,7 +346,8 @@ function embedForLink(link) { ...@@ -346,7 +346,8 @@ function embedForLink(link) {
return null; return null;
} }
/** Replace the given link element with an embed. /**
* Replace the given link element with an embed.
* *
* If the given link element is a link to an embeddable media and if its link * If the given link element is a link to an embeddable media and if its link
* text is the same as its href then it will be replaced in the DOM with an * text is the same as its href then it will be replaced in the DOM with an
...@@ -375,12 +376,17 @@ function replaceLinkWithEmbed(link) { ...@@ -375,12 +376,17 @@ function replaceLinkWithEmbed(link) {
// The link's text may or may not be percent encoded. The `link.href` property // The link's text may or may not be percent encoded. The `link.href` property
// will always be percent encoded. When comparing the two we need to be // will always be percent encoded. When comparing the two we need to be
// agnostic as to which representation is used. // agnostic as to which representation is used.
if ( if (link.href !== link.textContent) {
link.href !== link.textContent && try {
decodeURI(link.href) !== link.textContent const encodedText = encodeURI(/** @type {string} */ (link.textContent));
) { if (link.href !== encodedText) {
return null; return null;
}
} catch {
return null;
}
} }
const embed = embedForLink(link); const embed = embedForLink(link);
if (embed) { if (embed) {
/** @type {Element} */ (link.parentElement).replaceChild(embed, link); /** @type {Element} */ (link.parentElement).replaceChild(embed, link);
......
...@@ -363,6 +363,28 @@ describe('sidebar/media-embedder', () => { ...@@ -363,6 +363,28 @@ describe('sidebar/media-embedder', () => {
assert.equal(element.children[0].tagName, 'A'); assert.equal(element.children[0].tagName, 'A');
}); });
[
{
href: 'https://youtu.be/abcd',
// URL that cannot be percent-encoded. Taken from MDN `encodeURI` docs.
text: 'https://youtu.be/abcd\uD800',
},
{
// URL that cannot be percent-decoded. Taken from MDN `decodeURI` docs.
href: 'https://youtu.be/abcd/%E0%A4%A',
text: 'https://youtu.be/abcd',
},
].forEach(({ href, text }) => {
it('does not replace links if percent-encoding agnostic comparison of href and text fails', () => {
const element = domElement(`<a href="${href}">${text}</a>`);
mediaEmbedder.replaceLinksWithEmbeds(element);
assert.equal(element.childElementCount, 1);
assert.equal(element.children[0].tagName, 'A');
});
});
it('does not replace non-media links', () => { it('does not replace non-media links', () => {
const url = 'https://example.com/example.html'; const url = 'https://example.com/example.html';
const element = domElement('<a href="' + url + '">' + url + '</a>'); const element = domElement('<a href="' + url + '">' + url + '</a>');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment