Skip to content

Commit

Permalink
#1 'Stack level too deep' when HTML-izing something with lots of whit…
Browse files Browse the repository at this point in the history
…espace
  • Loading branch information
JasonBarnabe committed Jun 26, 2014
1 parent f35fd84 commit 87ccbd7
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 39 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ruby '2.0.0'

gem 'rails', '~> 4.0.3'
gem 'mysql2', '~> 0.3.0'
gem 'sanitize', '~> 2.0.6'
gem 'sanitize', '~> 3.0.0'
gem 'csspool', '~> 4.0.2'
gem 'public_suffix', '~> 1.3.1'
gem 'thinking-sphinx', '~> 3.1.1'
Expand Down
15 changes: 10 additions & 5 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ GEM
coffee-script-source
execjs
coffee-script-source (1.7.0)
crass (0.2.0)
csspool (4.0.2)
daemons (1.1.9)
dalli (2.6.4)
Expand All @@ -60,13 +61,15 @@ GEM
treetop (~> 1.4.8)
middleware (0.1.0)
mime-types (1.25.1)
mini_portile (0.5.2)
mini_portile (0.6.0)
minitest (4.7.5)
multi_json (1.8.4)
mysql2 (0.3.15)
newrelic_rpm (3.7.2.195)
nokogiri (1.6.1)
mini_portile (~> 0.5.0)
nokogiri (1.6.2.1)
mini_portile (= 0.6.0)
nokogumbo (1.1.9)
nokogiri
polyglot (0.3.4)
public_suffix (1.3.3)
rack (1.5.2)
Expand All @@ -91,8 +94,10 @@ GEM
ref (1.0.5)
riddle (1.5.11)
ruby-openid (2.3.0)
sanitize (2.0.6)
sanitize (3.0.0)
crass (~> 0.2.0)
nokogiri (>= 1.4.4)
nokogumbo (= 1.1.9)
sass (3.2.14)
sass-rails (4.0.1)
railties (>= 4.0.0, < 5.0)
Expand Down Expand Up @@ -155,7 +160,7 @@ DEPENDENCIES
rails-observers (~> 0.1.2)
riddle (~> 1.5.11)
ruby-openid (~> 2.3.0)
sanitize (~> 2.0.6)
sanitize (~> 3.0.0)
sass-rails
strip_attributes (~> 1.5.0)
therubyracer
Expand Down
50 changes: 17 additions & 33 deletions app/helpers/styles_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,22 +92,18 @@ def format_user_text(text)
linkify_urls = lambda do |env|
node = env[:node]
return unless node.text?
return if has_anchor_ancestor(node)
return if has_ancestor(node, 'a')
return if has_ancestor(node, 'pre')
url_reference = node.text.match(/(\s|^|\()(https?:\/\/[^\s\)\]]*)/i)
return if url_reference.nil?
resulting_nodes = replace_text_with_link(node, url_reference[2], url_reference[2], url_reference[2])
# sanitize the new nodes ourselves; they won't be picked up otherwise.
resulting_nodes.delete(node)
resulting_nodes.each do |new_node|

Sanitize.clean_node!(new_node, env[:config])
end
end

linkify_styles = lambda do |env|
node = env[:node]
return unless node.text?
return if has_anchor_ancestor(node)
return if has_ancestor(node, 'a')
return if has_ancestor(node, 'pre')
style_pattern = /(\s|^|\()(style ([0-9]+))/i
style_reference = node.text.match(style_pattern)
return if style_reference.nil?
Expand All @@ -121,11 +117,6 @@ def format_user_text(text)
style = Style.find(style_id)
resulting_nodes = replace_text_with_link(node, original_text, style.short_description, style.pretty_url)
# the current node will not contain any more references as all subsequent text will be in the newly created nodes.
# sanitize the new nodes ourselves; they won't be picked up otherwise.
resulting_nodes.delete(node)
resulting_nodes.each do |new_node|
Sanitize.clean_node!(new_node, env[:config])
end
return
rescue ActiveRecord::RecordNotFound => ex
# not a valid reference, move on to the next
Expand All @@ -138,7 +129,8 @@ def format_user_text(text)
linkify_users = lambda do |env|
node = env[:node]
return unless node.text?
return if has_anchor_ancestor(node)
return if has_ancestor(node, 'a')
return if has_ancestor(node, 'pre')
user_pattern = /(\s|^|\()(user ([0-9]+))/i
user_reference = node.text.match(user_pattern)
return if user_reference.nil?
Expand All @@ -152,11 +144,6 @@ def format_user_text(text)
user = User.find(user_id)
resulting_nodes = replace_text_with_link(node, original_text, user.name, url_for(:controller => "users", :action => "show", :id => user.id))
# the current node will not contain any more references as all subsequent text will be in the newly created nodes.
# sanitize the new nodes ourselves; they won't be picked up otherwise.
resulting_nodes.delete(node)
resulting_nodes.each do |new_node|
Sanitize.clean_node!(new_node, env[:config])
end
return
rescue ActiveRecord::RecordNotFound => ex
# not a valid reference, move on to the next
Expand All @@ -169,21 +156,12 @@ def format_user_text(text)
fix_whitespace = lambda do |env|
node = env[:node]
return unless node.text?
# stupid workaround for https://github.com/JasonBarnabe/userstyles/issues/1 - give up if the stack is too big
if caller.size > 2000
node.content = ''
return
end
node.content = node.content.lstrip if node.previous_sibling.nil? or (!node.previous_sibling.description.nil? and node.previous_sibling.description.block?)
node.content = node.content.rstrip if node.next_sibling.nil? or (!node.next_sibling.description.nil? and node.next_sibling.description.block?)
return if has_ancestor(node, 'pre')
node.content = node.content.lstrip if element_is_block(node.previous_sibling)
node.content = node.content.rstrip if element_is_block(node.next_sibling)
return if node.text.empty?
return unless node.text.include?("\n")
resulting_nodes = replace_text_with_node(node, "\n", Nokogiri::XML::Node.new('br', node.document))
# sanitize the new nodes ourselves; they won't be picked up otherwise.
resulting_nodes.delete(node)
resulting_nodes.each do |new_node|
Sanitize.clean_node!(new_node, env[:config])
end
end

config = Sanitize::Config::BASIC.merge({
Expand Down Expand Up @@ -214,13 +192,19 @@ def replace_text_with_node(node, text, node_to_insert)
return [node, node.next_sibling, node.next_sibling.next_sibling]
end

def has_anchor_ancestor(node)
def has_ancestor(node, ancestor_node_name)
until node.nil?
return true if node.name == 'a'
return true if node.name == ancestor_node_name
node = node.parent
end
return false
end

def element_is_block(node)
return false if node.nil?
# https://github.com/rgrove/sanitize/issues/108
d = Nokogiri::HTML::ElementDescription[node.name]
return !d.nil? && d.block?
end

end

0 comments on commit 87ccbd7

Please sign in to comment.