Merge pull request #370 from shakacode/fix-sanitization-server-consol…

…e-messages-2

Completes prior fix 366 for script sanitization

.rubocop.yml

@@ -48,7 +48,7 @@ Lint/HandleExceptions:
     - 'spec/dummy/bin/rake'
 
 Metrics/AbcSize:
-  Max: 26
+  Max: 28
 
 Metrics/CyclomaticComplexity:
   Max: 7

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

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.
 

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,17 +356,18 @@ 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
         href: request.original_url,
         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)

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);

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');
 };

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)<script>alert(\'WTF\')\
-(/script)","{\\"a\\":\\"Wow(/script)<script>alert(\'WTF\')(/script)\\",\\"b\\":2}"]);
+  const expected = `console.log.apply(console, ["some message (/script><script>alert(\'WTF\')\
+(/script>","{\\"a\\":\\"Wow(/script><script>alert(\'WTF\')(/script>\\",\\"b\\":2}"]);
 console.warn.apply(console, ["other message","{\\"c\\":3,\\"d\\":4}"]);`;
 
   assert.equals(actual, expected, 'Unexpected value for console replay history');

node_package/tests/scriptSanitizedVal.test.js

@@ -1,11 +1,47 @@
 import test from 'tape';
 import scriptSanitizedVal, { consoleReplay } from '../src/scriptSanitizedVal';
 
-test('scriptSanitizedVal returns no </script>', (assert) => {
+test('scriptSanitizedVal returns no </script if spaces, uppercase 1', (assert) => {
   assert.plan(1);
   const input = '[SERVER] This is a script:\"</div>\"</script> <script>alert(\'WTF\')</  SCRIPT >';
   const actual = scriptSanitizedVal(input);
-  const expected = '[SERVER] This is a script:\"</div>\"(/script) <script>alert(\'WTF\')(/script)';;
+  const expected = '[SERVER] This is a script:\"</div>\"(/script> <script>alert(\'WTF\')(/script >';
   assert.equals(actual, expected,
-    'consoleReplay should return an empty string if no console.history');
+    'scriptSanitizedVal replaces closing script tags');
+});
+
+test('scriptSanitizedVal returns no </script> 2', (assert) => {
+  assert.plan(1);
+  const input = 'Script2:"</div>"</script xx> <script>alert(\'WTF2\')</script xx>';
+  const actual = scriptSanitizedVal(input);
+  const expected = 'Script2:"</div>"(/script xx> <script>alert(\'WTF2\')(/script xx>';
+  assert.equals(actual, expected,
+    'scriptSanitizedVal replaces closing script tags');
+});
+
+test('scriptSanitizedVal returns no </script> 3', (assert) => {
+  assert.plan(1);
+  const input = 'Script3:"</div>"</  SCRIPT xx> <script>alert(\'WTF3\')</script xx>';
+  const actual = scriptSanitizedVal(input);
+  const expected = 'Script3:"</div>"(/script xx> <script>alert(\'WTF3\')(/script xx>';
+  assert.equals(actual, expected,
+    'scriptSanitizedVal replaces closing script tags');
+});
+
+test('scriptSanitizedVal returns no </script> 4', (assert) => {
+  assert.plan(1);
+  const input = 'Script4"</div>"</script <script>alert(\'WTF4\')</script>';
+  const actual = scriptSanitizedVal(input);
+  const expected = 'Script4"</div>"(/script <script>alert(\'WTF4\')(/script>';
+  assert.equals(actual, expected,
+    'scriptSanitizedVal replaces closing script tags');
+});
+
+test('scriptSanitizedVal returns no </script> 5', (assert) => {
+  assert.plan(1);
+  const input = 'Script5:"</div>"</ script> <script>alert(\'WTF5\')</script>';
+  const actual = scriptSanitizedVal(input);
+  const expected = 'Script5:"</div>"(/script> <script>alert(\'WTF5\')(/script>';
+  assert.equals(actual, expected,
+    'scriptSanitizedVal replaces closing script tags');
 });

spec/dummy/client/app/components/HelloWorldRedux.jsx

@@ -32,7 +32,11 @@ export default class HelloWorldRedux extends React.Component {
 
     // If this creates an alert, we have a problem!
     // see file node_package/src/scriptSanitizedVal.js for the fix to this prior issue.
-    console.log('This is a script:"</div>"</script> <script>alert(\'WTF\')</script>');
+    console.log('This is a script:"</div>"</script> <script>alert(\'WTF1\')</script>');
+    console.log('Script2:"</div>"</script xx> <script>alert(\'WTF2\')</script xx>');
+    console.log('Script3:"</div>"</  SCRIPT xx> <script>alert(\'WTF3\')</script xx>');
+    console.log('Script4"</div>"</script <script>alert(\'WTF4\')</script>');
+    console.log('Script5:"</div>"</ script> <script>alert(\'WTF5\')</script>');
 
     return (
       <div>

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(
-      <tr key={className}>
-        <td><strong>
-          {key}:&nbsp;
-        </strong></td>
-        <td className={className}>{value}</td>
-      </tr>
-    );
+    if (key !== 'serverSide') {
+      const className = `js-${key}`;
+      accum.push(
+        <tr key={className}>
+          <td><strong>
+            {key}:&nbsp;
+          </strong></td>
+          <td className={className}>{value + ''}</td>
+        </tr>
+      );
+    }
   }, []);
 }
 

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}",

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"]);