This project is archived and is in readonly mode.
strip_tags marks as html_safe incorrectly
Reported by Andreas Mayer | October 29th, 2010 @ 02:10 PM
Rails 3.0.0: strip_tags marks its result as html_safe but forgets that there may be quotes that must be converted to entities before the string is really HTML safe.
irb(main):007:0> h("Strip <i>these</i> \"tags\"!")
=> "Strip <i>these</i> "tags"!"
irb(main):008:0> strip_tags("Strip <i>these</i> \"tags\"!")
=> "Strip these \"tags\"!"
irb(main):009:0> h(strip_tags("Strip <i>these</i> \"tags\"!"))
=> "Strip these \"tags\"!"
Expected results:
h(strip_tags("Strip <i>these</i> \"tags\"!"))
-> "Strip these "tags"!"
So either strip_tags should convert the quotes or it shouldn't mark as html_safe.
Comments and changes to this ticket
-
Andreas Mayer October 29th, 2010 @ 02:28 PM
Workaround:
h(String.new(strip_tags("Strip <i>these</i> \"tags\"!"))) -
Andreas Mayer October 29th, 2010 @ 03:12 PM
- Tag changed from “rails 3.0.0, html_safe” to “rails 3.0.0, html_safe, patch”
please review
-
Andreas Mayer October 30th, 2010 @ 02:31 PM
- Tag changed from “rails 3.0.0, html_safe, patch” to “rails 3.0.1, actionview, html_safe, patch”
-
Christopher Meiklejohn March 3rd, 2011 @ 05:19 AM
Just verified this patch applies cleanly to master and has the desired effect.
ree-1.8.7-head :003 > strip_tags("<I>HI \"YOU\" </I>").html_safe? => falseCan we get this pulled in?
-
Christopher Meiklejohn March 3rd, 2011 @ 05:28 AM
Actually, disregard.
ActionView::Helpers::SanitizeHelper specifically states that once the string is html_safe it may still contain unescaped characters.
Taken from #sanitize:
Please note that sanitizing user-provided text does not guarantee that the resulting markup is valid (conforming to a document type) or even well-formed. The output may still contain e.g. unescaped ‘<’, ‘>’, ‘&’ characters and confuse browsers.Seems like this isn't an issue, and should be marked invalid.
-
Andreas Mayer March 3rd, 2011 @ 10:30 PM
@Christopher: I think it's absolutely OK for strip_tags to return text that is not valid in any form (it only strips tags, and uses a very simple parse for this so no guarantee for anything). However, why has this resulting string to be marked as html_safe? I can't read this in the docs you quoted and it's that what I wrote the patch for.
This report is the result of a problem that really occured and I think it makes no sense that a non-HTML-safe string is marked as html_safe - it means that you
1) can the Rails XSS protection using strip_tags, AND
2) that you always have to write
h(String.new(strip_tags(original_string)))
in your views. -
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Andreas Mayer
Christopher Meiklejohn