#3767 [PATCH] simple_format output should not be HTML-escaped in Rails 3 - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#3767 ✓committed

[PATCH] simple_format output should not be HTML-escaped in Rails 3

Reported by Adam McCrea | January 21st, 2010 @ 06:20 PM | in 2.3.6

Since the default behavior of Rails 3 is to escape all Ruby strings in HTML, we're forced to use raw() on simple_format() to get it behaving as expected. IMO, since the purpose of simple_format is to inject HTML into a string, it should also take care to prevent that HTML from being escaped.

Likewise, I think simple_format() should automatically escape the string that is passed to it. This seems to go along with the Rails 3 assumption that no string is safe unless explicitly stated.

Separate patches are attached for each of these changes.

Comments and changes to this ticket

You flagged this item as spam.
Stephen Celis February 8th, 2010 @ 02:50 PM
- Assigned user set to “Pratik”
Applies cleanly, but doesn't work as intended due to safe_concat.

Here's an additional patch (to retain new test cases) that uses String#insert for the final, closing "
". +1 otherwise.
- avoid-safe-concat.patch 947 Bytes
You flagged this item as spam.
Stephen Celis February 8th, 2010 @ 02:51 PM
I guess Lighthouse is raw(). That was supposed to read "closing '</p>'".
Jeremy Kemper February 8th, 2010 @ 08:27 PM
- State changed from “new” to “open”
- Milestone cleared.
Santiago Pastorino February 12th, 2010 @ 07:09 PM
- Assigned user changed from “Pratik” to “Yehuda Katz (wycats)”
Here is the correct patch
- simple_format_escapes_unsafe_input.patch 2.5 KB
Santiago Pastorino February 13th, 2010 @ 01:17 AM
New patch after DHH change
Santiago Pastorino February 13th, 2010 @ 01:20 AM
- no changes were found...
Repository February 13th, 2010 @ 01:25 AM
- State changed from “open” to “committed”
(from [4158282e32bf0a7d9fbb1a7669ade2226f909b12]) simple_format returns a safe buffer escaping unsafe input [Santiago Pastorino] (Closes #3767)

Signed-off-by: David Heinemeier Hansson david@loudthinking.com
http://github.com/rails/rails/commit/4158282e32bf0a7d9fbb1a7669ade2...
You flagged this item as spam.
David Reese May 7th, 2010 @ 05:51 PM
This should not have been applied to Rails 2.3! When moving to Rails 3, I would expect more escaping by default, but this change will jump out and bite many of us running "2.3 stable".

As Adam wrote, "Since the default behavior of Rails 3...".

Also, a documentation change would have been helpful -- at least mentioning that the incoming string will be escaped.
Santiago Pastorino May 7th, 2010 @ 07:48 PM
- Milestone set to “2.3.6”
- State changed from “committed” to “open”
- Assigned user changed from “Yehuda Katz (wycats)” to “Jeremy Kemper”
David you're right, we should escape this on rails_xss, sorry my bad.

On rails
http://github.com/spastorino/rails/commit/571593da45834dae10b20cbd8...

On rails_xss (I can merge this one)
http://github.com/spastorino/rails_xss/commit/96cb0059f79d62e3660a9...
Santiago Pastorino May 7th, 2010 @ 07:55 PM
I've removed a test that i shouldn't here is the right patch with [#OMG] thing
- simple_format-2-3-stable.patch 2.1 KB
Repository May 7th, 2010 @ 07:57 PM
- State changed from “open” to “committed”
(from [adcfb4e8bd886fe9d14a9b97afdb9182d8abdc95]) simple_format should return html_safe but not escape text, that's for rails_xss plugin [#3767 state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/adcfb4e8bd886fe9d14a9b97afdb91...