From 031fc24daeae6bcdc5e5f6959b778e1c2ed5f378 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 9 Sep 2015 23:19:00 -0700 Subject: [PATCH] Use a Symbol to tag every ReactElement Fixes #3473 I tag each React element with `$$typeof: Symbol.for('react.element')`. We need this to be able to safely distinguish these from plain objects that might have come from user provided JSON. The idiomatic JavaScript way of tagging an object is for it to inherent some prototype and then use `instanceof` to test for it. However, this has limitations since it doesn't work with value types which require `typeof` checks. They also don't work across realms. Which is why there are alternative tag checks like `Array.isArray` or the `toStringTag`. Another problem is that different instances of React that might have been created not knowing about eachother. npm tends to make this kind of problem occur a lot. Additionally, it is our hope that ReactElement will one day be specified in terms of a "Value Type" style record instead of a plain Object. This Value Types proposal by @nikomatsakis is currently on hold but does satisfy all these requirements: https://github.com/nikomatsakis/typed-objects-explainer/blob/master/valuetypes.md#the-typeof-operator Additionally, there is already a system for coordinating tags across module systems and even realms in ES6. Namely using `Symbol.for`. Currently these objects are not able to transfer between Workers but there is nothing preventing that from being possible in the future. You could imagine even `Symbol.for` working across Worker boundaries. You could also build a system that coordinates Symbols and Value Types from server to client or through serialized forms. That's beyond the scope of React itself, and if it was built it seems like it would belong with the `Symbol` system. A system could override the `Symbol.for('react.element')` to return a plain yet cryptographically random or unique number. That would allow ReactElements to pass through JSON without risking the XSS issue. The fallback solution is a plain well-known number. This makes it unsafe with regard to the XSS issue described in #3473. We could have used a much more convoluted solution to protect against JSON specifically but that would require some kind of significant coordination, or change the check to do a `typeof element.$$typeof === 'function'` check which would not make it unique to React. It seems cleaner to just use a fixed number since the protection is just a secondary layer anyway. I'm not sure if this is the right tradeoff. In short, if you want the XSS protection, use a proper Symbol polyfill. Finally, the reason for calling it `$$typeof` is to avoid confusion with `.type` and the use case is to add a tag that the `typeof` operator would refer to. I would use `@@typeof` but that seems to deopt in JSC. I also don't use `__typeof` because this is more than a framework private. It should really be part of the polyfilling layer. --- .../classic/element/ReactElement.js | 57 +++++++++++++------ .../element/__tests__/ReactElement-test.js | 51 +++++++++++++++++ .../element/__tests__/ReactJSXElement-test.js | 1 + 3 files changed, 93 insertions(+), 16 deletions(-) diff --git a/src/isomorphic/classic/element/ReactElement.js b/src/isomorphic/classic/element/ReactElement.js index b2c122b2a8afe..f24192b115390 100644 --- a/src/isomorphic/classic/element/ReactElement.js +++ b/src/isomorphic/classic/element/ReactElement.js @@ -15,6 +15,11 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var assign = require('Object.assign'); +// The Symbol used to tag the ReactElement type. If there is no native Symbol +// nor polyfill, then a plain number is used for performance. +var TYPE_SYMBOL = (typeof Symbol === 'function' && Symbol.for && + Symbol.for('react.element')) || 0xeac7; + var RESERVED_PROPS = { key: true, ref: true, @@ -52,17 +57,17 @@ if (__DEV__) { */ var ReactElement = function(type, key, ref, self, source, owner, props) { var element = { + // This tag allow us to uniquely identify this as a React Element + $$typeof: TYPE_SYMBOL, + // Built-in properties that belong on the element type: type, key: key, ref: ref, - self: self, - source: source, + props: props, // Record the component responsible for creating this element. _owner: owner, - - props: props, }; if (__DEV__) { @@ -83,8 +88,25 @@ var ReactElement = function(type, key, ref, self, source, owner, props) { writable: true, value: false, }); + // self and source are DEV only properties. + Object.defineProperty(element, '_self', { + configurable: false, + enumerable: false, + writable: false, + value: self, + }); + // Two elements created in two different places should be considered + // equal for testing purposes and therefore we hide it from enumeration. + Object.defineProperty(element, '_source', { + configurable: false, + enumerable: false, + writable: false, + value: source, + }); } else { - this._store.validated = false; + element._store.validated = false; + element._self = self; + element._source = source; } Object.freeze(element.props); Object.freeze(element); @@ -164,12 +186,12 @@ ReactElement.createFactory = function(type) { }; ReactElement.cloneAndReplaceKey = function(oldElement, newKey) { - var newElement = new ReactElement( + var newElement = ReactElement( oldElement.type, newKey, oldElement.ref, - oldElement.self, - oldElement.source, + oldElement._self, + oldElement._source, oldElement._owner, oldElement.props ); @@ -182,8 +204,8 @@ ReactElement.cloneAndReplaceProps = function(oldElement, newProps) { oldElement.type, oldElement.key, oldElement.ref, - oldElement.self, - oldElement.source, + oldElement._self, + oldElement._source, oldElement._owner, newProps ); @@ -205,8 +227,12 @@ ReactElement.cloneElement = function(element, config, children) { // Reserved names are extracted var key = element.key; var ref = element.ref; - var self = element.__self; - var source = element.__source; + // Self is preserved since the owner is preserved. + var self = element._self; + // Source is preserved since cloneElement is unlikely to be targeted by a + // transpiler, and the original source is probably a better indicator of the + // true owner. + var source = element._source; // Owner will be preserved, unless ref is overridden var owner = element._owner; @@ -259,11 +285,10 @@ ReactElement.cloneElement = function(element, config, children) { * @final */ ReactElement.isValidElement = function(object) { - return !!( + return ( typeof object === 'object' && - object != null && - 'type' in object && - 'props' in object + object !== null && + object.$$typeof === TYPE_SYMBOL ); }; diff --git a/src/isomorphic/classic/element/__tests__/ReactElement-test.js b/src/isomorphic/classic/element/__tests__/ReactElement-test.js index f7f8114ede330..2499eda8167d4 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElement-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElement-test.js @@ -24,6 +24,10 @@ describe('ReactElement', function() { beforeEach(function() { require('mock-modules').dumpCache(); + // Delete the native Symbol if we have one to ensure we test the + // unpolyfilled environment. + delete global.Symbol; + React = require('React'); ReactDOM = require('ReactDOM'); ReactTestUtils = require('ReactTestUtils'); @@ -190,6 +194,10 @@ describe('ReactElement', function() { expect(React.isValidElement('string')).toEqual(false); expect(React.isValidElement(React.DOM.div)).toEqual(false); expect(React.isValidElement(Component)).toEqual(false); + expect(React.isValidElement({ type: 'div', props: {} })).toEqual(false); + + var jsonElement = JSON.stringify(React.createElement('div')); + expect(React.isValidElement(JSON.parse(jsonElement))).toBe(true); }); it('allows the use of PropTypes validators in statics', function() { @@ -305,4 +313,47 @@ describe('ReactElement', function() { expect(console.error.argsForCall.length).toBe(0); }); + it('identifies elements, but not JSON, if Symbols are supported', function() { + // Rudimentary polyfill + // Once all jest engines support Symbols natively we can swap this to test + // WITH native Symbols by default. + var TYPE_SYMBOL = function() {}; // fake Symbol + var OTHER_SYMBOL = function() {}; // another fake Symbol + global.Symbol = function(name) { + return OTHER_SYMBOL; + }; + global.Symbol.for = function(key) { + if (key === 'react.element') { + return TYPE_SYMBOL; + } + return OTHER_SYMBOL; + }; + + require('mock-modules').dumpCache(); + + React = require('React'); + + var Component = React.createClass({ + render: function() { + return React.createElement('div'); + }, + }); + + expect(React.isValidElement(React.createElement('div'))) + .toEqual(true); + expect(React.isValidElement(React.createElement(Component))) + .toEqual(true); + + expect(React.isValidElement(null)).toEqual(false); + expect(React.isValidElement(true)).toEqual(false); + expect(React.isValidElement({})).toEqual(false); + expect(React.isValidElement('string')).toEqual(false); + expect(React.isValidElement(React.DOM.div)).toEqual(false); + expect(React.isValidElement(Component)).toEqual(false); + expect(React.isValidElement({ type: 'div', props: {} })).toEqual(false); + + var jsonElement = JSON.stringify(React.createElement('div')); + expect(React.isValidElement(JSON.parse(jsonElement))).toBe(false); + }); + }); diff --git a/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js b/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js index e0de22bb5ba3c..0a87205c17fab 100644 --- a/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js +++ b/src/isomorphic/modern/element/__tests__/ReactJSXElement-test.js @@ -153,6 +153,7 @@ describe('ReactJSXElement', function() { expect(React.isValidElement({})).toEqual(false); expect(React.isValidElement('string')).toEqual(false); expect(React.isValidElement(Component)).toEqual(false); + expect(React.isValidElement({ type: 'div', props: {} })).toEqual(false); }); it('is indistinguishable from a plain object', function() {