Permalink
Browse files

do not return html safe strings from auto_link

  • Loading branch information...
tenderlove committed Apr 5, 2011
1 parent b13be61 commit 61ee3449674c591747db95f9b3472c5c3bd9e84d
Showing with 13 additions and 7 deletions.
  1. +2 −2 actionpack/lib/action_view/helpers/text_helper.rb
  2. +11 −5 actionpack/test/template/text_helper_test.rb
@@ -299,7 +299,7 @@ def simple_format(text, html_options={}, options={})
# # => "Welcome to my new blog at <a href=\"http://www.myblog.com/\" target=\"_blank\">http://www.myblog.com</a>.
# Please e-mail me at <a href=\"mailto:me@email.com\">me@email.com</a>."
def auto_link(text, *args, &block)#link = :all, html = {}, &block)
- return ''.html_safe if text.blank?
+ return '' if text.blank?
options = args.size == 2 ? {} : args.extract_options! # this is necessary because the old auto_link API has a Hash as its last parameter
unless args.empty?
@@ -503,7 +503,7 @@ def auto_link_urls(text, html_options = {}, options = {})
end
content_tag(:a, link_text, link_attributes.merge('href' => href), !!options[:sanitize]) + punctuation.reverse.join('')
end
- end.html_safe
+ end
end
# Turns all email addresses into clickable links. If a block is given,
@@ -315,14 +315,20 @@ def generate_result(link_text, href = nil, escape = false)
end
end
- def test_auto_link_should_be_html_safe
+ def test_auto_link_should_not_be_html_safe
email_raw = 'santiago@wyeworks.com'
link_raw = 'http://www.rubyonrails.org'
- assert auto_link(nil).html_safe?
- assert auto_link('').html_safe?
- assert auto_link("#{link_raw} #{link_raw} #{link_raw}").html_safe?
- assert auto_link("hello #{email_raw}").html_safe?
+ assert !auto_link(nil).html_safe?, 'should not be html safe'
+ assert !auto_link('').html_safe?, 'should not be html safe'
+ assert !auto_link("#{link_raw} #{link_raw} #{link_raw}").html_safe?, 'should not be html safe'
+ assert !auto_link("hello #{email_raw}").html_safe?, 'should not be html safe'
+ end
+
+ def test_auto_link_email_address
+ email_raw = 'aaron@tenderlovemaking.com'
+ email_result = %{<a href="mailto:#{email_raw}">#{email_raw}</a>}
+ assert !auto_link_email_addresses(email_result).html_safe?, 'should not be html safe'
end
def test_auto_link

1 comment on commit 61ee344

Contributor

xuanxu commented on 61ee344 Apr 15, 2011

IMHO is a better option to have auto_link return a sanitized string.
This code returns insecure content:
auto_link("<script>alert('malicious')</script> www.rubyonrails.org", :sanitize => true)
which I think is the not expected result

I propose to avoid the vulnerability and at the same time give a better use to the existent (but not documented) :sanitize option.
I've issued a pull request here: #281

Please sign in to comment.