shakacode · cheremukhin23 · Mar 31, 2017 · Mar 31, 2017 · Apr 1, 2017 · Apr 1, 2017
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@ All notable changes to this project's source code will be documented in this fil
 Contributors: please follow the recommendations outlined at [keepachangelog.com](http://keepachangelog.com/). Please use the existing headings and styling as a guide, and add a link for the version diff at the bottom of the file. Also, please update the `Unreleased` link to compare to the latest release version.
 
 ## [Unreleased]
+
+### Improved
+- Improve json conversion with tests and support for older Rails 3.x. [#787](https://github.com/shakacode/react_on_rails/pull/787) by [cheremukhin23](https://github.com/cheremukhin23).
+
 ### Added
 - Add an ability to return multiple HTML strings in a `Hash` as a result of `react_component` method call. Allows to build `<head>` contents with [React Helmet](https://github.com/nfl/react-helmet).
 [#800](https://github.com/shakacode/react_on_rails/pull/800) by [udovenko](https://github.com/udovenko).

diff --git a/app/helpers/react_on_rails_helper.rb b/app/helpers/react_on_rails_helper.rb
@@ -224,8 +224,33 @@ def server_render_js(js_expression, options = {})
     # rubocop:enable Style/RaiseArgs
   end
 
+  def json_safe_and_pretty(hash_or_string)
+    unless hash_or_string.class.in?([Hash, String])
+      raise "#{hash_or_string.class} is unsupported argument class for this method"
+    end
+    json_value = hash_or_string.is_a?(String) ? hash_or_string : hash_or_string.to_json
+    escape_json(json_value)
+  end
+
   private
 
+  def escape_json(json)
+    return old_json_escape(json) if rails_version_less_than("4")
+    ERB::Util.json_escape(json)
+  end
+
+  def rails_version_less_than(version)
+    Gem::Version.new(Rails.version) <= Gem::Version.new(version)
+  end
+
+  def old_json_escape(json)
+    # https://github.com/rails/rails/blob/60257141462137331387d0e34931555cf0720886/activesupport/lib/active_support/core_ext/string/output_safety.rb#L113
+
+    json_escape = { "&" => '\u0026', ">" => '\u003e', "<" => '\u003c', "\u2028" => '\u2028', "\u2029" => '\u2029' }
+    json_escape_regexp = /[\u2028\u2029&><]/u
+    json.to_s.gsub(json_escape_regexp, json_escape)
+  end
+
   def build_react_component_result_for_server_rendered_string(
     server_rendered_html: required("server_rendered_html"),
     component_specification_tag: required("component_specification_tag"),
@@ -290,26 +315,6 @@ def compose_react_component_html_with_spec_and_console(component_specification_t
     HTML
   end
 
-  def json_safe_and_pretty(hash_or_string)
-    # if Rails.env.development?
-    #   # TODO: for json_safe_and_pretty
-    #   # 1. Add test
-    #   # 2. Add error handler if cannot parse the string with nice message
-    #   # 3. Consider checking that if not a string then a Hash
-    #   hash_value = hash_or_string.is_a?(String) ? JSON.parse(hash_or_string) : hash_or_string
-    #   ERB::Util.json_escape(JSON.pretty_generate(hash_value))
-    # else
-    #
-    # Temp fix given that a hash may contain active record objects and that crashed with the new
-    # code to JSON.pretty_generate
-
-    # If to_json is called on a String, then the quotes are escaped.
-    json_value = hash_or_string.is_a?(String) ? hash_or_string : hash_or_string.to_json
-
-    ERB::Util.json_escape(json_value)
-    # end
-  end
-
   # prepend the rails_context if not yet applied
   def prepend_render_rails_context(render_value)
     return render_value if @rendered_rails_context

diff --git a/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb b/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb
@@ -10,24 +10,40 @@
     }
   end
 
-  describe "#sanitized_props_string(props)" do
-    let(:hash) do
-      {
-        hello: "world",
-        free: "of charge",
-        x: "</script><script>alert('foo')</script>"
-      }
-    end
+  let(:hash) do
+    {
+      hello: "world",
+      free: "of charge",
+      x: "</script><script>alert('foo')</script>"
+    }
+  end
 
-    let(:hash_sanitized) do
-      '{"hello":"world","free":"of charge","x":"\\u003c/script\\u003e\\u003cscrip'\
+  let(:hash_sanitized) do
+    '{"hello":"world","free":"of charge","x":"\\u003c/script\\u003e\\u003cscrip'\
       "t\\u003ealert('foo')\\u003c/script\\u003e\"}"
+  end
+
+  let(:hash_unsanitized) do
+    "{\"hello\":\"world\",\"free\":\"of charge\",\"x\":\"</script><script>alert('foo')</script>\"}"
+  end
+
+  describe "#json_safe_and_pretty(hash_or_string)" do
+    it "should raise an error if not hash nor string passed" do
+      expect { helper.json_safe_and_pretty(false) }.to raise_error
+    end
+
+    it "converts a hash to escaped JSON" do
+      escaped_json = helper.json_safe_and_pretty(hash)
+      expect(escaped_json).to eq(hash_sanitized)
     end
 
-    let(:hash_unsanitized) do
-      "{\"hello\":\"world\",\"free\":\"of charge\",\"x\":\"</script><script>alert('foo')</script>\"}"
+    it "converts a string to escaped JSON" do
+      escaped_json = helper.json_safe_and_pretty(hash_unsanitized)
+      expect(escaped_json).to eq(hash_sanitized)
     end
+  end
 
+  describe "#sanitized_props_string(props)" do
     it "converts a hash to JSON and escapes </script>" do
       sanitized = helper.sanitized_props_string(hash)
       expect(sanitized).to eq(hash_sanitized)