Link URLs only in a message's text

auto_link finds URLs and email addresses with regular expressions over
the rendered html, which can't tell an attribute value from text, so it
could put a link inside an attribute. Hand it each text node on its own,
escaped, after the final sanitize, so links only ever land between tags.

Bump the presentation cache versions so nothing rendered the old way is
served again.

app/helpers/content_filters.rb

@@ -5,7 +5,7 @@ module ContentFilters
   #
   #   * SanitizeTags strips disallowed markup from the message body
   #   * Action Text sanitizes the rendered content (lib/rails_ext/action_text_allowed_tags.rb)
-  #   * auto_link re-sanitizes the final html (MessagesHelper#message_presentation)
+  #   * the final html is re-sanitized before its text is linked (MessagesHelper#auto_link_text)
   EDITOR_FORMATTING_TAGS = %w[ s u mark table thead tbody tfoot tr th td ]
   EDITOR_FORMATTING_ATTRIBUTES = %w[ data-language ]
 

app/helpers/messages_helper.rb

@@ -1,9 +1,12 @@
 module MessagesHelper
-  # auto_link re-sanitizes with Rails' default safe list, which lacks the
+  # Rendered text is re-sanitized before linking. Rails' default safe list
-  # formatting the rich text editor produces
+  # lacks the formatting the rich text editor produces
   AUTO_LINK_ALLOWED_TAGS = Rails::HTML5::SafeListSanitizer.allowed_tags + ContentFilters::EDITOR_FORMATTING_TAGS
   AUTO_LINK_ALLOWED_ATTRIBUTES = Rails::HTML5::SafeListSanitizer.allowed_attributes + ContentFilters::EDITOR_FORMATTING_ATTRIBUTES
 
+  # Text auto_link could turn into a link: a URL has a scheme or www., an email address an @
+  LINKABLE_TEXT = /:|@|www\./i
+
   def message_area_tag(room, &)
     tag.div id: "message-area", class: "message-area", contents: true, data: {
       controller: "messages presence drop-target",
@@ -101,21 +104,35 @@ module MessagesHelper
 
   private
     def text_message_presentation(body)
-      render = -> do
+      render = -> { auto_link_text h(ContentFilters::TextMessagePresentationFilters.apply(body)) }
-        auto_link h(ContentFilters::TextMessagePresentationFilters.apply(body)),
-          html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES }
-      end
 
       # Embedded attachments render database-backed metadata and signed URLs.
       # Plain text HTML depends only on its content, even after a foreign edit.
       html = body.to_html
       if controller.perform_caching && !html.include?("<action-text-attachment") && html.bytesize <= ResponseCache::MAX_ENTRY_BYTES
-        FragmentCache.store.fetch([ "text-presentation-v1", I18n.locale, Digest::SHA256.hexdigest(html) ]) { render.call }
+        FragmentCache.store.fetch([ "text-presentation-v2", I18n.locale, Digest::SHA256.hexdigest(html) ]) { render.call }
       else
         render.call
       end
     end
 
+    # auto_link finds URLs and email addresses with regular expressions, which
+    # can't tell an attribute value from text. Hand it one text node at a time,
+    # escaped, so a link can only land between tags.
+    def auto_link_text(html)
+      fragment = Nokogiri::HTML5.fragment(sanitize(html, tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES))
+
+      fragment.xpath(".//text()[not(ancestor::a)]").each do |node|
+        if node.text.match?(LINKABLE_TEXT)
+          text = h(node.text)
+          linked = auto_link(text, html: { target: "_blank" })
+          node.replace Nokogiri::HTML5.fragment(linked) unless linked == text
+        end
+      end
+
+      fragment.to_html.html_safe
+    end
+
     def messages_actions
       "turbo:before-stream-render@document->messages#beforeStreamRender keydown.up@document->messages#editMyLastMessage"
     end

app/views/messages/_message.html.erb

@@ -1,7 +1,7 @@
 <%# Be sure to check/update messages/_template.html.erb when changing this file %>
 
 <%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %>
-<% cache [ message_fragment_cache_key(message), "presentation-v7" ] do %>
+<% cache [ message_fragment_cache_key(message), "presentation-v8" ] do %>
   <%= message_tag message do %>
     <h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2>
 

test/helpers/messages_helper_test.rb

@@ -37,6 +37,34 @@ class MessagesHelperTest < ActionView::TestCase
     assert_match /<strong>bold<\/strong>/, presentation
   end
 
+  test "message_presentation links URLs and email addresses in text" do
+    message = Message.create! room: rooms(:pets), body: "<div>See https://example.com/a?b=1&amp;c=2, or write to me@example.com.</div>", client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ "https://example.com/a?b=1&c=2", "mailto:me@example.com" ], links.map { it["href"] }
+    assert_equal [ "_blank", "_blank" ], links.map { it["target"] }
+    assert_equal [ "https://example.com/a?b=1&c=2", "me@example.com" ], links.map(&:text)
+  end
+
+  test "message_presentation leaves URLs and email addresses in attribute values as they are" do
+    [ "class", "title" ].product([ "https://example.com", "www.example.com", "me@example.com" ]).each do |attribute, linkable|
+      value = "a> #{linkable} <em>b</em> "
+      message = Message.create! room: rooms(:pets), body: %(<div><span #{attribute}="#{value}">c</span> https://example.org</div>), client_message_id: "0015", creator: users(:jason)
+
+      presentation = Nokogiri::HTML5.fragment(view.message_presentation(message))
+      assert_equal [ value ], presentation.css("span[#{attribute}]").map { it[attribute] }, "#{attribute} with #{linkable}"
+      assert_empty presentation.css("em"), "#{attribute} with #{linkable}"
+      assert_equal [ "https://example.org" ], presentation.css("a").map { it["href"] }, "#{attribute} with #{linkable}"
+    end
+  end
+
+  test "message_presentation leaves an existing link's text alone" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="https://example.com/">https://example.org/</a></div>', client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ [ "https://example.com/", "https://example.org/" ] ], links.map { [ it["href"], it.text ] }
+  end
+
   test "message_presentation shows an image's thumbnail made when it was posted" do
     presentation = view.message_presentation(attachment_message("moon.jpg", "image/jpeg", processed: true))