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

Use <script type="application/json"> for props and store #775

Merged
merged 22 commits into from
Mar 30, 2017
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
0c3409e
Fix webpack.config.js for Procfile.hot and Webpack 2
squadette Mar 11, 2017
bc3f197
Refactor console output to use <script> tag with a certain ID
squadette Mar 11, 2017
f835c5f
Use <script type="application/json"> with JSON content instead of att…
squadette Mar 11, 2017
8492f0e
Pretty-print JSON in development and testing
squadette Mar 16, 2017
12f88a5
getAttribute() is better, because it handles attributes which are not…
squadette Mar 16, 2017
807e140
Revert "Pretty-print JSON in development and testing"
squadette Mar 16, 2017
cdd8722
Return skip display none to react on rails configuration
cheremukhin23 Mar 27, 2017
a5a414a
Extract data-js-react-on-rails-store attribute to the const
cheremukhin23 Mar 27, 2017
27bfe25
Return to_json method call to app_props_server_render to ensure handl…
cheremukhin23 Mar 27, 2017
2486892
Merge branch 'master' of https://github.com/shakacode/react_on_rails …
cheremukhin23 Mar 27, 2017
c64db94
Change const name
cheremukhin23 Mar 27, 2017
d4947b7
Remove rubocop exclusions to line length
cheremukhin23 Mar 28, 2017
9a76dc2
Delete unnecessary style method and HIDDEN constant from react compon…
cheremukhin23 Mar 28, 2017
0975f04
Format strings to pass the tests
cheremukhin23 Mar 28, 2017
cb3c9d0
Improve jsonEl parsing to handle props as already converted from JSON…
cheremukhin23 Mar 28, 2017
949f501
Update code to pass linter
cheremukhin23 Mar 28, 2017
09b03e5
Fix rubocop offenses
cheremukhin23 Mar 28, 2017
25bb18c
Move handling props already converted to json string to ruby code
cheremukhin23 Mar 29, 2017
857f7d8
Escape json from props as json string
cheremukhin23 Mar 29, 2017
202f3c0
Add test to ensure that even json string props are sanitezed
cheremukhin23 Mar 29, 2017
d186aa1
Delete test json props because we parse all props to a hash now
cheremukhin23 Mar 29, 2017
82adca1
Update CHANGELOG.md
cheremukhin23 Mar 30, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ Contributors: please follow the recommendations outlined at [keepachangelog.com]
## [Unreleased]
*Please add entries here for your pull requests.*

## Improved
- Use <script type="application/json"> for props and store instead of hidden div [#775] (https://github.com/shakacode/react_on_rails/pull/775) by [cheremukhin23](https://github.com/cheremukhin23).

## [6.8.2] - 2017-03-24
## Fixed
- Change webpack output path to absolute and update webpack to version ^2.3.1. [#771](https://github.com/shakacode/react_on_rails/pull/771) by [cheremukhin23](https://github.com/cheremukhin23).
Expand Down
45 changes: 24 additions & 21 deletions app/helpers/react_on_rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 11 additions & 3 deletions lib/react_on_rails/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ def self.configure
def self.setup_config_values
ensure_webpack_generated_files_exists
configure_generated_assets_dirs_deprecation
configure_skip_display_none_deprecation
ensure_generated_assets_dir_present
ensure_server_bundle_js_file_has_no_path
check_i18n_directory_exists
Expand Down Expand Up @@ -61,6 +62,11 @@ def self.ensure_server_bundle_js_file_has_no_path
@configuration.server_bundle_js_file = File.basename(@configuration.server_bundle_js_file)
end

def self.configure_skip_display_none_deprecation
return if @configuration.skip_display_none.nil?
puts "[DEPRECATION] ReactOnRails: remove skip_display_none from configuration."
end

def self.configuration
@configuration ||= Configuration.new(
generated_assets_dirs: nil,
Expand All @@ -76,7 +82,9 @@ def self.configuration
development_mode: Rails.env.development?,
server_renderer_pool_size: 1,
server_renderer_timeout: 20,
skip_display_none: false,
skip_display_none: nil,

# skip_display_none is deprecated
webpack_generated_files: [],
rendering_extension: nil,
server_render_method: "",
Expand All @@ -91,8 +99,8 @@ class Configuration
attr_accessor :server_bundle_js_file, :prerender, :replay_console,
: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,
:server_renderer_timeout, :skip_display_none, :raise_on_prerender_error,
:generated_assets_dirs, :generated_assets_dir,
:webpack_generated_files, :rendering_extension, :npm_build_test_command,
:npm_build_production_command,
:i18n_dir,
Expand Down
9 changes: 2 additions & 7 deletions lib/react_on_rails/react_component/options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ class Options
include Utils::Required

NO_PROPS = {}.freeze
HIDDEN = "display:none".freeze

def initialize(name: required("name"), options: required("options"))
@name = name
@options = options
end

def props
options.fetch(:props) { NO_PROPS }
props = options.fetch(:props) { NO_PROPS }
props.is_a?(String) ? JSON.parse(ERB::Util.json_escape(props)) : props
end

def name
Expand Down Expand Up @@ -54,11 +54,6 @@ def data
}
end

def style
return nil if ReactOnRails.configuration.skip_display_none
HIDDEN
end

private

attr_reader :options
Expand Down
4 changes: 2 additions & 2 deletions node_package/src/RenderUtils.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
export default {
wrapInScriptTags(scriptBody) {
wrapInScriptTags(scriptId, scriptBody) {
if (!scriptBody) {
return '';
}

return `\n<script>
return `\n<script id="${scriptId}">
${scriptBody}
</script>`;
},
Expand Down
2 changes: 1 addition & 1 deletion node_package/src/buildConsoleReplay.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ export function consoleReplay() {
}

export default function buildConsoleReplay() {
return RenderUtils.wrapInScriptTags(consoleReplay());
return RenderUtils.wrapInScriptTags('consoleReplayLog', consoleReplay());
}
32 changes: 20 additions & 12 deletions node_package/src/clientStartup.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ 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';
const REACT_ON_RAILS_STORE_ATTRIBUTE = 'data-js-react-on-rails-store';

function findContext() {
if (typeof window.ReactOnRails !== 'undefined') {
Expand Down Expand Up @@ -42,21 +41,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.getAttribute(REACT_ON_RAILS_STORE_ATTRIBUTE);
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, REACT_ON_RAILS_STORE_ATTRIBUTE, railsContext);
}

function turbolinksVersion5() {
Expand Down Expand Up @@ -91,10 +97,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);
Expand Down Expand Up @@ -130,7 +137,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;
Expand All @@ -145,7 +152,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);
}
Expand Down
2 changes: 1 addition & 1 deletion node_package/tests/buildConsoleReplay.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ test('buildConsoleReplay wraps console replay in a script tag', (assert) => {
const actual = buildConsoleReplay();

const expected = `
<script>
<script id="consoleReplayLog">
console.log.apply(console, ["some message","{\\"a\\":1,\\"b\\":2}"]);
console.warn.apply(console, ["other message","{\\"c\\":3,\\"d\\":4}"]);
</script>`;
Expand Down
7 changes: 4 additions & 3 deletions spec/dummy/app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 0 additions & 2 deletions spec/dummy/config/initializers/react_on_rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading