Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Do not modify a safe buffer in helpers

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information...
commit ed3796434af6069ced6a641293cf88eef3b284da 1 parent 53a2c0b
Bruno Michel nono authored tenderlove committed
40 actionpack/lib/action_view/helpers/text_helper.rb
@@ -115,13 +115,12 @@ def highlight(text, phrases, *args)
115 115 end
116 116 options.reverse_merge!(:highlighter => '<strong class="highlight">\1</strong>')
117 117
118   - text = sanitize(text) unless options[:sanitize] == false
119   - if text.blank? || phrases.blank?
120   - text
121   - else
  118 + if text.present? && phrases.present?
122 119 match = Array(phrases).map { |p| Regexp.escape(p) }.join('|')
123   - text.gsub(/(#{match})(?!(?:[^<]*?)(?:["'])[^<>]*>)/i, options[:highlighter])
124   - end.html_safe
  120 + text = text.to_str.gsub(/(#{match})(?!(?:[^<]*?)(?:["'])[^<>]*>)/i, options[:highlighter])
  121 + end
  122 + text = sanitize(text) unless options[:sanitize] == false
  123 + text
125 124 end
126 125
127 126 # Extracts an excerpt from +text+ that matches the first instance of +phrase+.
@@ -251,14 +250,16 @@ def word_wrap(text, *args)
251 250 # simple_format("Look ma! A class!", :class => 'description')
252 251 # # => "<p class='description'>Look ma! A class!</p>"
253 252 def simple_format(text, html_options={}, options={})
254   - text = ''.html_safe if text.nil?
  253 + text = text ? text.to_str : ''
  254 + text = text.dup if text.frozen?
255 255 start_tag = tag('p', html_options, true)
256   - text = sanitize(text) unless options[:sanitize] == false
257 256 text.gsub!(/\r\n?/, "\n") # \r\n and \r -> \n
258 257 text.gsub!(/\n\n+/, "</p>\n\n#{start_tag}") # 2+ newline -> paragraph
259 258 text.gsub!(/([^\n]\n)(?=[^\n])/, '\1<br />') # 1 newline -> br
260 259 text.insert 0, start_tag
261   - text.html_safe.safe_concat("</p>")
  260 + text.concat("</p>")
  261 + text = sanitize(text) unless options[:sanitize] == false
  262 + text
262 263 end
263 264
264 265 # Turns all URLs and e-mail addresses into clickable links. The <tt>:link</tt> option
@@ -477,7 +478,7 @@ def set_cycle(name, cycle_object)
477 478 # is yielded and the result is used as the link text.
478 479 def auto_link_urls(text, html_options = {}, options = {})
479 480 link_attributes = html_options.stringify_keys
480   - text.gsub(AUTO_LINK_RE) do
  481 + text.to_str.gsub(AUTO_LINK_RE) do
481 482 scheme, href = $1, $&
482 483 punctuation = []
483 484
@@ -494,14 +495,11 @@ def auto_link_urls(text, html_options = {}, options = {})
494 495 end
495 496 end
496 497
497   - link_text = block_given?? yield(href) : href
  498 + link_text = block_given? ? yield(href) : href
498 499 href = 'http://' + href unless scheme
499 500
500   - unless options[:sanitize] == false
501   - link_text = sanitize(link_text)
502   - href = sanitize(href)
503   - end
504   - content_tag(:a, link_text, link_attributes.merge('href' => href), !!options[:sanitize]) + punctuation.reverse.join('')
  501 + sanitize = options[:sanitize] != false
  502 + content_tag(:a, link_text, link_attributes.merge('href' => href), sanitize) + punctuation.reverse.join('')
505 503 end
506 504 end
507 505 end
@@ -509,18 +507,14 @@ def auto_link_urls(text, html_options = {}, options = {})
509 507 # Turns all email addresses into clickable links. If a block is given,
510 508 # each email is yielded and the result is used as the link text.
511 509 def auto_link_email_addresses(text, html_options = {}, options = {})
512   - text.gsub(AUTO_EMAIL_RE) do
  510 + text.to_str.gsub(AUTO_EMAIL_RE) do
513 511 text = $&
514 512
515 513 if auto_linked?($`, $')
516 514 text.html_safe
517 515 else
518   - display_text = (block_given?) ? yield(text) : text
519   -
520   - unless options[:sanitize] == false
521   - text = sanitize(text)
522   - display_text = sanitize(display_text) unless text == display_text
523   - end
  516 + display_text = block_given? ? yield(text) : text
  517 + display_text = sanitize(display_text) unless options[:sanitize] == false
524 518 mail_to text, display_text, html_options
525 519 end
526 520 end
26 actionpack/test/template/text_helper_test.rb
@@ -48,6 +48,10 @@ def test_simple_format_should_not_sanitize_input_when_sanitize_option_is_false
48 48 assert_equal "<p><b> test with unsafe string </b><script>code!</script></p>", simple_format("<b> test with unsafe string </b><script>code!</script>", {}, :sanitize => false)
49 49 end
50 50
  51 + def test_simple_format_should_not_be_html_safe_when_sanitize_option_is_false
  52 + assert !simple_format("<b> test with unsafe string </b><script>code!</script>", {}, :sanitize => false).html_safe?
  53 + end
  54 +
51 55 def test_truncate_should_not_be_html_safe
52 56 assert !truncate("Hello World!", :length => 12).html_safe?
53 57 end
@@ -166,6 +170,13 @@ def test_highlight_with_options_hash
166 170 )
167 171 end
168 172
  173 + def test_highlight_on_an_html_safe_string
  174 + assert_equal(
  175 + "<p>This is a <b>beautiful</b> morning, but also a <b>beautiful</b> day</p>",
  176 + highlight("<p>This is a beautiful morning, but also a beautiful day</p>".html_safe, "beautiful", :highlighter => '<b>\1</b>')
  177 + )
  178 + end
  179 +
169 180 def test_highlight_with_html
170 181 assert_equal(
171 182 "<p>This is a <strong class=\"highlight\">beautiful</strong> morning, but also a <strong class=\"highlight\">beautiful</strong> day</p>",
@@ -306,13 +317,10 @@ def test_auto_link_parsing
306 317 end
307 318 end
308 319
309   - def generate_result(link_text, href = nil, escape = false)
310   - href ||= link_text
311   - if escape
312   - %{<a href="#{CGI::escapeHTML href}">#{CGI::escapeHTML link_text}</a>}
313   - else
314   - %{<a href="#{href}">#{link_text}</a>}
315   - end
  320 + def generate_result(link_text, href = nil)
  321 + href = CGI::escapeHTML(href || link_text)
  322 + text = CGI::escapeHTML(link_text)
  323 + %{<a href="#{href}">#{text}</a>}
316 324 end
317 325
318 326 def test_auto_link_should_not_be_html_safe
@@ -323,6 +331,8 @@ def test_auto_link_should_not_be_html_safe
323 331 assert !auto_link('').html_safe?, 'should not be html safe'
324 332 assert !auto_link("#{link_raw} #{link_raw} #{link_raw}").html_safe?, 'should not be html safe'
325 333 assert !auto_link("hello #{email_raw}").html_safe?, 'should not be html safe'
  334 + assert !auto_link(link_raw.html_safe).html_safe?, 'should not be html safe'
  335 + assert !auto_link(email_raw.html_safe).html_safe?, 'should not be html safe'
326 336 end
327 337
328 338 def test_auto_link_email_address
@@ -425,7 +435,7 @@ def test_auto_link
425 435
426 436 def test_auto_link_should_sanitize_input_when_sanitize_option_is_not_false
427 437 link_raw = %{http://www.rubyonrails.com?id=1&num=2}
428   - assert_equal %{<a href="http://www.rubyonrails.com?id=1&num=2">http://www.rubyonrails.com?id=1&num=2</a>}, auto_link(link_raw)
  438 + assert_equal %{<a href="http://www.rubyonrails.com?id=1&amp;num=2">http://www.rubyonrails.com?id=1&amp;num=2</a>}, auto_link(link_raw)
429 439 end
430 440
431 441 def test_auto_link_should_not_sanitize_input_when_sanitize_option_is_false

0 comments on commit ed37964

Please sign in to comment.
Something went wrong with that request. Please try again.