Use <script type="application/json"> with JSON content instead of att…

…ribute

Initial implementation by Elias Lopez Gutierrez <[email protected]>

app/helpers/react_on_rails_helper.rb

@@ -103,13 +103,10 @@ def react_component(component_name, raw_options = {})
 
     # Setup the page_loaded_js, which is the same regardless of prerendering or not!
     # The reason is that React is smart about not doing extra work if the server rendering did its job.
-
-    component_specification_tag =
-      content_tag(:div,
-                  "",
-                  class: "js-react-on-rails-component",
-                  style: options.style,
-                  data: options.data)
+    component_specification_tag = content_tag(:script,
+                                              json_safe_and_pretty(options.data).html_safe,
+                                              type: "application/json",
+                                              class: "js-react-on-rails-component")
 
     # Create the HTML rendering part
     result = server_rendered_react_component_html(options.props, options.name, options.dom_id,
@@ -174,7 +171,7 @@ def redux_store_hydration_data
   end
 
   def sanitized_props_string(props)
-    props.is_a?(String) ? json_escape(props) : props.to_json
+    props.is_a?(String) ? ERB::Util.json_escape(props) : props.to_json
   end
 
   # Helper method to take javascript expression and returns the output from evaluating it.
@@ -225,30 +222,36 @@ def server_render_js(js_expression, options = {})
 
   private
 
+  def json_safe_and_pretty(something)
+    if Rails.env.development?
+      ERB::Util.json_escape(JSON.pretty_generate(something.as_json))
+    else
+      ERB::Util.json_escape(something.to_json)
+    end
+  end
+
   # prepend the rails_context if not yet applied
   def prepend_render_rails_context(render_value)
     return render_value if @rendered_rails_context
 
-    data = {
-      rails_context: rails_context(server_side: false)
-    }
+    data = rails_context(server_side: false)
 
     @rendered_rails_context = true
 
-    rails_context_content = content_tag(:div,
-                                        "",
-                                        id: "js-react-on-rails-context",
-                                        style: ReactOnRails.configuration.skip_display_none ? nil : "display:none",
-                                        data: data)
+    rails_context_content = content_tag(:script,
+                                        json_safe_and_pretty(data).html_safe,
+                                        type: "application/json",
+                                        id: "js-react-on-rails-context")
+
     "#{rails_context_content}\n#{render_value}".html_safe
   end
 
   def render_redux_store_data(redux_store_data)
-    result = content_tag(:div,
-                         "",
-                         class: "js-react-on-rails-store",
-                         style: ReactOnRails.configuration.skip_display_none ? nil : "display:none",
-                         data: redux_store_data)
+    result = content_tag(:script,
+                         json_safe_and_pretty(redux_store_data[:props]).html_safe,
+                         type: "application/json",
+                         "data-js-react-on-rails-store" => redux_store_data[:store_name].html_safe)
+
     prepend_render_rails_context(result)
   end
 

lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt

@@ -70,9 +70,6 @@ ReactOnRails.configure do |config|
   # MISCELLANEOUS OPTIONS
   ################################################################################
 
-  # Default is false, enable if your content security policy doesn't include `style-src: 'unsafe-inline'`
-  config.skip_display_none = false
-
   # The server render method - either ExecJS or NodeJS
   <%- if options.node? -%>
   config.server_render_method = "NodeJS"

lib/react_on_rails/configuration.rb

@@ -76,7 +76,6 @@ def self.configuration
       development_mode: Rails.env.development?,
       server_renderer_pool_size: 1,
       server_renderer_timeout: 20,
-      skip_display_none: false,
       webpack_generated_files: [],
       rendering_extension: nil,
       server_render_method: "",
@@ -92,7 +91,7 @@ class Configuration
                   :trace, :development_mode,
                   :logging_on_server, :server_renderer_pool_size,
                   :server_renderer_timeout, :raise_on_prerender_error,
-                  :skip_display_none, :generated_assets_dirs, :generated_assets_dir,
+                  :generated_assets_dirs, :generated_assets_dir,
                   :webpack_generated_files, :rendering_extension, :npm_build_test_command,
                   :npm_build_production_command,
                   :i18n_dir,
@@ -102,7 +101,7 @@ def initialize(server_bundle_js_file: nil, prerender: nil, replay_console: nil,
                    trace: nil, development_mode: nil,
                    logging_on_server: nil, server_renderer_pool_size: nil,
                    server_renderer_timeout: nil, raise_on_prerender_error: nil,
-                   skip_display_none: nil, generated_assets_dirs: nil,
+                   generated_assets_dirs: nil,
                    generated_assets_dir: nil, webpack_generated_files: nil,
                    rendering_extension: nil, npm_build_test_command: nil,
                    npm_build_production_command: nil,
@@ -125,7 +124,6 @@ def initialize(server_bundle_js_file: nil, prerender: nil, replay_console: nil,
                               end
       self.trace = trace.nil? ? Rails.env.development? : trace
       self.raise_on_prerender_error = raise_on_prerender_error
-      self.skip_display_none = skip_display_none
 
       # Server rendering:
       self.server_renderer_pool_size = self.development_mode ? 1 : server_renderer_pool_size

lib/react_on_rails/react_component/options.rb

@@ -55,7 +55,6 @@ def data
       end
 
       def style
-        return nil if ReactOnRails.configuration.skip_display_none
         HIDDEN
       end
 

node_package/src/clientStartup.js

@@ -5,9 +5,6 @@ import ReactDOM from 'react-dom';
 import createReactElement from './createReactElement';
 import isRouterResult from './isCreateReactElementResultNonReactComponent';
 
-const REACT_ON_RAILS_COMPONENT_CLASS_NAME = 'js-react-on-rails-component';
-const REACT_ON_RAILS_STORE_CLASS_NAME = 'js-react-on-rails-store';
-
 function findContext() {
   if (typeof window.ReactOnRails !== 'undefined') {
     return window;
@@ -42,21 +39,28 @@ function forEach(fn, className, railsContext) {
   }
 }
 
+function forEachByAttribute(fn, attributeName, railsContext) {
+  const els = document.querySelectorAll(`[${attributeName}]`);
+  for (let i = 0; i < els.length; i += 1) {
+    fn(els[i], railsContext);
+  }
+}
+
 function forEachComponent(fn, railsContext) {
-  forEach(fn, REACT_ON_RAILS_COMPONENT_CLASS_NAME, railsContext);
+  forEach(fn, 'js-react-on-rails-component', railsContext);
 }
 
 function initializeStore(el, railsContext) {
   const context = findContext();
-  const name = el.getAttribute('data-store-name');
-  const props = JSON.parse(el.getAttribute('data-props'));
+  const name = el.attributes['data-js-react-on-rails-store'].value;
+  const props = JSON.parse(el.textContent);
   const storeGenerator = context.ReactOnRails.getStoreGenerator(name);
   const store = storeGenerator(props, railsContext);
   context.ReactOnRails.setStore(name, store);
 }
 
 function forEachStore(railsContext) {
-  forEach(initializeStore, REACT_ON_RAILS_STORE_CLASS_NAME, railsContext);
+  forEachByAttribute(initializeStore, 'data-js-react-on-rails-store', railsContext);
 }
 
 function turbolinksVersion5() {
@@ -91,10 +95,11 @@ DELEGATING TO RENDERER ${name} for dom node with id: ${domNodeId} with props, ra
  */
 function render(el, railsContext) {
   const context = findContext();
-  const name = el.getAttribute('data-component-name');
-  const domNodeId = el.getAttribute('data-dom-id');
-  const props = JSON.parse(el.getAttribute('data-props'));
-  const trace = JSON.parse(el.getAttribute('data-trace'));
+  const jsonEl = JSON.parse(el.textContent);
+  const name = jsonEl.component_name;
+  const domNodeId = jsonEl.dom_id;
+  const props = jsonEl.props;
+  const trace = jsonEl.trace;
 
   try {
     const domNode = document.getElementById(domNodeId);
@@ -130,7 +135,7 @@ You should return a React.Component always for the client side entry point.`);
 function parseRailsContext() {
   const el = document.getElementById('js-react-on-rails-context');
   if (el) {
-    return JSON.parse(el.getAttribute('data-rails-context'));
+    return JSON.parse(el.textContent);
   }
 
   return null;
@@ -145,7 +150,8 @@ export function reactOnRailsPageLoaded() {
 }
 
 function unmount(el) {
-  const domNodeId = el.getAttribute('data-dom-id');
+  const elData = JSON.parse(el.textContent);
+  const domNodeId = elData.dom_id;
   const domNode = document.getElementById(domNodeId);
   ReactDOM.unmountComponentAtNode(domNode);
 }

spec/dummy/app/controllers/pages_controller.rb

@@ -26,23 +26,24 @@ def initialize_shared_store
   end
 
   def data
+    xss_payload = { "<script>window.alert('xss1');</script>" => '<script>window.alert("xss2");</script>' }
     # This is the props used by the React component.
     @app_props_server_render = {
       helloWorldData: {
         name: "Mr. Server Side Rendering"
-      }
+      }.merge(xss_payload)
     }
 
     @app_props_hello = {
       helloWorldData: {
         name: "Mrs. Client Side Rendering"
-      }
+      }.merge(xss_payload)
     }
 
     @app_props_hello_again = {
       helloWorldData: {
         name: "Mrs. Client Side Hello Again"
-      }
+      }.merge(xss_payload)
     }
   end
 end

spec/dummy/app/views/pages/server_side_hello_world_with_options.html.erb

@@ -1,7 +1,7 @@
 <%= render "header" %>
 
 <%= react_component("HelloWorld",
-                    props: @app_props_server_render.to_json,
+                    props: @app_props_server_render,
                     prerender: true,
                     trace: true,
                     id: "my-hello-world-id",
@@ -27,7 +27,7 @@
 </ul>
 <pre>
 <%%= react_component("HelloWorld",
-                    props: @app_props_server_render.to_json,
+                    props: @app_props_server_render,
                     prerender: true,
                     trace: true,
                     id: "my-hello-world-id",

spec/dummy/config/initializers/react_on_rails.rb

@@ -81,8 +81,6 @@ def self.custom_context(view_context)
   ################################################################################
   # MISCELLANEOUS OPTIONS
   ################################################################################
-  # Default is false, enable if your content security policy doesn't include `style-src: 'unsafe-inline'`
-  config.skip_display_none = false
 
   # This allows you to add additional values to the Rails Context. Implement one static method
   # called `custom_context(view_context)` and return a Hash.

spec/dummy/spec/helpers/react_on_rails_helper_spec.rb

@@ -59,76 +59,44 @@
 
     let(:id) { "App-react-component-0" }
 
-    let(:react_definition_div) do
-      %(<div class="js-react-on-rails-component"
-            style="display:none"
-            data-component-name="App"
-            data-props="{&quot;name&quot;:&quot;My Test Name&quot;}"
-            data-trace="false"
-            data-dom-id="#{id}"></div>).squish
+    # rubocop:disable Metrics/LineLength
+    let(:react_definition_script) do
+      %(<script type="application/json" class="js-react-on-rails-component">{"component_name":"App","props":{"name":"My Test Name"},"trace":false,"dom_id":"App-react-component-0"}</script>).squish
     end
 
-    let(:react_definition_div_no_params) do
-      %(<div class="js-react-on-rails-component"
-            style="display:none"
-            data-component-name="App"
-            data-props="{}"
-            data-trace="false"
-            data-dom-id="#{id}"></div>).squish
+    let(:react_definition_script_no_params) do
+      %(<script type="application/json" class="js-react-on-rails-component">{"component_name":"App","props":{},"trace":false,"dom_id":"App-react-component-0"}</script>).squish
     end
+    # rubocop:enable Metrics/LineLength
 
     describe "API with component name only" do
       subject { react_component("App") }
       it { is_expected.to be_an_instance_of ActiveSupport::SafeBuffer }
       it { is_expected.to include react_component_div }
-      it { is_expected.to include react_definition_div_no_params }
+      it { is_expected.to include react_definition_script_no_params }
     end
 
     it { expect(self).to respond_to :react_component }
 
     it { is_expected.to be_an_instance_of ActiveSupport::SafeBuffer }
-    it { is_expected.to start_with "<div" }
-    it { is_expected.to match %r{</div>\s*$} }
+    it { is_expected.to start_with "<script" }
+    it { is_expected.to match %r{</script>\s*$} }
     it { is_expected.to include react_component_div }
-    it { is_expected.to include react_definition_div }
+    it { is_expected.to include react_definition_script }
 
     context "with 'id' option" do
       subject { react_component("App", props: props, id: id) }
 
       let(:id) { "shaka_div" }
 
-      it { is_expected.to include id }
-      it { is_expected.not_to include react_component_div }
-      it { is_expected.to include react_definition_div }
-    end
-
-    context "with skip_display_none option true" do
-      before { ReactOnRails.configuration.skip_display_none = true }
-
-      let(:react_definition_div_skip_display_none_true) do
-        "<div class=\"js-react-on-rails-component\"
-              data-component-name=\"App\"
-              data-props=\"{&quot;name&quot;:&quot;My Test Name&quot;}\"
-              data-trace=\"false\"
-              data-dom-id=\"#{id}\"></div>".squish
-      end
-
-      it { is_expected.to include react_definition_div_skip_display_none_true }
-    end
-
-    context "with skip_display_none option false" do
-      before { ReactOnRails.configuration.skip_display_none = false }
-
-      let(:react_definition_div_skip_display_none_false) do
-        "<div class=\"js-react-on-rails-component\"
-              style=\"display:none\"
-              data-component-name=\"App\"
-              data-props=\"{&quot;name&quot;:&quot;My Test Name&quot;}\"
-              data-trace=\"false\"
-              data-dom-id=\"#{id}\"></div>".squish
+      let(:react_definition_script) do
+        # rubocop:disable Metrics/LineLength
+        %(<script type="application/json" class="js-react-on-rails-component">{"component_name":"App","props":{"name":"My Test Name"},"trace":false,"dom_id":"shaka_div"}</script>).squish
       end
 
-      it { is_expected.to include react_definition_div_skip_display_none_false }
+      it { is_expected.to include id }
+      it { is_expected.not_to include react_component_div }
+      it { is_expected.to include react_definition_script }
     end
   end
 
@@ -139,36 +107,16 @@
       { name: "My Test Name" }
     end
 
-    let(:react_store_div) do
-      %(<div class="js-react-on-rails-store"
-            style="display:none"
-            data-store-name="reduxStore"
-            data-props="{&quot;name&quot;:&quot;My Test Name&quot;}"></div>).squish
+    let(:react_store_script) do
+      %(<script type="application/json" data-js-react-on-rails-store="reduxStore">{"name":"My Test Name"}</script>).squish
     end
 
     it { expect(self).to respond_to :redux_store }
 
     it { is_expected.to be_an_instance_of ActiveSupport::SafeBuffer }
-    it { is_expected.to start_with "<div" }
-    it { is_expected.to end_with "</div>" }
-    it { is_expected.to include react_store_div }
-
-    context "with skip_display_none option true" do
-      before { ReactOnRails.configuration.skip_display_none = true }
-
-      let(:react_store_definition_div_skip_display_none_true) do
-        %(<div class="js-react-on-rails-store"
-            data-store-name="reduxStore"
-            data-props="{&quot;name&quot;:&quot;My Test Name&quot;}"></div>).squish
-      end
-
-      it { is_expected.to include react_store_definition_div_skip_display_none_true }
-    end
-
-    context "with skip_display_none option false" do
-      before { ReactOnRails.configuration.skip_display_none = false }
-      it { is_expected.to include react_store_div }
-    end
+    it { is_expected.to start_with "<script" }
+    it { is_expected.to end_with "</script>" }
+    it { is_expected.to include react_store_script }
   end
 
   describe "#server_render_js" do