#4878 truncate should always return unsafe strings - 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.

#4878 ✓committed

truncate should always return unsafe strings

Reported by Michael Koziarski | June 16th, 2010 @ 10:25 PM | in 3.0.2

As mentioned in #4825 the change to using santize in truncate has broken some valid use cases. e.g. descriptions such as:

  <script> tags not working in admin section

Instead the helper should simply return the strings as unsafe, and let the user either mark them as raw() or rely on the default escaping.

We can't use the html_safety of the input to determine whether to return a safe string because of cases like this:

  <%= truncate(h("wtf&"), 4) %>

Comments and changes to this ticket

You flagged this item as spam.
Wincent Colaiuta June 17th, 2010 @ 07:43 AM
Proposed fix pushed to the "ticket4878" branch of my fork:

http://github.com/wincent/rails/tree/ticket4878

Specifically, this commit:

http://github.com/wincent/rails/commit/157db6a2f7df1924f0c738fa4ac1...

I've modified the tests, but I am having trouble running the test suite on this machine so I'd appreciate it if someone could confirm that this is correct.
You flagged this item as spam.
Wincent Colaiuta June 17th, 2010 @ 08:02 AM
Ok, fixed the problems in my test environment. Confirmed that everything passes.
You flagged this item as spam.
Wincent Colaiuta June 17th, 2010 @ 08:04 AM
- Tag set to “patch”
Adding patch.
- 0001-truncate-should-not-try-to-produce-html-safe-output.patch 4 KB
Santiago Pastorino June 17th, 2010 @ 11:40 AM
- State changed from “new” to “verified”
+1 yesterday i did the patch and is the same as yours, but i'm attaching the patch again maintaining Wincent as the author of the original patch with one more test for excerpt method that shouldn't return safe too, so please someone of the core go ahead and apply it ;).
- truncate-should-not-try-to-produce-html-safe-output.patch 5 KB
Repository June 17th, 2010 @ 03:41 PM
(from [7240a960f2e515316ac4d962482f678b67b83b8c]) excerpt shoudn't return safe output test added

[#4878]

Signed-off-by: David Heinemeier Hansson david@loudthinking.com
http://github.com/rails/rails/commit/7240a960f2e515316ac4d962482f67...
DHH June 17th, 2010 @ 03:42 PM
- State changed from “verified” to “committed”
You flagged this item as spam.
Wincent Colaiuta June 17th, 2010 @ 03:46 PM
Still not fixed though.

Commit 7240a960 contains only Santiago's additional test for the "excerpt" method.

Missing is my patch, the one which actually addresses the subject of this ticket ("truncate should always return unsafe strings").
Rohit Arondekar June 17th, 2010 @ 03:48 PM
Wincent, actually it does include the patch. http://github.com/rails/rails/commit/a424808df95c5ebe66d02f0d743e74...
You flagged this item as spam.
Wincent Colaiuta June 17th, 2010 @ 03:50 PM
Ah, cool.
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
- Importance changed from “” to “Low”
[bulk edit]