Don't always escape JSON when calling render json:

When `render json:` is used in a controller, and there's no callback
option (used for JSONP), the resulting JSON document doesn't need to be
HTML-safe (no need to escape HTML entities) or embeddable into
JavaScript (no need to escape U+2028 and U+2029).

This both saves a costly operation and renders cleaner JSON.

Co-authored-by: Jean Boussier <[email protected]>

Author: etiennebarrie

actionpack/CHANGELOG.md

@@ -1,3 +1,26 @@
+*   The JSON renderer doesn't escape HTML entities or Unicode line separators anymore.
+
+    Using `render json:` will no longer escape a few characters that can cause errors when the resulting JSON is
+    embedded in JavaScript, or vulnerabilities when the resulting JSON is embedded in HTML.
+
+    Since the renderer is used to return a JSON document as `application/json`, it's typically not necessary to escape
+    those characters, and it improves performance.
+
+    Escaping will still occur when the `:callback` option is set, since the JSON is used as JavaScript code in this
+    situation (JSONP).
+
+    You can use the `:escape` option to restore the escaping behavior.
+
+    ```ruby
+    class PostsController < ApplicationController
+      def index
+        render json: Post.last(30), escape: true
+      end
+    end
+    ```
+
+    *Étienne Barrié*, *Jean Boussier*
+
 *   Load lazy route sets before inserting test routes
 
     Without loading lazy route sets early, we miss `after_routes_loaded` callbacks, or risk

actionpack/lib/action_controller/metal/renderers.rb

@@ -86,7 +86,7 @@ def self.remove(key)
       remove_possible_method(method_name)
     end
 
-    def self._render_with_renderer_method_name(key)
+    def self._render_with_renderer_method_name(key) # :nodoc:
       "_render_with_renderer_#{key}"
     end
 
@@ -140,7 +140,7 @@ def render_to_body(options)
       _render_to_body_with_renderer(options) || super
     end
 
-    def _render_to_body_with_renderer(options)
+    def _render_to_body_with_renderer(options) # :nodoc:
       _renderers.each do |name|
         if options.key?(name)
           _process_options(options)
@@ -153,6 +153,7 @@ def _render_to_body_with_renderer(options)
 
     add :json do |json, options|
       json_options = options.except(:callback, :content_type, :status)
+      json_options[:escape] ||= false unless options[:callback].present?
       json = json.to_json(json_options) unless json.kind_of?(String)
 
       if options[:callback].present?

actionpack/test/controller/render_json_test.rb

@@ -46,6 +46,14 @@ def render_json_hello_world_with_callback
       render json: ActiveSupport::JSON.encode(hello: "world"), callback: "alert"
     end
 
+    def render_json_unsafe_chars_with_callback
+      render json: { hello: "\u2028\u2029<script>" }, callback: "alert"
+    end
+
+    def render_json_unsafe_chars_without_callback
+      render json: { hello: "\u2028\u2029<script>" }
+    end
+
     def render_json_with_custom_content_type
       render json: ActiveSupport::JSON.encode(hello: "world"), content_type: "text/javascript"
     end
@@ -106,6 +114,18 @@ def test_render_json_with_callback
     assert_equal "text/javascript", @response.media_type
   end
 
+  def test_render_json_with_callback_escapes_js_chars
+    get :render_json_unsafe_chars_with_callback, xhr: true
+    assert_equal '/**/alert({"hello":"\\u2028\\u2029\\u003cscript\\u003e"})', @response.body
+    assert_equal "text/javascript", @response.media_type
+  end
+
+  def test_render_json_without_callback_does_not_escape_js_chars
+    get :render_json_unsafe_chars_without_callback
+    assert_equal %({"hello":"\u2028\u2029<script>"}), @response.body
+    assert_equal "application/json", @response.media_type
+  end
+
   def test_render_json_with_custom_content_type
     get :render_json_with_custom_content_type, xhr: true
     assert_equal '{"hello":"world"}', @response.body
@@ -137,6 +157,6 @@ def test_render_json_calls_to_json_from_object
 
   def test_render_json_avoids_view_options
     get :render_json_inspect_options
-    assert_equal '{"options":{}}', @response.body
+    assert_equal '{"options":{"escape":false}}', @response.body
   end
 end