Concat calls in plugins/gems are escaped

[dpickett]

IE

in binarylogic/searchlogic (line 69 of rails_helpers.rb):

concat(content_tag("div", hidden_field_tag("#{args.first}[order]", search_obj.order)) + "\n")

and in dpickett/tab_menu (line 15 of view_helpers.rb):

concat("<ul #{html_option_strings.join}>")

[NZKoz]

The first case is an interesting one, it's not 'html_safe?' because there's the + "\n" in there, another option would be to make String#+ escape it's other operands instead of just marking the result as unsafe... Not sure what the right fix is there, do you have any thoughts?

That second case is expected behaviour, you should be able to use content_tag or tag to build those up and have it work without issue.

[dpickett]

ok, that makes sense - I patched tab_menu

I wouldn't classify \n or \r as un-safe - should the check call "strip" on the string before checking?

[NZKoz]

yeah, \n isn't 'unsafe' in the sense that it's an attack vector, but with the new xss code everything is considered unsafe unless marked safe. So the options when adding an unsafe string to a safe one are:

1. new_string = (self + other).unsafe!
2. new_string = (self + escape(other)).safe!

Currently we do 1, and my gut says that's the right approach, though this single trailing newline version is pretty annoying....