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.
2024-11-22 06:19:17 +08:00 · 2023-06-20 11:49:22 +10:00 · 2023-06-20 11:49:22 +10:00 · 9e241e82e9
commit 9e241e82e9
parent a999deaab9
15 changed files with 123 additions and 115 deletions
--- a/1
+++ b/1
@ -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
--- 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
--- 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
--- 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
--- 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 = {}

--- 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
--- 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

--- 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
      <div class="chat-transcript-username">
      martin</div>
      <div class="chat-transcript-datetime">
-      <span title="2022-01-25T05:40:39Z"></span>
-      </div>
+      <span title="2022-01-25T05:40:39Z"></span></div>
      </div>
      <div class="chat-transcript-messages">
-      <p>This is a chat message.</p>
-      </div>
+      <p>This is a chat message.</p></div>
      </div>
    COOKED
  end
@ -34,19 +32,16 @@ describe "chat bbcode quoting in posts" do
    expect(post.cooked.chomp).to eq(<<~COOKED.chomp)
      <div class="chat-transcript" data-message-id="2321" data-username="martin" data-datetime="2022-01-25T05:40:39Z" data-channel-name="Cool Cats Club" data-channel-id="1234">
      <div class="chat-transcript-meta">
-      Originally sent in <a href="/chat/c/-/1234">Cool Cats Club</a>
-      </div>
+      Originally sent in <a href="/chat/c/-/1234">Cool Cats Club</a></div>
      <div class="chat-transcript-user">
      <div class="chat-transcript-user-avatar"></div>
      <div class="chat-transcript-username">
      martin</div>
      <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
-      </div>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a></div>
      </div>
      <div class="chat-transcript-messages">
-      <p>This is a chat message.</p>
-      </div>
+      <p>This is a chat message.</p></div>
      </div>
    COOKED
  end
@ -63,14 +58,11 @@ describe "chat bbcode quoting in posts" do
      <div class="chat-transcript-username">
      martin</div>
      <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
-      </div>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a></div>
      <a class="chat-transcript-channel" href="/chat/c/-/1234">
-      #Cool Cats Club</a>
-      </div>
+      #Cool Cats Club</a></div>
      <div class="chat-transcript-messages">
-      <p>This is a chat message.</p>
-      </div>
+      <p>This is a chat message.</p></div>
      </div>
    COOKED
  end
@ -87,14 +79,11 @@ describe "chat bbcode quoting in posts" do
      <div class="chat-transcript-username">
      martin</div>
      <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
-      </div>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a></div>
      <a class="chat-transcript-channel" href="/chat/c/-/1234">
-      #Cool Cats Club</a>
-      </div>
+      #Cool Cats Club</a></div>
      <div class="chat-transcript-messages">
-      <p>This is a chat message.</p>
-      </div>
+      <p>This is a chat message.</p></div>
      </div>
    COOKED
  end
@ -107,19 +96,16 @@ describe "chat bbcode quoting in posts" do
    expect(post.cooked.chomp).to eq(<<~COOKED.chomp)
      <div class="chat-transcript" data-message-id="2321" data-username="martin" data-datetime="2022-01-25T05:40:39Z" data-channel-name="Cool Cats Club" data-channel-id="1234">
      <div class="chat-transcript-meta">
-      Originally sent in <a href="/chat/c/-/1234">Cool Cats Club</a>
-      </div>
+      Originally sent in <a href="/chat/c/-/1234">Cool Cats Club</a></div>
      <div class="chat-transcript-user">
      <div class="chat-transcript-user-avatar"></div>
      <div class="chat-transcript-username">
      martin</div>
      <div class="chat-transcript-datetime">
-      <span title="2022-01-25T05:40:39Z"></span>
-      </div>
+      <span title="2022-01-25T05:40:39Z"></span></div>
      </div>
      <div class="chat-transcript-messages">
-      <p>This is a chat message.</p>
-      </div>
+      <p>This is a chat message.</p></div>
      </div>
    COOKED
  end
@ -137,14 +123,11 @@ describe "chat bbcode quoting in posts" do
      <div class="chat-transcript-username">
      martin</div>
      <div class="chat-transcript-datetime">
-      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a>
-      </div>
+      <a href="/chat/c/-/1234/2321" title="2022-01-25T05:40:39Z"></a></div>
      <a class="chat-transcript-channel" href="/chat/c/-/1234">
-      #Cool Cats Club</a>
-      </div>
+      #Cool Cats Club</a></div>
      <div class="chat-transcript-messages">
-      <p>This is a chat message.</p>
-      <div class="chat-transcript-reactions">
+      <p>This is a chat message.</p><div class="chat-transcript-reactions">
      <div class="chat-transcript-reaction">
      <img width="20" height="20" src="/images/emoji/twitter/+1.png?v=12" title="+1" loading="lazy" alt="+1" class="emoji"> 1</div>
      <div class="chat-transcript-reaction">
@ -237,35 +220,27 @@ martin</div>
 <div class="chat-transcript" data-message-id="#{message1.id}" data-username="#{message1.user.username}" data-datetime="#{message1.created_at.iso8601}" data-channel-name="#{channel.name}" data-channel-id="#{channel.id}">
 <div class="chat-transcript-user">
 <div class="chat-transcript-user-avatar">
-<img loading="lazy" alt="" width="24" height="24" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "48")}" class="avatar">
-</div>
+<img loading="lazy" alt="" width="24" height="24" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "48")}" class="avatar"></div>
 <div class="chat-transcript-username">
 #{message1.user.username}</div>
 <div class="chat-transcript-datetime">
-<a href="/chat/c/-/#{channel.id}/#{message1.id}" title="#{message1.created_at.iso8601}"></a>
-</div>
+<a href="/chat/c/-/#{channel.id}/#{message1.id}" title="#{message1.created_at.iso8601}"></a></div>
 <a class="chat-transcript-channel" href="/chat/c/-/#{channel.id}">
-##{channel.name}</a>
-</div>
+##{channel.name}</a></div>
 <div class="chat-transcript-messages">
 <div class="chat-transcript" data-message-id="#{message2.id}" data-username="#{message2.user.username}" data-datetime="#{message2.created_at.iso8601}" data-channel-name="#{channel.name}" data-channel-id="#{channel.id}">
 <div class="chat-transcript-user">
 <div class="chat-transcript-user-avatar">
-<img loading="lazy" alt="" width="24" height="24" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "48")}" class="avatar">
-</div>
+<img loading="lazy" alt="" width="24" height="24" src="//test.localhost#{post.user.avatar_template.gsub("{size}", "48")}" class="avatar"></div>
 <div class="chat-transcript-username">
 #{message2.user.username}</div>
 <div class="chat-transcript-datetime">
-<a href="/chat/c/-/#{channel.id}/#{message2.id}" title="#{message1.created_at.iso8601}"></a>
-</div>
+<a href="/chat/c/-/#{channel.id}/#{message2.id}" title="#{message1.created_at.iso8601}"></a></div>
 <a class="chat-transcript-channel" href="/chat/c/-/#{channel.id}">
-##{channel.name}</a>
-</div>
+##{channel.name}</a></div>
 <div class="chat-transcript-messages">
-<p>#{message2.message}</p>
-</div>
-</div>
-</div>
+<p>#{message2.message}</p></div>
+</div></div>
 </div>
    COOKED
  end
--- 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
      <aside class="quote no-group" data-username="#{post.user.username}" data-post="#{post.post_number}" data-topic="#{topic.id}">
      <div class="title">
      <div class="quote-controls"></div>
-      <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src}" class="avatar"><a href="http://test.localhost/t/some-quotable-topic/#{topic.id}/#{post.post_number}">#{topic.title}</a>
-      </div>
+      <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src}" class="avatar"><a href="http://test.localhost/t/some-quotable-topic/#{topic.id}/#{post.post_number}">#{topic.title}</a></div>
      <blockquote>
      <p>Mark me...this will go down in history.</p>
      </blockquote>
@ -131,36 +130,29 @@ describe Chat::Message do
      expect(cooked).to eq(<<~COOKED.chomp)
        <div class="chat-transcript chat-transcript-chained" data-message-id="#{msg1.id}" data-username="chatbbcodeuser" data-datetime="#{msg1.created_at.iso8601}" data-channel-name="testchannel" data-channel-id="#{chat_channel.id}">
        <div class="chat-transcript-meta">
-        Originally sent in <a href="/chat/c/-/#{chat_channel.id}">testchannel</a>
-        </div>
+        Originally sent in <a href="/chat/c/-/#{chat_channel.id}">testchannel</a></div>
        <div class="chat-transcript-user">
        <div class="chat-transcript-user-avatar">
-        <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src}" class="avatar">
-        </div>
+        <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src}" class="avatar"></div>
        <div class="chat-transcript-username">
        chatbbcodeuser</div>
        <div class="chat-transcript-datetime">
-        <a href="/chat/c/-/#{chat_channel.id}/#{msg1.id}" title="#{msg1.created_at.iso8601}"></a>
-        </div>
+        <a href="/chat/c/-/#{chat_channel.id}/#{msg1.id}" title="#{msg1.created_at.iso8601}"></a></div>
        </div>
        <div class="chat-transcript-messages">
-        <p>this is the first message</p>
-        </div>
+        <p>this is the first message</p></div>
        </div>
        <div class="chat-transcript chat-transcript-chained" data-message-id="#{msg2.id}" data-username="otherbbcodeuser" data-datetime="#{msg2.created_at.iso8601}">
        <div class="chat-transcript-user">
        <div class="chat-transcript-user-avatar">
-        <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src2}" class="avatar">
-        </div>
+        <img loading="lazy" alt="" width="24" height="24" src="#{avatar_src2}" class="avatar"></div>
        <div class="chat-transcript-username">
        otherbbcodeuser</div>
        <div class="chat-transcript-datetime">
-        <span title="#{msg2.created_at.iso8601}"></span>
-        </div>
+        <span title="#{msg2.created_at.iso8601}"></span></div>
        </div>
        <div class="chat-transcript-messages">
-        <p>and another cool one</p>
-        </div>
+        <p>and another cool one</p></div>
        </div>
      COOKED
    end
--- a/plugins/poll/spec/controllers/posts_controller_spec.rb
+++ b/plugins/poll/spec/controllers/posts_controller_spec.rb
@ -129,8 +129,7 @@ RSpec.describe PostsController do

      expect(response.status).to eq(200)
      json = response.parsed_body
-      expect(json["cooked"]).to match("data-poll-")
-      expect(json["cooked"]).to include("&lt;script&gt;")
+      expect(json["cooked"]).to include("data-poll-name=\"<script>alert('xss')</script>\"")
      expect(Poll.find_by(post_id: json["id"]).name).to eq(
        "&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;",
      )
--- a/plugins/poll/spec/lib/pretty_text_spec.rb
+++ b/plugins/poll/spec/lib/pretty_text_spec.rb
@ -94,10 +94,8 @@ RSpec.describe PrettyText do
    expected = <<~HTML
      <div class="poll" data-poll-status="open" data-poll-type="multiple" data-poll-name="poll">
      <div>
-      <div class="poll-container">
-      <ol>
-      <li data-poll-option-id="b6475cbf6acb8676b20c60582cfc487a">test 1 <img src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title=":slight_smile:" class="emoji" alt=":slight_smile:" loading="lazy" width="20" height="20"> <b>test</b>
-      </li>
+      <div class="poll-container"><ol>
+      <li data-poll-option-id="b6475cbf6acb8676b20c60582cfc487a">test 1 <img src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title=":slight_smile:" class="emoji" alt=":slight_smile:" loading="lazy" width="20" height="20"> <b>test</b></li>
      <li data-poll-option-id="7158af352698eb1443d709818df097d4">test 2</li>
      </ol>
      </div>
@ -159,10 +157,9 @@ RSpec.describe PrettyText do
      [/poll]
    MD

-    expect(cooked).to include(<<~HTML)
-      <div class="poll-title">What’s your favorite <em>berry</em>? <img src="/images/emoji/twitter/wink.png?v=#{Emoji::EMOJI_VERSION}" title=":wink:" class="emoji" alt=":wink:" loading="lazy" width="20" height="20"> <a href="https://google.com/" rel="noopener nofollow ugc">https://google.com/</a>
-      </div>
-    HTML
+    expect(cooked).to include(
+      "<div class=\"poll-title\">What’s your favorite <em>berry</em>? <img src=\"/images/emoji/twitter/wink.png?v=#{Emoji::EMOJI_VERSION}\" title=\":wink:\" class=\"emoji\" alt=\":wink:\" loading=\"lazy\" width=\"20\" height=\"20\"> <a href=\"https://google.com/\" rel=\"noopener nofollow ugc\">https://google.com/</a></div>",
+    )
  end

  it "does not break when there are headings before/after a poll with a title" do
@ -179,16 +176,15 @@ RSpec.describe PrettyText do
      # Post-heading
    MD

-    expect(cooked).to include(<<~HTML)
-      <div class="poll-title">What’s your favorite <em>berry</em>? <img src="/images/emoji/twitter/wink.png?v=#{Emoji::EMOJI_VERSION}" title=":wink:" class="emoji" alt=":wink:" loading="lazy" width="20" height="20"> <a href="https://google.com/" rel="noopener nofollow ugc">https://google.com/</a>
-      </div>
-    HTML
+    expect(cooked).to include(
+      "<div class=\"poll-title\">What’s your favorite <em>berry</em>? <img src=\"/images/emoji/twitter/wink.png?v=#{Emoji::EMOJI_VERSION}\" title=\":wink:\" class=\"emoji\" alt=\":wink:\" loading=\"lazy\" width=\"20\" height=\"20\"> <a href=\"https://google.com/\" rel=\"noopener nofollow ugc\">https://google.com/</a></div>",
+    )

    expect(cooked).to include(
-      "<h1>\n<a name=\"pre-heading-1\" class=\"anchor\" href=\"#pre-heading-1\"></a>Pre-heading</h1>",
+      "<h1><a name=\"pre-heading-1\" class=\"anchor\" href=\"#pre-heading-1\"></a>Pre-heading</h1>",
    )
    expect(cooked).to include(
-      "<h1>\n<a name=\"post-heading-2\" class=\"anchor\" href=\"#post-heading-2\"></a>Post-heading</h1>",
+      "<h1><a name=\"post-heading-2\" class=\"anchor\" href=\"#post-heading-2\"></a>Post-heading</h1>",
    )
  end

@ -211,10 +207,10 @@ RSpec.describe PrettyText do
    HTML

    expect(cooked).to include(
-      "<h1>\n<a name=\"pre-heading-1\" class=\"anchor\" href=\"#pre-heading-1\"></a>Pre-heading</h1>",
+      "<h1><a name=\"pre-heading-1\" class=\"anchor\" href=\"#pre-heading-1\"></a>Pre-heading</h1>",
    )
    expect(cooked).to include(
-      "<h1>\n<a name=\"post-heading-2\" class=\"anchor\" href=\"#post-heading-2\"></a>Post-heading</h1>",
+      "<h1><a name=\"post-heading-2\" class=\"anchor\" href=\"#post-heading-2\"></a>Post-heading</h1>",
    )
  end
 end
--- a/spec/lib/cooked_post_processor_spec.rb
+++ b/spec/lib/cooked_post_processor_spec.rb
@ -1668,15 +1668,23 @@ RSpec.describe CookedPostProcessor do
              audio_upload.url.sub(SiteSetting.s3_cdn_url, "#{Discourse.base_url}/secure-uploads")

            expect(cpp.html).to match_html <<~HTML
-              <p>This post has a video upload.</p>
-              <div class="onebox video-onebox">
+              <p>This post has a video upload.</p><div class="onebox video-onebox">
                <video width="100%" height="100%" controls="">
                  <source src="#{secure_video_url}">
-                  <a href="#{secure_video_url}">#{secure_video_url}</a>
+                  <a href="#{secure_video_url}">
+                    #{secure_video_url}
+                  </a>
                </video>
              </div>
+
              <p>This post has an audio upload.<br>
-              <audio controls=""><source src="#{secure_audio_url}"><a href="#{secure_audio_url}">#{secure_audio_url}</a></audio></p>
+              <audio controls="">
+                <source src="#{secure_audio_url}">
+                <a href="#{secure_audio_url}">
+                  #{secure_audio_url}
+                </a>
+              </audio>
+              </p>
              <p>And an image upload.<br>
              <img src="#{image_upload.url}" alt="#{image_upload.original_filename}" data-base62-sha1="#{image_upload.base62_sha1}"></p>
            HTML
@ -2136,10 +2144,10 @@ RSpec.describe CookedPostProcessor do
  end

  describe "#html" do
-    it "escapes attributes" do
-      post = Fabricate(:post, raw: '<img alt="<something>">')
-      expect(post.cook(post.raw)).to eq('<p><img alt="&lt;something&gt;"></p>')
-      expect(CookedPostProcessor.new(post).html).to eq('<p><img alt="&lt;something&gt;"></p>')
+    it "escapes html entities in attributes per html5" do
+      post = Fabricate(:post, raw: '<img alt="&<something>">')
+      expect(post.cook(post.raw)).to eq('<p><img alt="&amp;<something>"></p>')
+      expect(CookedPostProcessor.new(post).html).to eq('<p><img alt="&amp;<something>"></p>')
    end
  end
 end
--- a/spec/lib/oneboxer_spec.rb
+++ b/spec/lib/oneboxer_spec.rb
@ -795,7 +795,8 @@ RSpec.describe Oneboxer do
    it "does keeps SVGs valid" do
      raw = "Onebox\n\nhttps://example.com"
      cooked = PrettyText.cook(raw)
-      cooked = Oneboxer.apply(Loofah.fragment(cooked)) { "<div><svg><path></path></svg></div>" }
+      cooked =
+        Oneboxer.apply(Loofah.html5_fragment(cooked)) { "<div><svg><path></path></svg></div>" }
      doc = Nokogiri::HTML5.fragment(cooked.to_html)
      expect(doc.to_html).to match_html <<~HTML
        <p>Onebox</p>
--- a/spec/lib/pretty_text_spec.rb
+++ b/spec/lib/pretty_text_spec.rb
@ -34,8 +34,7 @@ RSpec.describe PrettyText do
          <aside class="quote no-group" data-username="codinghorror" data-post="2" data-topic="#{topic.id}">
          <div class="title">
          <div class="quote-controls"></div>
-          <a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic <img width="20" height="20" src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title="slight_smile" loading="lazy" alt="slight_smile" class="emoji"></a>
-          </div>
+          <a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic <img width="20" height="20" src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title="slight_smile" loading="lazy" alt="slight_smile" class="emoji"></a></div>
          <blockquote>
          <p>ddd</p>
          </blockquote>
@ -56,8 +55,7 @@ RSpec.describe PrettyText do
          <aside class="quote no-group" data-username="EvilTrout" data-post="2" data-topic="#{topic.id}">
          <div class="title">
          <div class="quote-controls"></div>
-          <a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic <img width="20" height="20" src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title="slight_smile" loading="lazy" alt="slight_smile" class="emoji"></a>
-          </div>
+          <a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic <img width="20" height="20" src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title="slight_smile" loading="lazy" alt="slight_smile" class="emoji"></a></div>
          <blockquote>
          <p>ddd</p>
          </blockquote>
@ -222,8 +220,7 @@ RSpec.describe PrettyText do
          <aside class="quote no-group" data-username="maja" data-post="3" data-topic="#{topic.id}">
          <div class="title">
          <div class="quote-controls"></div>
-          <a href="http://test.localhost/t/#{topic.id}/3">#{I18n.t("on_another_topic")}</a>
-          </div>
+          <a href="http://test.localhost/t/#{topic.id}/3">#{I18n.t("on_another_topic")}</a></div>
          <blockquote>
          <p>I have nothing to say.</p>
          </blockquote>
@ -245,8 +242,7 @@ RSpec.describe PrettyText do
          <aside class="quote no-group" data-username="maja" data-post="3" data-topic="#{topic.id}">
          <div class="title">
          <div class="quote-controls"></div>
-          <a href="http://test.localhost/t/this-is-an-off-topic-topic/#{topic.id}/3">#{topic.title}</a>
-          </div>
+          <a href="http://test.localhost/t/this-is-an-off-topic-topic/#{topic.id}/3">#{topic.title}</a></div>
          <blockquote>
          <p>I have nothing to say.</p>
          </blockquote>
@ -342,8 +338,7 @@ RSpec.describe PrettyText do
          <aside class="quote group-#{group.name}" data-username="#{user.username}" data-post="2" data-topic="#{topic.id}">
          <div class="title">
          <div class="quote-controls"></div>
-          <img loading="lazy" alt="" width="24" height="24" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/48.png" class="avatar"><a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic</a>
-          </div>
+          <img loading="lazy" alt="" width="24" height="24" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/48.png" class="avatar"><a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic</a></div>
          <blockquote>
          <p>ddd</p>
          </blockquote>
@ -2205,11 +2200,11 @@ HTML
    expect(cooked).to eq(html)
  end

-  it "provides safety for img bbcode" do
-    cooked = PrettyText.cook "[img]http://aaa.com<script>alert(1);</script>[/img]"
-    html =
-      '<p><img src="http://aaa.com&lt;script&gt;alert(1);&lt;/script&gt;" alt="" role="presentation"></p>'
-    expect(cooked).to eq(html)
+  it "supports img bbcode entities in attributes" do
+    actual = PrettyText.cook "[img]http://aaa.com/?a=1&b=<script>alert(1);</script>[/img]"
+    expected =
+      '<p><img src="http://aaa.com/?a=1&b=&lt;script&gt;alert(1);&lt;/script&gt;" alt="" role="presentation"></p>'
+    expect(expected).to be_same_dom(actual)
  end

  it "supports email bbcode" do
@ -2282,6 +2277,13 @@ HTML

  it "should strip SCRIPT" do
    expect(PrettyText.cook("<script>alert(42)</script>")).to eq ""
+    expect(PrettyText.cook("<div><script>alert(42)</script></div>")).to eq "<div></div>"
+  end
+
+  it "strips script regardless of sanitize" do
+    expect(
+      PrettyText.cook("<div><script>alert(42)</script></div>", sanitize: false),
+    ).to eq "<div></div>"
  end

  it "should allow sanitize bypass" do
@ -2651,4 +2653,18 @@ HTML
      expect(cooked).to eq("<p>:grin: <span class=\"mention\">@mention</span></p>")
    end
  end
+
+  it "does not amend HTML when scrubbing" do
+    md = <<~MD
+      <s>\n\nhello\n\n</s>
+    MD
+
+    html = <<~HTML
+      <s>\n<p>hello</p>\n</s>
+    HTML
+
+    cooked = PrettyText.cook(md)
+
+    expect(cooked.strip).to eq(html.strip)
+  end
 end
--- a/spec/support/dom_matcher.rb
+++ b/spec/support/dom_matcher.rb
@ -0,0 +1,14 @@
+# frozen_string_literal: true
+include Rails::Dom::Testing::Assertions::DomAssertions
+
+RSpec::Matchers.define :be_same_dom do |expected|
+  match do |actual|
+    begin
+      assert_dom_equal(expected, actual)
+    rescue MiniTest::Assertion
+      false
+    end
+  end
+
+  failure_message { |actual| "Expected DOM:\n#{expected}\nto be the same as:\n#{actual}" }
+end