From 7ea05e18f8fc0c1296000fe191a3afe9c1742553 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 16:30:26 -0700 Subject: [PATCH 1/4] 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 | 2 +- app/helpers/messages_helper.rb | 31 +++++++++++++++++++++------- app/views/messages/_message.html.erb | 2 +- test/helpers/messages_helper_test.rb | 28 +++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/app/helpers/content_filters.rb b/app/helpers/content_filters.rb index 0b9ee14..6c23fcd 100644 --- a/app/helpers/content_filters.rb +++ b/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 ] diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb index fcc4dcf..2b3faf6 100644 --- a/app/helpers/messages_helper.rb +++ b/app/helpers/messages_helper.rb @@ -1,9 +1,12 @@ module MessagesHelper - # auto_link re-sanitizes with Rails' default safe list, which lacks the - # formatting the rich text editor produces + # Rendered text is re-sanitized before linking. Rails' default safe list + # 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 - auto_link h(ContentFilters::TextMessagePresentationFilters.apply(body)), - html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES } - end + render = -> { auto_link_text h(ContentFilters::TextMessagePresentationFilters.apply(body)) } # 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?("messages#beforeStreamRender keydown.up@document->messages#editMyLastMessage" end diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb index 307e31f..a94a579 100644 --- a/app/views/messages/_message.html.erb +++ b/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 %>

<%= local_datetime_tag message.created_at, style: :date %>

diff --git a/test/helpers/messages_helper_test.rb b/test/helpers/messages_helper_test.rb index 5bf77ac..65830f1 100644 --- a/test/helpers/messages_helper_test.rb +++ b/test/helpers/messages_helper_test.rb @@ -37,6 +37,34 @@ class MessagesHelperTest < ActionView::TestCase assert_match /bold<\/strong>/, presentation end + test "message_presentation links URLs and email addresses in text" do + message = Message.create! room: rooms(:pets), body: "
See https://example.com/a?b=1&c=2, or write to me@example.com.
", 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} b " + message = Message.create! room: rooms(:pets), body: %(
c https://example.org
), 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: '
https://example.org/
', 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)) From 6052b123d5e2e457a798937647f3da5c26667aa9 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 16:59:25 -0700 Subject: [PATCH 2/4] Quote a mention as text when replying Replying swaps each mention in the quoted copy for its text. Assigning that text to outerHTML parsed it as html; replace the mention with a text node instead. --- app/javascript/controllers/reply_controller.js | 2 +- test/system/composer_test.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/javascript/controllers/reply_controller.js b/app/javascript/controllers/reply_controller.js index a90f73c..696eaff 100644 --- a/app/javascript/controllers/reply_controller.js +++ b/app/javascript/controllers/reply_controller.js @@ -28,7 +28,7 @@ export default class extends Controller { } #stripMentionAttachments(node) { - node.querySelectorAll(".mention").forEach(mention => mention.outerHTML = mention.textContent.trim()) + node.querySelectorAll(".mention").forEach(mention => mention.replaceWith(mention.textContent.trim())) return node } diff --git a/test/system/composer_test.rb b/test/system/composer_test.rb index 24ccd12..01ebf12 100644 --- a/test/system/composer_test.rb +++ b/test/system/composer_test.rb @@ -174,6 +174,20 @@ class ComposerTest < ApplicationSystemTestCase assert_selector last_message_selector("cite"), text: "JZ" end + test "replying quotes a mention's text as text" do + message = rooms(:designers).messages.create! creator: users(:kevin), client_message_id: "reply-mention", + body: %(
<em>Kevin</em> says hi
) + join_room rooms(:designers) + + within_message message do + reveal_message_actions + find("[aria-label='Reply']").click + end + + assert_composer_text "Kevin says hi" + assert_no_selector "#composer lexxy-editor em" + end + test "arrow up edits my last message when the composer is empty" do composer_editor.click press_in_composer :up From 4e29c6e32b5463bc637862b3ccc11ffd0de35d44 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 15 Oct 2026 16:59:25 -0700 Subject: [PATCH 3/4] Keep a code block's leading newline when linking a message's text A 
 whose text starts with a newline needs one more written out to
survive being parsed again, as the html5 serializer does with
preserve_newline. Without it, linking text dropped one more of a code
block's leading blank lines than rendering did before.
---
 app/helpers/messages_helper.rb       | 3 ++-
 test/helpers/messages_helper_test.rb | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb
index 2b3faf6..687d9a4 100644
--- a/app/helpers/messages_helper.rb
+++ b/app/helpers/messages_helper.rb
@@ -130,7 +130,8 @@ module MessagesHelper
         end
       end
 
-      fragment.to_html.html_safe
+      # A 
 starting with a newline needs an extra one written out to survive the next parse
+      fragment.to_html(preserve_newline: true).html_safe
     end
 
     def messages_actions
diff --git a/test/helpers/messages_helper_test.rb b/test/helpers/messages_helper_test.rb
index 65830f1..911317a 100644
--- a/test/helpers/messages_helper_test.rb
+++ b/test/helpers/messages_helper_test.rb
@@ -58,6 +58,12 @@ class MessagesHelperTest < ActionView::TestCase
     end
   end
 
+  test "linking text doesn't eat a code block's leading blank line" do
+    linked = view.send(:auto_link_text, "
\n\n\nfirst https://example.com
".html_safe)
+
+    assert_equal "\nfirst https://example.com", Nokogiri::HTML5.fragment(linked).at_css("pre").text
+  end
+
   test "message_presentation leaves an existing link's text alone" do
     message = Message.create! room: rooms(:pets), body: '', client_message_id: "0015", creator: users(:jason)
 

From 9b271d0e1986399236eb39d44e4c612d4cd555d2 Mon Sep 17 00:00:00 2001
From: Jeremy Daer 
Date: Thu, 15 Oct 2026 17:28:02 -0700
Subject: [PATCH 4/4] Hand auto_link text escaped the way the serializer writes
 it

That's exactly what it was given before, with quotes and apostrophes
left as they are, rather than relying on its own sanitize pass to undo
h's '. Covers an email address with an apostrophe.
---
 app/helpers/messages_helper.rb       | 4 ++--
 test/helpers/messages_helper_test.rb | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb
index 687d9a4..6bef743 100644
--- a/app/helpers/messages_helper.rb
+++ b/app/helpers/messages_helper.rb
@@ -118,13 +118,13 @@ module MessagesHelper
 
     # 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.
+    # escaped as the serializer writes text, 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)
+          text = node.to_html
           linked = auto_link(text, html: { target: "_blank" })
           node.replace Nokogiri::HTML5.fragment(linked) unless linked == text
         end
diff --git a/test/helpers/messages_helper_test.rb b/test/helpers/messages_helper_test.rb
index 911317a..83902f5 100644
--- a/test/helpers/messages_helper_test.rb
+++ b/test/helpers/messages_helper_test.rb
@@ -46,6 +46,13 @@ class MessagesHelperTest < ActionView::TestCase
     assert_equal [ "https://example.com/a?b=1&c=2", "me@example.com" ], links.map(&:text)
   end
 
+  test "message_presentation links an email address with an apostrophe whole" do
+    message = Message.create! room: rooms(:pets), body: "
Write to o'brien@example.com
", client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ [ "mailto:o%27brien@example.com", "o'brien@example.com" ] ], links.map { [ it["href"], it.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} b "