diff --git a/app/helpers/content_filters.rb b/app/helpers/content_filters.rb index 0b9ee145..6c23fcd9 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 066e15e9..d14f60c7 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", @@ -63,8 +66,7 @@ module MessagesHelper when "sound" message_sound_presentation(message) else - auto_link h(ContentFilters::TextMessagePresentationFilters.apply(message.body.body)), - html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES } + auto_link_text h(ContentFilters::TextMessagePresentationFilters.apply(message.body.body)) end rescue Exception => e Sentry.capture_exception(e, extra: { message: message }) @@ -74,6 +76,24 @@ module MessagesHelper end private + # 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 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 = node.to_html + linked = auto_link(text, html: { target: "_blank" }) + node.replace Nokogiri::HTML5.fragment(linked) unless linked == text + end + end + + # 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
"turbo:before-stream-render@document->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 0e02bc4d..7d02c825 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, "presentation-v6" ] do %>
+<% cache [ message, "presentation-v7" ] 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 78a97c99..20eaf6a7 100644
--- a/test/helpers/messages_helper_test.rb
+++ b/test/helpers/messages_helper_test.rb
@@ -25,6 +25,47 @@ 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 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 "
+ 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 "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)
+
+ 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))