From 9539776dff3e0e5b832c82b29ebdb9bb049d278a Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 4 Apr 2016 21:46:16 -1000 Subject: [PATCH] Completes prior fix 366 for script sanitization * Some cases missed, per: http://stackoverflow.com/a/23983448/1009332 * Added serverRendering and port to railsContext * Only logging attributes client side if tracing to avoid overwhelming server logs. On the client, one can optionally toggle expand the props. On the server, we stringified, and with a big set of props, that could be huge. --- .rubocop.yml | 2 +- CHANGELOG.md | 9 +++- README.md | 10 ++++- app/helpers/react_on_rails_helper.rb | 11 +++-- node_package/src/createReactElement.js | 9 +++- node_package/src/scriptSanitizedVal.js | 4 +- node_package/tests/buildConsoleReplay.test.js | 4 +- node_package/tests/scriptSanitizedVal.test.js | 42 +++++++++++++++++-- .../client/app/components/HelloWorldRedux.jsx | 6 ++- .../client/app/components/RailsContext.jsx | 21 ++++++---- .../dummy/spec/features/rails_context_spec.rb | 1 + .../spec/requests/console_logging_spec.rb | 2 +- 12 files changed, 93 insertions(+), 28 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 110c35258..6cdb80ef4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -48,7 +48,7 @@ Lint/HandleExceptions: - 'spec/dummy/bin/rake' Metrics/AbcSize: - Max: 26 + Max: 28 Metrics/CyclomaticComplexity: Max: 7 diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aa616af4..95b4249b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. Items under 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] +## [5.1.1] - 2016-04-04 +##### Fixed +- [Security] Address failure to sanitize console messages when server rendering and displaying in the browser console. See [#366](https://github.com/shakacode/react_on_rails/pull/366) and [#370](https://github.com/shakacode/react_on_rails/pull/370) by [justin808](https://github.com/justin808) +##### Added +- railsContext includes the port number and a boolean if the code is being run on the server or client. + ## [5.1.0] - 2016-04-03 ##### Added All 5.1.0 changes can be found in [#362](https://github.com/shakacode/react_on_rails/pull/362) by [justin808](https://github.com/justin808). @@ -262,7 +268,8 @@ Best done with Object destructing: ##### Fixed - Fix several generator related issues. -[Unreleased]: https://github.com/shakacode/react_on_rails/compare/5.1.0...master +[Unreleased]: https://github.com/shakacode/react_on_rails/compare/5.1.1...master +[5.1.1]: https://github.com/shakacode/react_on_rails/compare/5.0.0...5.1.1 [5.1.0]: https://github.com/shakacode/react_on_rails/compare/5.0.0...5.1.0 [5.0.0]: https://github.com/shakacode/react_on_rails/compare/4.0.3...5.0.0 [4.0.3]: https://github.com/shakacode/react_on_rails/compare/4.0.2...4.0.3 diff --git a/README.md b/README.md index 0663689e7..7b7359e54 100644 --- a/README.md +++ b/README.md @@ -234,13 +234,19 @@ The `railsContext` has: (see implementation in file react_on_rails_helper.rb for location: "#{uri.path}#{uri.query.present? ? "?#{uri.query}": ""}", scheme: uri.scheme, # http host: uri.host, # foo.com + port: uri.port, pathname: uri.path, # /posts search: uri.query, # id=30&limit=5 # Locale settings i18nLocale: I18n.locale, i18nDefaultLocale: I18n.default_locale, - httpAcceptLanguage: request.env["HTTP_ACCEPT_LANGUAGE"] + httpAcceptLanguage: request.env["HTTP_ACCEPT_LANGUAGE"], + + # Other + serverSide: boolean # Are we being called on the server or client? NOTE, if you conditionally + # render something different on the server than the client, then React will only show the + # server version! } ``` @@ -327,7 +333,7 @@ This is how you actually render the React components you exposed to `window` ins + **prerender:** enable server-side rendering of component. Set to false when debugging! + **id:** Id for the div. This will get assigned automatically if you do not provide an id. + **html_options:** Any other html options to get placed on the added div for the component. - + **trace:** set to true to print additional debugging information in the browser. Defaults to true for development, off otherwise. + + **trace:** set to true to print additional debugging information in the browser. Defaults to true for development, off otherwise. Note, on the client you will so both the railsContext and your props. On the server, you only see the railsContext being logged. + **replay_console:** Default is true. False will disable echoing server-rendering logs to the browser. While this can make troubleshooting server rendering difficult, so long as you have the default configuration of logging_on_server set to true, you'll still see the errors on the server. + **raise_on_prerender_error:** Default is false. True will throw an error on the server side rendering. Your controller will have to handle the error. diff --git a/app/helpers/react_on_rails_helper.rb b/app/helpers/react_on_rails_helper.rb index 65c08267e..2ea881a63 100644 --- a/app/helpers/react_on_rails_helper.rb +++ b/app/helpers/react_on_rails_helper.rb @@ -250,7 +250,7 @@ def prepend_render_rails_context(render_value) return render_value if @rendered_rails_context data = { - rails_context: rails_context + rails_context: rails_context(server_side: false) } @rendered_rails_context = true @@ -297,7 +297,7 @@ def server_rendered_react_component_html(props, react_component_name, dom_id, wrapper_js = <<-JS (function() { - var railsContext = #{rails_context.to_json}; + var railsContext = #{rails_context(server_side: true).to_json}; #{initialize_redux_stores} var props = #{props_string(props)}; return ReactOnRails.serverRenderReactComponent({ @@ -356,10 +356,10 @@ def initialize_redux_stores # This is the definitive list of the default values used for the rails_context, which is the # second parameter passed to both component and store generator functions. - def rails_context + def rails_context(server_side:) @rails_context ||= begin uri = URI.parse(request.original_url) - # uri = URI("http://foo.com/posts?id=30&limit=5#time=1305298413") + # uri = URI("http://foo.com:3000/posts?id=30&limit=5#time=1305298413") result = { # URL settings @@ -367,6 +367,7 @@ def rails_context location: "#{uri.path}#{uri.query.present? ? "?#{uri.query}" : ''}", scheme: uri.scheme, # http host: uri.host, # foo.com + port: uri.port, pathname: uri.path, # /posts search: uri.query, # id=30&limit=5 @@ -382,6 +383,8 @@ def rails_context end result end + + @rails_context.merge(serverSide: server_side) end def raise_on_prerender_error_option(val) diff --git a/node_package/src/createReactElement.js b/node_package/src/createReactElement.js index 4f9f33457..9d4374092 100644 --- a/node_package/src/createReactElement.js +++ b/node_package/src/createReactElement.js @@ -21,8 +21,13 @@ export default function createReactElement({ location, }) { if (trace) { - console.log(`RENDERED ${name} to dom node with id: ${domNodeId} with props, railsContext:`, - props, railsContext); + if (railsContext && railsContext.serverSide) { + console.log(`RENDERED ${name} to dom node with id: ${domNodeId} with railsContext:`, + railsContext); + } else { + console.log(`RENDERED ${name} to dom node with id: ${domNodeId} with props, railsContext:`, + props, railsContext); + } } const componentObj = ReactOnRails.getComponent(name); diff --git a/node_package/src/scriptSanitizedVal.js b/node_package/src/scriptSanitizedVal.js index 5221b3dc0..85f0cfdc6 100644 --- a/node_package/src/scriptSanitizedVal.js +++ b/node_package/src/scriptSanitizedVal.js @@ -1,5 +1,5 @@ export default (val) => { // Replace closing - const re = /<\/\W*script\W*>/gi; - return val.replace(re, '(/script)'); + const re = /<\/\W*script/gi; + return val.replace(re, '(/script'); }; diff --git a/node_package/tests/buildConsoleReplay.test.js b/node_package/tests/buildConsoleReplay.test.js index 9831b34b7..cf3639a48 100644 --- a/node_package/tests/buildConsoleReplay.test.js +++ b/node_package/tests/buildConsoleReplay.test.js @@ -66,8 +66,8 @@ test('consoleReplay replays converts script tag inside of object string to be sa ]; const actual = consoleReplay(); - const expected = `console.log.apply(console, ["some message (/script)', (assert) => { +test('scriptSanitizedVal returns no { assert.plan(1); const input = '[SERVER] This is a script:\"\" 2', (assert) => { + assert.plan(1); + const input = 'Script2:"" '; + const actual = scriptSanitizedVal(input); + const expected = 'Script2:""(/script xx> 3', (assert) => { + assert.plan(1); + const input = 'Script3:"" '; + const actual = scriptSanitizedVal(input); + const expected = 'Script3:""(/script xx> 4', (assert) => { + assert.plan(1); + const input = 'Script4""alert(\'WTF4\')'; + const actual = scriptSanitizedVal(input); + const expected = 'Script4""(/script 5', (assert) => { + assert.plan(1); + const input = 'Script5:"" '; + const actual = scriptSanitizedVal(input); + const expected = 'Script5:""(/script> '); + console.log('This is a script:"" '); + console.log('Script2:"" '); + console.log('Script3:"" '); + console.log('Script4""alert(\'WTF4\')'); + console.log('Script5:"" '); return (
diff --git a/spec/dummy/client/app/components/RailsContext.jsx b/spec/dummy/client/app/components/RailsContext.jsx index 5a5222600..46351f80a 100644 --- a/spec/dummy/client/app/components/RailsContext.jsx +++ b/spec/dummy/client/app/components/RailsContext.jsx @@ -2,16 +2,19 @@ import React, { PropTypes } from 'react'; import _ from 'lodash'; function renderContextRows(railsContext) { + console.log('railsContext.serverSide is ', railsContext.serverSide) return _.transform(railsContext, (accum, value, key) => { - const className = `js-${key}`; - accum.push( - - - {key}:  - - {value} - - ); + if (key !== 'serverSide') { + const className = `js-${key}`; + accum.push( + + + {key}:  + + {value + ''} + + ); + } }, []); } diff --git a/spec/dummy/spec/features/rails_context_spec.rb b/spec/dummy/spec/features/rails_context_spec.rb index e5d5f7e7e..c2d4ab1e1 100644 --- a/spec/dummy/spec/features/rails_context_spec.rb +++ b/spec/dummy/spec/features/rails_context_spec.rb @@ -22,6 +22,7 @@ keys_to_vals = { href: "http://#{host_port}/#{pathname}?ab=cd", location: "/#{pathname}?ab=cd", + port: port, scheme: "http", host: host, pathname: "/#{pathname}", diff --git a/spec/dummy/spec/requests/console_logging_spec.rb b/spec/dummy/spec/requests/console_logging_spec.rb index 665c5d562..81d7232fd 100644 --- a/spec/dummy/spec/requests/console_logging_spec.rb +++ b/spec/dummy/spec/requests/console_logging_spec.rb @@ -7,7 +7,7 @@ expected = <<-JS console.log.apply(console, ["[SERVER] RENDERED HelloWorldWithLogAndThrow to dom node \ -with id: HelloWorldWithLogAndThrow-react-component-0 with props, railsContext:" +with id: HelloWorldWithLogAndThrow-react-component-0 with railsContext:" console.log.apply(console, ["[SERVER] console.log in HelloWorld"]); console.warn.apply(console, ["[SERVER] console.warn in HelloWorld"]); console.error.apply(console, ["[SERVER] console.error in HelloWorld"]);