From 6e2b484f12338a92642dc6ccc982c5b4b6d5cf58 Mon Sep 17 00:00:00 2001 From: David Battersby Date: Wed, 13 Sep 2023 14:33:08 +0800 Subject: [PATCH] FIX: prevent lightbox images from double escaping titles (#23458) This change fixes an issue where lightbox images are showing escaped text in the link title and lightbox image description area. --- .../javascripts/discourse/app/lib/lightbox.js | 7 ++----- .../discourse/app/lib/lightbox/process-html.js | 5 ++--- .../unit/lib/lightbox/process-html-test.js | 18 ------------------ lib/cooked_post_processor.rb | 2 +- spec/lib/cooked_post_processor_spec.rb | 2 +- 5 files changed, 6 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/lightbox.js b/app/assets/javascripts/discourse/app/lib/lightbox.js index eb5c92170a5..bdf87d802e6 100644 --- a/app/assets/javascripts/discourse/app/lib/lightbox.js +++ b/app/assets/javascripts/discourse/app/lib/lightbox.js @@ -1,7 +1,4 @@ -import { - escapeExpression, - postRNWebviewMessage, -} from "discourse/lib/utilities"; +import { postRNWebviewMessage } from "discourse/lib/utilities"; import I18n from "I18n"; import User from "discourse/models/user"; @@ -119,7 +116,7 @@ export default function lightbox(elem, siteSettings) { titleSrc(item) { const href = item.el.data("download-href") || item.src; let src = [ - escapeExpression(item.el.attr("title")), + item.el.attr("title"), $("span.informations", item.el).text(), ]; if ( diff --git a/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js b/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js index cf65e807383..c0e4793a1b8 100644 --- a/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js +++ b/app/assets/javascripts/discourse/app/lib/lightbox/process-html.js @@ -1,5 +1,4 @@ import { SELECTORS } from "./constants"; -import { escapeExpression } from "discourse/lib/utilities"; import { htmlSafe } from "@ember/template"; export async function processHTML({ container, selector, clickTarget }) { @@ -41,7 +40,7 @@ export async function processHTML({ container, selector, clickTarget }) { null; const _title = - item.title || item.alt || innerImage.title || innerImage.alt || null; + item.title || item.alt || innerImage.title || innerImage.alt || ""; const _aspectRatio = item.dataset?.aspectRatio || @@ -67,7 +66,7 @@ export async function processHTML({ container, selector, clickTarget }) { fullsizeURL: encodeURI(_fullsizeURL), smallURL: encodeURI(_smallURL), downloadURL: encodeURI(_downloadURL), - title: escapeExpression(_title), + title: _title, fileDetails: _fileDetails, dominantColor: _dominantColor, aspectRatio: _aspectRatio, diff --git a/app/assets/javascripts/discourse/tests/unit/lib/lightbox/process-html-test.js b/app/assets/javascripts/discourse/tests/unit/lib/lightbox/process-html-test.js index d0eb3ddce2c..f9bf75e83cb 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/lightbox/process-html-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/lightbox/process-html-test.js @@ -127,24 +127,6 @@ module("Unit | lib | Experimental lightbox | processHTML()", function () { assert.strictEqual(items[0].title, ""); }); - test("correctly escapes the title", async function (assert) { - const container = wrap.cloneNode(true); - - container - .querySelector("a") - .setAttribute("title", `"><\x00script>javascript:alert(1)`); - - const { items } = await processHTML({ - container, - selector, - }); - - assert.strictEqual( - items[0].title, - `"><\x00script>javascript:alert(1)</script>` - ); - }); - test("handles missing aspect ratio", async function (assert) { const container = wrap.cloneNode(true); diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 910d5068d1e..0364fc209aa 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -292,7 +292,7 @@ class CookedPostProcessor informations = +"#{original_width}×#{original_height}" informations << " #{upload.human_filesize}" if upload - a["title"] = CGI.escapeHTML(img["title"] || img["alt"] || filename) + a["title"] = img["title"] || img["alt"] || filename meta.add_child create_icon_node("far-image") meta.add_child create_span_node("filename", a["title"]) diff --git a/spec/lib/cooked_post_processor_spec.rb b/spec/lib/cooked_post_processor_spec.rb index b95882b2614..fbbdf419bb9 100644 --- a/spec/lib/cooked_post_processor_spec.rb +++ b/spec/lib/cooked_post_processor_spec.rb @@ -649,7 +649,7 @@ RSpec.describe CookedPostProcessor do cpp.post_process expect(cpp.html).to match_html <<~HTML -

&gt;&lt;img src=x onerror=alert(&#39;haha&#39;)&gt;.png1750×2000 1.21 KB

+

><img src=x onerror=alert('haha')>.png1750×2000 1.21 KB

HTML end end