Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions lib/rich_text.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module RichText
DESCRIPTION_MAX_LENGTH = 500
DESCRIPTION_WORD_BREAK_THRESHOLD_LENGTH = 450
URL_UNSAFE_CHARS = "[^\\w!#$%&'*+,./:;=?@_~^\\-]"
LINK_ATTRIBUTES = 'rel="nofollow noopener noreferrer" dir="auto"'

def self.new(format, text)
case format
Expand Down Expand Up @@ -89,6 +90,7 @@ def sanitize(text)

def linkify(text, mode = :urls)
ERB::Util.html_escape(text)
.then { |html| linkify_users(html) }
.then { |html| expand_link_shorthands(html) }
.then { |html| expand_host_shorthands(html) }
.then { |html| auto_link(html, mode) }
Expand All @@ -97,6 +99,22 @@ def linkify(text, mode = :urls)

private

def linkify_users(text)
regex = /(?<!\w)@(?:“([^”]+?)”|”([^”]+?)”|‘([^’]+?)’|’([^’]+?)’|"([^"]+?)"|'([^']+?)'|(\w+))/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some way to match that doesn't involve an unreadable regular expression?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a lot of logic in this regular expression that is not tested. It looks like it matches

  • “”
  • ””
  • ‘’
  • ’’
  • ""
  • ''

Should we have a test that shows that it doesn't linkify something like @"foo'? What happens if there are multiple potential matches? e.g. @“foo@”bar”


text.gsub(regex) do |match|
name = ::Regexp.last_match.captures.compact.first
next match unless name

slug_name = name.tr("“”‘’", %q(""''
))
slug = ERB::Util.url_encode(slug_name)

safe_match = match.sub("@", "&#64;")
"<a href=\"/user/#{slug}\" #{LINK_ATTRIBUTES}>#{safe_match}</a>"
end
end

def gsub_pairs_for_linkify_detection
Array
.wrap(Settings.linkify&.detection_rules)
Expand Down Expand Up @@ -128,8 +146,7 @@ def expand_host_shorthands(text)
end

def auto_link(text, mode)
link_attr = 'rel="nofollow noopener noreferrer" dir="auto"'
Rinku.auto_link(text, mode, link_attr) { |url| format_link_text(url) }
Rinku.auto_link(text, mode, LINK_ATTRIBUTES) { |url| format_link_text(url) }
end

def format_link_text(url)
Expand Down
27 changes: 24 additions & 3 deletions test/lib/rich_text_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ def test_markdown_to_html
end

def test_markdown_table_alignment
# Ensure that kramdown table alignment styles are converted to bootstrap classes
markdown_table = <<~MARKDOWN
| foo | bar |
|:----:|----:|
Expand Down Expand Up @@ -225,6 +224,14 @@ def test_text_to_html_linkify
end
end

def test_linkify_username_with_space
with_settings(:linkify => { :detection_rules => [{ :patterns => ["@(?<username>\\w+)"], :path_template => "user/\\k<username>" }] }) do
text = 'Hello @"Open Mapper"'
html = RichText.new("markdown", text).to_html
assert_match %r{<a href="/user/Open%20Mapper" #{RichText::LINK_ATTRIBUTES}>&#64;[“”]Open Mapper[“”]</a>}o, html
end
end

def test_text_to_html_linkify_replace
with_settings(:linkify_hosts => ["replace-me.example.com"], :linkify_hosts_replacement => "repl.example.com") do
r = RichText.new("text", "foo https://replace-me.example.com/some/path?query=te<st&limit=20>10#result12 bar")
Expand All @@ -237,6 +244,18 @@ def test_text_to_html_linkify_replace
end
end

def test_linkify_username_with_quotes
text = "Hello @'\"Yo\"'"
html = RichText.new("markdown", text).to_html
assert_match %r{<a href="/user/%22Yo%22" #{RichText::LINK_ATTRIBUTES}>&#64;[‘'’]“Yo”[‘'’]</a>}o, html
end

def test_linkify_short_username
text = "Hello @Yo"
html = RichText.new("markdown", text).to_html
assert_match %r{<a href="/user/Yo" #{RichText::LINK_ATTRIBUTES}>&#64;Yo</a>}o, html
end

def test_text_to_html_linkify_recognize
with_settings(:linkify_hosts => ["replace-me.example.com"], :linkify_hosts_replacement => "repl.example.com") do
r = RichText.new("text", "foo repl.example.com/some/path?query=te<st&limit=20>10#result12 bar")
Expand Down Expand Up @@ -356,8 +375,10 @@ def test_text_to_html_linkify_recognize_path
with_settings(:linkify => { :detection_rules => [{ :patterns => ["@(?<username>\\w+)"], :path_template => "user/\\k<username>" }] }) do
r = RichText.new("text", "foo @example bar")
assert_html r do
assert_dom "a", :count => 1, :text => "http://test.host/user/example" do
assert_dom "> @href", "http://test.host/user/example"
# We now expect the visible text to be "@example", not the full URL
assert_dom "a", :count => 1, :text => "@example" do
# We now expect a standard relative URL
assert_dom "> @href", "/user/example"
assert_dom "> @rel", "nofollow noopener noreferrer"
end
end
Expand Down