Add an option to escape forward slash character

[casperisfine]

This is a rebase of #235.

As per the spec, the forward slash should be escaped in json strings. Not escaping the forward slash causes a security issue when a json object is interpolated inside of a script tag.

For instance:

irb(main):013:0> include ActionView::Helpers::TagHelper
=> Object
irb(main):014:0> include ActionView::Helpers::JavaScriptHelper
=> Object
irb(main):015:0> puts javascript_tag 'var json='+JSON.generate({"key"=>"</script><script>whatever()//"})
<script>
//<![CDATA[
var json={"key":"</script><script>whatever()//"}
//]]>
</script>

In javascript </script> has precedence over everything else, so in this case the script tag is terminated and whatever() is executed even though it should be part of the string.

The correct way to defend against this security issue is to escape the forward slash, so that </script> does not terminate the original script tag and the json string stays a string.

[casperisfine]

@hsbt any chance this could be considered?

[marcandre]

Great PR. A few minor observations

[marcandre]

If I'm not mistaken, the default for escape_slash is false in general, but it is true for j, jj, dump_default_options and thus JSON.generate? I don't know if that the best default.

[casperisfine]

Thanks for the review. I'll update the PR soon.

[casperisfine]

@marcandre all done!

[casperisfine]

If I'm not mistaken, the default for escape_slash is false in general, but it is true for j, jj, dump_default_options and thus JSON.generate? I don't know if that the best default.

I just pushed another commit to address this. These were leftovers from #235 which made escape_slash enabled by default, but that was deemed a compatibility breakage.

IMHO it would be preferable to enable it by default at some point, but I understand how it's better to be cautious. So let's keep it purely opt-in for now.

[marcandre]

Good job! LGTM

[marcandre]

Documentation has been greatly improved; @casperisfine could you resolve the conflicts?

[casperisfine]

Done.

[marcandre]

Thanks @casperisfine 👍

@hsbt thank you in advance for merging this

[casperisfine]

Any news on this?

[marcandre]

@hsbt can this be merged?

[hsbt]

I have no opnion about this.

@nurse Can you review this?

[nurse]

As flori said in #235 (comment), he agreed this feature if it's configurable and disabled by default. And the policy sounds reasonable for me, the release manager of Ruby 2.8/3.0. Now this PR follows the comment and it's considered as acceptable. I merge this.