diff --git a/CHANGELOG.md b/CHANGELOG.md index 4652ddf06..827c556a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,11 @@ Contributors: please follow the recommendations outlined at [keepachangelog.com] ## [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) and [Ynote](https://github.com/Ynote). + ## [6.10.0] - 2017-04-13 + ### Added - Add an ability to return multiple HTML strings in a `Hash` as a result of `react_component` method call. Allows to build `` 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 index 3cf7bea03..469344b15 100644 --- a/app/helpers/react_on_rails_helper.rb +++ b/app/helpers/react_on_rails_helper.rb @@ -6,6 +6,7 @@ require "react_on_rails/prerender_error" require "addressable/uri" require "react_on_rails/utils" +require "react_on_rails/json_output" module ReactOnRailsHelper include ReactOnRails::Utils::Required @@ -224,6 +225,17 @@ 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 "#{__method__} only accepts String or Hash as argument "\ + "(#{hash_or_string.class} given)." + end + + json_value = hash_or_string.is_a?(String) ? hash_or_string : hash_or_string.to_json + + ReactOnRails::JsonOutput.escape(json_value) + end + private def build_react_component_result_for_server_rendered_string( @@ -290,26 +302,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/lib/react_on_rails/json_output.rb b/lib/react_on_rails/json_output.rb new file mode 100644 index 000000000..6152f9822 --- /dev/null +++ b/lib/react_on_rails/json_output.rb @@ -0,0 +1,26 @@ +require "active_support/core_ext/string/output_safety" + +module ReactOnRails + class JsonOutput + ESCAPE_REPLACEMENT = { + "&" => '\u0026', + ">" => '\u003e', + "<" => '\u003c', + "\u2028" => '\u2028', + "\u2029" => '\u2029' + }.freeze + ESCAPE_REGEXP = /[\u2028\u2029&><]/u + + def self.escape(json) + return escape_without_erb_util(json) if Utils.rails_version_less_than_4_1_1 + + ERB::Util.json_escape(json) + end + + def self.escape_without_erb_util(json) + # https://github.com/rails/rails/blob/60257141462137331387d0e34931555cf0720886/activesupport/lib/active_support/core_ext/string/output_safety.rb#L113 + + json.to_s.gsub(ESCAPE_REGEXP, ESCAPE_REPLACEMENT) + end + end +end diff --git a/lib/react_on_rails/utils.rb b/lib/react_on_rails/utils.rb index 599c4e3fe..0ebc3c8d1 100644 --- a/lib/react_on_rails/utils.rb +++ b/lib/react_on_rails/utils.rb @@ -24,6 +24,22 @@ def self.running_on_windows? (/cygwin|mswin|mingw|bccwin|wince|emx/ =~ RUBY_PLATFORM) != nil end + def self.rails_version_less_than(version) + @rails_version_less_than ||= {} + + if @rails_version_less_than.key?(version) + return @rails_version_less_than[version] + end + + @rails_version_less_than[version] = begin + Gem::Version.new(Rails.version) < Gem::Version.new(version) + end + end + + def self.rails_version_less_than_4_1_1 + rails_version_less_than("4.1.1") + end + module Required def required(arg_name) raise ArgumentError, "#{arg_name} is required" diff --git a/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb b/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb index f28d86581..de94c8034 100644 --- 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: "" - } - end + let(:hash) do + { + hello: "world", + free: "of charge", + x: "" + } + 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\":\"\"}" + 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\":\"\"}" + 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 " do sanitized = helper.sanitized_props_string(hash) expect(sanitized).to eq(hash_sanitized) diff --git a/spec/react_on_rails/json_output_spec.rb b/spec/react_on_rails/json_output_spec.rb new file mode 100644 index 000000000..51897e302 --- /dev/null +++ b/spec/react_on_rails/json_output_spec.rb @@ -0,0 +1,45 @@ +require_relative "spec_helper" +require "react_on_rails/json_output" + +module ReactOnRails + describe JsonOutput do + let(:hash_value) do + { + simple: "hello world", + special: '<>&\u2028\u2029' + } + end + + let(:escaped_json) do + '{"simple":"hello world","special":"\\u003c\\u003e\\u0026\\\\u2028\\\\u2029"}' + end + + shared_examples :escaped_json do + it "returns a well-formatted json with escaped characters" do + expect(subject).to eq(escaped_json) + end + end + + describe ".escape" do + subject { described_class.escape(hash_value.to_json) } + + context "with Rails version 4.1.1 and higher" do + before { allow(Rails).to receive(:version).and_return("4.1.1") } + + it_behaves_like :escaped_json + end + + context "with Rails version lower than 4.1.1" do + before { allow(Rails).to receive(:version).and_return("4.1.0") } + + it_behaves_like :escaped_json + end + end + + describe ".escaped_without_erb_utils" do + subject { described_class.escape_without_erb_util(hash_value.to_json) } + + it_behaves_like :escaped_json + end + end +end diff --git a/spec/react_on_rails/utils_spec.rb b/spec/react_on_rails/utils_spec.rb new file mode 100644 index 000000000..7cc4a5743 --- /dev/null +++ b/spec/react_on_rails/utils_spec.rb @@ -0,0 +1,71 @@ +require_relative "spec_helper" + +module ReactOnRails + RSpec.describe Utils do + subject { Utils.rails_version_less_than("4") } + + describe ".rails_version_less_than" do + before(:each) { Utils.instance_variable_set :@rails_version_less_than, nil } + + context "with Rails 3" do + before { allow(Rails).to receive(:version).and_return("3") } + + it { expect(subject).to eq(true) } + end + + context "with Rails 3.2" do + before { allow(Rails).to receive(:version).and_return("3.2") } + + it { expect(subject).to eq(true) } + end + + context "with Rails 4" do + before { allow(Rails).to receive(:version).and_return("4") } + + it { expect(subject).to eq(false) } + end + + context "with Rails 4.2" do + before { allow(Rails).to receive(:version).and_return("4.2") } + + it { expect(subject).to eq(false) } + end + + context "with Rails 10.0" do + before { allow(Rails).to receive(:version).and_return("10.0") } + + it { expect(subject).to eq(false) } + end + + context "called twice" do + before do + allow(Rails).to receive(:version).and_return("4.2") + end + + it "should memoize the result" do + 2.times { Utils.rails_version_less_than("4") } + + expect(Rails).to have_received(:version).once + end + end + end + + describe ".rails_version_less_than_4_1_1" do + subject { Utils.rails_version_less_than_4_1_1 } + + before(:each) { Utils.instance_variable_set :@rails_version_less_than, nil } + + context "with Rails 4.1.0" do + before { allow(Rails).to receive(:version).and_return("4.1.0") } + + it { expect(subject).to eq(true) } + end + + context "with Rails 4.1.1" do + before { allow(Rails).to receive(:version).and_return("4.1.1") } + + it { expect(subject).to eq(false) } + end + end + end +end