Merge pull request rails#54643 from etiennebarrie/dont-escape-json-when-unnecessary

Don't always escape JSON when calling `render json:`

Author: byroot

actionpack/CHANGELOG.md

@@ -1,3 +1,27 @@
+*   The JSON renderer doesn't escape HTML entities or Unicode line separators anymore.
+
+    Using `render json:` will no longer escape `<`, `>`, `&`, `U+2028` and `U+2029` 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 or set `config.action_controller.escape_json_responses` to `true` 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

@@ -29,6 +29,7 @@ module Renderers
 
     included do
       class_attribute :_renderers, default: Set.new.freeze
+      class_attribute :escape_json_responses, instance_accessor: false, default: true
     end
 
     # Used in ActionController::Base and ActionController::API to include all
@@ -86,7 +87,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 +141,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 +154,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 if !self.class.escape_json_responses? && options[:callback].blank?
       json = json.to_json(json_options) unless json.kind_of?(String)
 
       if options[:callback].present?

actionpack/lib/action_controller/railtie.rb

@@ -133,5 +133,18 @@ class Railtie < Rails::Railtie # :nodoc:
         ActionController::TestCase.executor_around_each_request = app.config.active_support.executor_around_test_case
       end
     end
+
+    initializer "action_controller.escape_json_responses_deprecated_warning" do
+      config.after_initialize do
+        ActiveSupport.on_load(:action_controller) do
+          if ActionController::Base.escape_json_responses
+            ActionController.deprecator.warn(<<~MSG.squish)
+              Setting action_controller.escape_json_responses = true is deprecated and will have no effect in Rails 8.2.
+              Set it to `false` or use `config.load_defaults(8.1)`.
+            MSG
+          end
+        end
+      end
+    end
   end
 end

actionpack/test/controller/render_json_test.rb

@@ -3,6 +3,7 @@
 require "abstract_unit"
 require "controller/fake_models"
 require "active_support/logger"
+require "active_support/core_ext/object/with"
 
 class RenderJsonTest < ActionController::TestCase
   class JsonRenderable
@@ -46,6 +47,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 +115,20 @@ 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_with_new_default_and_without_callback_does_not_escape_js_chars
+    TestController.with(escape_json_responses: false) do
+      get :render_json_unsafe_chars_without_callback
+      assert_equal %({"hello":"\u2028\u2029<script>"}), @response.body
+      assert_equal "application/json", @response.media_type
+    end
+  end
+
   def test_render_json_with_custom_content_type
     get :render_json_with_custom_content_type, xhr: true
     assert_equal '{"hello":"world"}', @response.body

guides/source/configuring.md

@@ -60,6 +60,7 @@ Below are the default values associated with each target version. In cases of co
 
 #### Default Values for Target Version 8.1
 
+- [`config.action_controller.escape_json_responses`](#config-action-controller-escape-json-responses): `false`
 - [`config.yjit`](#config-yjit): `!Rails.env.local?`
 
 #### Default Values for Target Version 8.0
@@ -1964,6 +1965,21 @@ The default value depends on the `config.load_defaults` target version:
 Configures the [`ParamsWrapper`](https://api.rubyonrails.org/classes/ActionController/ParamsWrapper.html). This can be called at
 the top level, or on individual controllers.
 
+#### `config.action_controller.escape_json_responses`
+
+Configures the JSON renderer to escape HTML entities and Unicode characters that are invalid in JavaScript.
+
+This is useful if you relied on the JSON response having those characters escaped to embed the JSON document in
+\<script> tags in HTML.
+
+This is mainly for compatibility when upgrading Rails applications, otherwise you can use the `:escape` option for
+`render json:` in specific controller actions.
+
+| Starting with version | The default value is |
+| --------------------- | -------------------- |
+| (original)            | `true`               |
+| 8.1                   | `false`              |
+
 ### Configuring Action Dispatch
 
 #### `config.action_dispatch.cookies_serializer`

railties/lib/rails/application/configuration.rb

@@ -355,6 +355,10 @@ def load_defaults(target_version)
           # redefine methods (e.g. mocking), hence YJIT isn't generally
           # faster in these environments.
           self.yjit = !Rails.env.local?
+
+          if respond_to?(:action_controller)
+            action_controller.escape_json_responses = false
+          end
         else
           raise "Unknown version #{target_version.to_s.inspect}"
         end

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_8_1.rb.tt

@@ -8,3 +8,21 @@
 #
 # Read the Guide for Upgrading Ruby on Rails for more info on each option.
 # https://guides.rubyonrails.org/upgrading_ruby_on_rails.html
+
+###
+# Skips escaping HTML entities and line separators. When set to `false`, the
+# JSON renderer no longer escapes these to improve performance.
+#
+# Example:
+#   class PostsController < ApplicationController
+#     def index
+#       render json: { key: "\u2028\u2029<>&" }
+#     end
+#   end
+#
+# Renders `{"key":"\u2028\u2029\u003c\u003e\u0026"}` with the previous default, but `{"key":"

<>&"}` with the config
+# set to `false`.
+#
+# Applications that want to keep the escaping behavior can set the config to `true`.
+#++
+# Rails.configuration.action_controller.escape_json_responses = false