rails/rails — pull request #3716 by marcandre (2011-11-22 05:24:03+00:00)
Title: Make Array#join aware of SafeBuffers.

Currently, Array#join is not aware of SafeBuffers.

Because of that, `[foo, bar].join` is not the same to `foo+bar` if `foo` is a SafeBuffer.

This can lead many programmers to call `.html_safe` on the resulting string, which can be wrong and/or dangerous. I say "many programmers" judging from code I've seen (from good programmers), from this [accepted answer on StackOverflow](http://stackoverflow.com/questions/4191028/rails-3-0-2-arrayjoin-html-safe), or [warnings on blogs](https://makandracards.com/makandra/954-don-t-mix-array-join-and-string-html_safe).

This has been requested before (at least [here](https://github.com/NZKoz/rails_xss/issues/15) and [here](https://github.com/rails/rails/issues/2544) ), but no implementation was given.

Let me argue why patching `Array#join` is worth it.

1) It should not be too expensive to patch, since only the first element of the array needs to be checked. If it is not a SafeBuffer, we call the original `Array#join` and minimal time is spent (with a similar result as doing `first + second + ...`).

Otherwise we can safely expect that the safe join is what needs to be done (and the cost that is paid is cheap, since the implementation I propose will be more efficient than what many would write rolling their own)

2) Doing a correct and safe join is not easy! Calling `ary.inject(:+)` will be ineficient for large arrays, calling `ary.inject(:<<)` will mutate the first element, calling `ary.inject(:<<, "".html_safe)` is both long, ugly and not obvious for other readers. Note that doing a join with a separator is even more complicated.

3) Let me stress that the downside of leaving Array#join untouched is running the security risk that programmers will use `.join.html_safe` when they shouldn't. If they do, they are unlikely to notice a problem until someone uses that for an XSS attack...

Thanks