Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Extends #787] - Improve json conversion with tests and support for Rails older than 4.1 #812

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
703e1d7
Improve json_safe_and_pretty method to handle parse error
cheremukhin23 Mar 31, 2017
5867246
Add tests to json_safe_and_pretty method
cheremukhin23 Mar 31, 2017
68044df
Add old json escape method for Rails versions less than 4
cheremukhin23 Apr 1, 2017
a3ef532
Fix rubocop offenses
cheremukhin23 Apr 1, 2017
fb697cd
Merge branch 'master' of https://github.com/shakacode/react_on_rails …
cheremukhin23 Apr 11, 2017
80d3dbc
Fixed rubocop offense
cheremukhin23 Apr 11, 2017
358d572
Add json safe and pretty argument class test
cheremukhin23 Apr 11, 2017
5ad43e8
Add rails version less than method
cheremukhin23 Apr 12, 2017
78a1c3c
Merge branch 'master' of https://github.com/shakacode/react_on_rails …
cheremukhin23 Apr 13, 2017
19722db
Update CHANGELOG.md
cheremukhin23 Apr 13, 2017
c521f15
Add tests for json encoding helper methods
Ynote Apr 19, 2017
d190765
Move rails_version_less_than method into Utils module
Ynote Apr 20, 2017
633a7a6
Extract json escaping logic into JsonOutput class
Ynote Apr 20, 2017
f78d48f
Merge branch 'master' of https://github.com/shakacode/react_on_rails …
Ynote Apr 20, 2017
e7d0597
Update CHANGELOG
Ynote Apr 20, 2017
21d502a
Fix error message output
Ynote Apr 20, 2017
042f9c7
Remove "or equal" logic from rails_version_less_than util method
Ynote Apr 22, 2017
01c3926
Use monkey patch json escape from Rails 4.2 instead of Rails 4
Ynote Apr 22, 2017
460eb37
Fix Rubocop offenses
Ynote Apr 22, 2017
a168047
Memoize result for static method rails_version_less_than
Ynote Apr 22, 2017
06dd57d
Update json escaping method into class methods
Ynote Apr 23, 2017
f5feb91
Fix rails_version_less_than helper method
Ynote Apr 23, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<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).

Expand Down
32 changes: 12 additions & 20 deletions app/helpers/react_on_rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions lib/react_on_rails/json_output.rb
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions lib/react_on_rails/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
40 changes: 28 additions & 12 deletions spec/dummy/spec/helpers/react_on_rails_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
45 changes: 45 additions & 0 deletions spec/react_on_rails/json_output_spec.rb
Original file line number Diff line number Diff line change
@@ -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
71 changes: 71 additions & 0 deletions spec/react_on_rails/utils_spec.rb
Original file line number Diff line number Diff line change
@@ -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