Merge pull request #2740 from basecamp/autolink-skip-large-text-nodes
Skip auto-linking in very large text nodes
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
module HtmlHelper
|
||||
def format_html(html)
|
||||
@auto_link_scrubber ||= AutoLinkScrubber.new
|
||||
Loofah::HTML5::DocumentFragment.parse(html).scrub!(@auto_link_scrubber).to_html.html_safe
|
||||
Loofah::HTML5::DocumentFragment.parse(html).scrub!(AutoLinkScrubber.new).to_html.html_safe
|
||||
end
|
||||
|
||||
def card_html_title(card)
|
||||
|
||||
@@ -18,15 +18,16 @@ class AutoLinkScrubber < Loofah::Scrubber
|
||||
TRAILING_PUNCTUATION = %(.?,:!;"'<>)
|
||||
TRAILING_PUNCTUATION_REGEXP = /[#{Regexp.escape(TRAILING_PUNCTUATION)}]+\z/
|
||||
|
||||
MAX_TEXT_NODE_LENGTH = 10_000
|
||||
|
||||
def initialize
|
||||
@direction = :top_down
|
||||
@regexp_timeout_reported = false
|
||||
end
|
||||
|
||||
def scrub(node)
|
||||
return Loofah::Scrubber::STOP if EXCLUDED_ELEMENTS.include?(node.name)
|
||||
|
||||
if node.text? && !@regexp_timeout_reported
|
||||
if node.text?
|
||||
replacement = autolink_text_node(node)
|
||||
node.replace(replacement) if replacement
|
||||
end
|
||||
@@ -56,6 +57,8 @@ class AutoLinkScrubber < Loofah::Scrubber
|
||||
end
|
||||
|
||||
def find_links(text)
|
||||
return [] if text.length > MAX_TEXT_NODE_LENGTH
|
||||
|
||||
links = []
|
||||
|
||||
text.scan(AUTOLINK_REGEXP) do
|
||||
@@ -74,7 +77,6 @@ class AutoLinkScrubber < Loofah::Scrubber
|
||||
links
|
||||
rescue Regexp::TimeoutError => error
|
||||
Sentry.capture_exception error if Fizzy.saas?
|
||||
@regexp_timeout_reported = true
|
||||
[]
|
||||
end
|
||||
|
||||
|
||||
@@ -125,6 +125,17 @@ class HtmlHelperTest < ActionView::TestCase
|
||||
end
|
||||
end
|
||||
|
||||
test "skip auto-linking in very large text nodes" do
|
||||
url = "https://example.com"
|
||||
large_text = "x" * 5_000 + " #{url} " + "y" * 5_000
|
||||
input = "<p>#{large_text}</p>"
|
||||
|
||||
result = format_html(input)
|
||||
|
||||
assert_no_match(/<a/, result)
|
||||
assert_includes result, url
|
||||
end
|
||||
|
||||
test "don't autolink content in excluded elements" do
|
||||
%w[ figcaption pre code ].each do |element|
|
||||
assert_equal_html \
|
||||
|
||||
Reference in New Issue
Block a user