Skip to content

Commit

Permalink
Use a Symbol to tag every ReactElement
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage committed Sep 10, 2015
1 parent a05691f commit cb07b33
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 16 deletions.
57 changes: 41 additions & 16 deletions src/isomorphic/classic/element/ReactElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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__) {
Expand All @@ -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);
Expand Down Expand Up @@ -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
);
Expand All @@ -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
);
Expand All @@ -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;
Expand Down Expand Up @@ -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
);
};

Expand Down
51 changes: 51 additions & 0 deletions src/isomorphic/classic/element/__tests__/ReactElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit cb07b33

Please sign in to comment.