From 9e241e82e93d76229f25fa1ceb490f1458d5a644 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 20 Jun 2023 11:49:22 +1000 Subject: [PATCH] DEV: use HTML5 version of loofah (#21522) https://meta.discourse.org/t/markdown-preview-and-result-differ/263878 The result of this markdown had different results in the composer preview and the post. This is solved by updating Loofah to the latest version and using html5 fragments like our user had reported. While the change was only needed in cooked_post_processor.rb for this fix, other areas also had to be updated due to various side effects. --- Gemfile | 1 + Gemfile.lock | 3 +- app/jobs/regular/pull_hotlinked_images.rb | 5 ++ app/models/post_hotlinked_media.rb | 4 +- lib/cooked_post_processor.rb | 2 +- lib/oneboxer.rb | 4 +- lib/pretty_text.rb | 2 +- .../spec/integration/post_chat_quote_spec.rb | 75 +++++++------------ plugins/chat/spec/models/chat/message_spec.rb | 24 ++---- .../spec/controllers/posts_controller_spec.rb | 3 +- plugins/poll/spec/lib/pretty_text_spec.rb | 28 +++---- spec/lib/cooked_post_processor_spec.rb | 24 ++++-- spec/lib/oneboxer_spec.rb | 3 +- spec/lib/pretty_text_spec.rb | 46 ++++++++---- spec/support/dom_matcher.rb | 14 ++++ 15 files changed, 123 insertions(+), 115 deletions(-) create mode 100644 spec/support/dom_matcher.rb diff --git a/Gemfile b/Gemfile index b132eb07c02..a8298ab21b9 100644 --- a/Gemfile +++ b/Gemfile @@ -145,6 +145,7 @@ group :test do gem "selenium-webdriver", require: false gem "test-prof" gem "webdrivers", require: false + gem "rails-dom-testing", require: false end group :test, :development do diff --git a/Gemfile.lock b/Gemfile.lock index 3e0c626b632..611ca40e714 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -624,6 +624,7 @@ DEPENDENCIES rack rack-mini-profiler rack-protection + rails-dom-testing rails_failover rails_multisite railties (= 7.0.4.3) @@ -671,4 +672,4 @@ DEPENDENCIES yard BUNDLED WITH - 2.4.4 + 2.4.13 diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 43a64f519eb..ec8a7c78d6b 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -24,6 +24,7 @@ module Jobs extract_images_from(post.cooked).each do |node| download_src = original_src = node["src"] || node[PrettyText::BLOCKED_HOTLINKED_SRC_ATTR] || node["href"] + download_src = replace_encoded_src(download_src) download_src = "#{SiteSetting.force_https ? "https" : "http"}:#{original_src}" if original_src.start_with?( "//", @@ -198,6 +199,10 @@ module Jobs protected + def replace_encoded_src(src) + PostHotlinkedMedia.normalize_src(src, reset_scheme: false) + end + def normalize_src(src) PostHotlinkedMedia.normalize_src(src) end diff --git a/app/models/post_hotlinked_media.rb b/app/models/post_hotlinked_media.rb index de7d6aca7a5..53477bf7563 100644 --- a/app/models/post_hotlinked_media.rb +++ b/app/models/post_hotlinked_media.rb @@ -10,10 +10,10 @@ class PostHotlinkedMedia < ActiveRecord::Base upload_create_failed: "upload_create_failed", } - def self.normalize_src(src) + def self.normalize_src(src, reset_scheme: true) uri = Addressable::URI.heuristic_parse(src) uri.normalize! - uri.scheme = nil + uri.scheme = nil if reset_scheme uri.to_s rescue URI::Error, Addressable::URI::InvalidURIError src diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 9550a83690a..0d3a6d78367 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -26,7 +26,7 @@ class CookedPostProcessor @category_id = @post&.topic&.category_id cooked = post.cook(post.raw, @cooking_options) - @doc = Loofah.fragment(cooked) + @doc = Loofah.html5_fragment(cooked) @has_oneboxes = post.post_analyzer.found_oneboxes? @size_cache = {} diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb index 9bee58b6a34..781f4308cab 100644 --- a/lib/oneboxer.rb +++ b/lib/oneboxer.rb @@ -206,14 +206,14 @@ module Oneboxer def self.apply(string_or_doc, extra_paths: nil) doc = string_or_doc - doc = Loofah.fragment(doc) if doc.is_a?(String) + doc = Loofah.html5_fragment(doc) if doc.is_a?(String) changed = false each_onebox_link(doc, extra_paths: extra_paths) do |url, element| onebox, _ = yield(url, element) next if onebox.blank? - parsed_onebox = Loofah.fragment(onebox) + parsed_onebox = Loofah.html5_fragment(onebox) next if parsed_onebox.children.blank? changed = true diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index f3a4b6e4420..bbcaacee836 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -312,7 +312,7 @@ module PrettyText add_mentions(doc, user_id: opts[:user_id]) if SiteSetting.enable_mentions scrubber = Loofah::Scrubber.new { |node| node.remove if node.name == "script" } - loofah_fragment = Loofah.fragment(doc.to_html) + loofah_fragment = Loofah.html5_fragment(doc.to_html) loofah_fragment.scrub!(scrubber).to_html end diff --git a/plugins/chat/spec/integration/post_chat_quote_spec.rb b/plugins/chat/spec/integration/post_chat_quote_spec.rb index c3343d1c199..113fa04e155 100644 --- a/plugins/chat/spec/integration/post_chat_quote_spec.rb +++ b/plugins/chat/spec/integration/post_chat_quote_spec.rb @@ -16,12 +16,10 @@ describe "chat bbcode quoting in posts" do
martin
- -
+
-

This is a chat message.

-
+

This is a chat message.

COOKED end @@ -34,19 +32,16 @@ describe "chat bbcode quoting in posts" do expect(post.cooked.chomp).to eq(<<~COOKED.chomp)
- Originally sent in Cool Cats Club -
+ Originally sent in Cool Cats Club
martin
- -
+
-

This is a chat message.

-
+

This is a chat message.

COOKED end @@ -63,14 +58,11 @@ describe "chat bbcode quoting in posts" do
martin
- -
+ - #Cool Cats Club - + #Cool Cats Club
-

This is a chat message.

-
+

This is a chat message.

COOKED end @@ -87,14 +79,11 @@ describe "chat bbcode quoting in posts" do
martin
- -
+ - #Cool Cats Club - + #Cool Cats Club
-

This is a chat message.

-
+

This is a chat message.

COOKED end @@ -107,19 +96,16 @@ describe "chat bbcode quoting in posts" do expect(post.cooked.chomp).to eq(<<~COOKED.chomp)
- Originally sent in Cool Cats Club -
+ Originally sent in Cool Cats Club
martin
- -
+
-

This is a chat message.

-
+

This is a chat message.

COOKED end @@ -137,14 +123,11 @@ describe "chat bbcode quoting in posts" do
martin
- -
+ - #Cool Cats Club - + #Cool Cats Club
-

This is a chat message.

-
+

This is a chat message.

+1 1
@@ -237,35 +220,27 @@ martin
- -
+
#{message1.user.username}
- -
+
-##{channel.name} -
+##{channel.name}
- -
+
#{message2.user.username}
- -
+
-##{channel.name} -
+##{channel.name}
-

#{message2.message}

-
- - +

#{message2.message}

+ COOKED end diff --git a/plugins/chat/spec/models/chat/message_spec.rb b/plugins/chat/spec/models/chat/message_spec.rb index 5ddfb92adaa..a9c332c0800 100644 --- a/plugins/chat/spec/models/chat/message_spec.rb +++ b/plugins/chat/spec/models/chat/message_spec.rb @@ -87,8 +87,7 @@ describe Chat::Message do