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

Add trusted types to react on client side #16157

Merged
merged 13 commits into from
Sep 16, 2019
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,5 +149,6 @@ module.exports = {
spyOnProd: true,
__PROFILE__: true,
__UMD__: true,
trustedTypes: true,
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
},
};
16 changes: 14 additions & 2 deletions packages/react-dom/src/__tests__/ReactEmptyComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,22 @@ describe('ReactEmptyComponent', () => {

expect(function() {
ReactTestUtils.renderIntoDocument(instance1);
}).not.toThrow();
}).toWarnDev(
'Warning: Encountered script tag while rendering React component. ' +
'Scripts inside React components are parser inserted into document ' +
'and they are never executed. Furthemore rendering script nodes ' +
'inside components breaks when using Trusted Types.',
{withoutStack: true},
);
expect(function() {
ReactTestUtils.renderIntoDocument(instance2);
}).not.toThrow();
}).toWarnDev(
'Warning: Encountered script tag while rendering React component. ' +
'Scripts inside React components are parser inserted into document ' +
'and they are never executed. Furthemore rendering script nodes ' +
'inside components breaks when using Trusted Types.',
{withoutStack: true},
);

expect(log).toHaveBeenCalledTimes(4);
expect(log).toHaveBeenNthCalledWith(1, null);
Expand Down
14 changes: 9 additions & 5 deletions packages/react-dom/src/client/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import {
OVERLOADED_BOOLEAN,
} from '../shared/DOMProperty';
import sanitizeURL from '../shared/sanitizeURL';
import {toStringOrTrustedType} from './ToStringValue';
import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags';
import {setAttribute, setAttributeNS} from './setAttribute';

import type {PropertyInfo} from '../shared/DOMProperty';

Expand Down Expand Up @@ -142,7 +144,7 @@ export function setValueForProperty(
if (value === null) {
node.removeAttribute(attributeName);
} else {
node.setAttribute(attributeName, '' + (value: any));
setAttribute(node, attributeName, toStringOrTrustedType(value));
}
}
return;
Expand All @@ -168,19 +170,21 @@ export function setValueForProperty(
const {type} = propertyInfo;
let attributeValue;
if (type === BOOLEAN || (type === OVERLOADED_BOOLEAN && value === true)) {
// If attribute type is boolean, we know for sure it won't be an execution sink
// and we won't require Trusted Type here.
attributeValue = '';
} else {
// `setAttribute` with objects becomes only `[object]` in IE8/9,
// ('' + value) makes it output the correct toString()-value.
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
attributeValue = '' + (value: any);
attributeValue = toStringOrTrustedType(value);
if (propertyInfo.sanitizeURL) {
sanitizeURL(attributeValue);
sanitizeURL('' + attributeValue.toString());
}
}
if (attributeNamespace) {
node.setAttributeNS(attributeNamespace, attributeName, attributeValue);
setAttributeNS(node, attributeNamespace, attributeName, attributeValue);
} else {
node.setAttribute(attributeName, attributeValue);
setAttribute(node, attributeName, attributeValue);
}
}
}
16 changes: 14 additions & 2 deletions packages/react-dom/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import possibleStandardNames from '../shared/possibleStandardNames';
import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook';
import {validateProperties as validateInputProperties} from '../shared/ReactDOMNullInputValuePropHook';
import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook';
import {toStringOrTrustedType} from './ToStringValue';

import {enableFlareAPI} from 'shared/ReactFeatureFlags';

Expand Down Expand Up @@ -417,11 +418,19 @@ export function createElement(
type,
);
}

if (type === 'script') {
// Create the script via .innerHTML so its "parser-inserted" flag is
// set to true and it does not execute
const div = ownerDocument.createElement('div');
if (__DEV__) {
warningWithoutStack(
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
false,
'Encountered script tag while rendering React component. ' +
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
'Scripts inside React components are parser inserted into document ' +
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
'and they are never executed. Furthemore rendering script nodes ' +
'inside components breaks when using Trusted Types.',
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
);
}
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
div.innerHTML = '<script><' + '/script>'; // eslint-disable-line
// This is guaranteed to yield a script element.
const firstChild = ((div.firstChild: any): HTMLScriptElement);
Expand Down Expand Up @@ -776,7 +785,10 @@ export function diffProperties(
const lastHtml = lastProp ? lastProp[HTML] : undefined;
if (nextHtml != null) {
if (lastHtml !== nextHtml) {
(updatePayload = updatePayload || []).push(propKey, '' + nextHtml);
(updatePayload = updatePayload || []).push(
propKey,
toStringOrTrustedType(nextHtml),
);
}
} else {
// TODO: It might be too late to clear this if we have children
Expand Down
35 changes: 35 additions & 0 deletions packages/react-dom/src/client/ToStringValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,38 @@ export function getToStringValue(value: mixed): ToStringValue {
return '';
}
}

/**
* Returns true only if Trusted Types are available in global object and the value is a trusted type.
*/
let isTrustedTypesValue: (value: any) => boolean;
// $FlowExpectedError - TrustedTypes are defined only in some browsers or with polyfill
if (typeof trustedTypes !== 'undefined') {
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
isTrustedTypesValue = (value: any) =>
trustedTypes.isHTML(value) ||
trustedTypes.isScript(value) ||
trustedTypes.isScriptURL(value) ||
// TrustedURLs are deprecated and will be removed soon: https://github.com/WICG/trusted-types/pull/204
Copy link
Collaborator

@gaearon gaearon Sep 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even want to support them then?

Copy link
Contributor Author

@Siegrift Siegrift Sep 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at least for now we shouldn't. Both chrome and the polyfill currently support them and if we don't, trying out the integration will for the users will be harder (because the values will be stringified). They would need to create a default policy that would bless all URLs.

(trustedTypes.isURL && trustedTypes.isURL(value));
} else {
isTrustedTypesValue = () => false;
}

/** Trusted value is a wrapper for "safe" values which can be assigned to DOM execution sinks. */
export opaque type TrustedValue: {toString(): string} = {toString(): string};

/**
* We allow passing objects with toString method as element attributes or in dangerouslySetInnerHTML
* and we do validations that the value is safe. Once we do validation we want to use the validated
* value instead of the object (because object.toString may return something else on next call).
*
* If application uses Trusted Types we don't stringify trusted values, but preserve them as objects.
*/
export function toStringOrTrustedType(value: any): string | TrustedValue {
// fast-path string values as it's most frequent usage of the function
if (typeof value !== 'string' && isTrustedTypesValue(value)) {
return value;
} else {
return '' + value;
}
}
88 changes: 88 additions & 0 deletions packages/react-dom/src/client/__tests__/trustedTypes-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
describe('when Trusted Types are available in global object', () => {
let React;
let ReactDOM;

beforeEach(() => {
window.trustedTypes = {
isHTML: () => true,
isScript: () => false,
isScriptURL: () => false,
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
};
React = require('react');
ReactDOM = require('react-dom');
});

afterEach(() => {
delete window.trustedTypes;
});

it('should not stringify trusted values', () => {
const container = document.createElement('div');
const trustedObject = {toString: () => 'I look like a trusted object'};
class Component extends React.Component {
state = {inner: undefined};
render() {
return <div dangerouslySetInnerHTML={{__html: this.state.inner}} />;
}
}

const isHTMLSpy = jest.spyOn(window.trustedTypes, ['isHTML']);
const instace = ReactDOM.render(<Component />, container);
instace.setState({inner: trustedObject});

expect(container.firstChild.innerHTML).toBe(trustedObject.toString());
expect(isHTMLSpy).toHaveBeenCalledWith(trustedObject);
});

describe('dangerouslySetInnerHTML in svg elements in Internet Explorer', () => {
let innerHTMLDescriptor;

// simulate svg elements in Internet Explorer which don't have 'innerHTML' property
beforeEach(() => {
innerHTMLDescriptor = Object.getOwnPropertyDescriptor(
Element.prototype,
'innerHTML',
);
delete Element.prototype.innerHTML;
Object.defineProperty(
HTMLDivElement.prototype,
'innerHTML',
innerHTMLDescriptor,
);
});

afterEach(() => {
delete HTMLDivElement.prototype.innerHTML;
Object.defineProperty(
Element.prototype,
'innerHTML',
innerHTMLDescriptor,
);
});

it('should log a warning', () => {
class Component extends React.Component {
state = {inner: undefined};
render() {
return <svg dangerouslySetInnerHTML={{__html: this.state.inner}} />;
}
}
const errorSpy = spyOnDev(console, 'error');

const container = document.createElement('div');
const instace = ReactDOM.render(<Component />, container);
instace.setState({inner: 'anyValue'});

if (__DEV__) {
expect(errorSpy).toHaveBeenCalledWith(
"Warning: Using 'dangerouslySetInnerHTML' in an svg element with " +
'Trusted Types enabled in an Internet Explorer will cause ' +
'the trusted value to be converted to string. Assigning string ' +
"to 'innerHTML' will throw an error if Trusted Types are enforced. " +
"You can try to wrap your svg element inside a div and use 'dangerouslySetInnerHTML' " +
'on the enclosing div instead.',
);
}
});
});
});
35 changes: 35 additions & 0 deletions packages/react-dom/src/client/setAttribute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import type {TrustedValue} from './ToStringValue';

/**
* Set attribute for a node. The attribute value can be either string or
* Trusted value (if application uses Trusted Types).
*/
export function setAttribute(
node: Element,
attributeName: string,
attributeValue: string | TrustedValue,
) {
node.setAttribute(attributeName, (attributeValue: any));
}

/**
* Set attribute with namespace for a node. The attribute value can be either string or
* Trusted value (if application uses Trusted Types).
*/
export function setAttributeNS(
node: Element,
attributeNamespace: string,
attributeName: string,
attributeValue: string | TrustedValue,
) {
node.setAttributeNS(attributeNamespace, attributeName, (attributeValue: any));
}
43 changes: 31 additions & 12 deletions packages/react-dom/src/client/setInnerHTML.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

import {Namespaces} from '../shared/DOMNamespaces';
import createMicrosoftUnsafeLocalFunction from '../shared/createMicrosoftUnsafeLocalFunction';
import warningWithoutStack from 'shared/warningWithoutStack';
import type {TrustedValue} from './ToStringValue';

// SVG temp container for IE lacking innerHTML
let reusableSVGContainer;
Expand All @@ -22,25 +24,42 @@ let reusableSVGContainer;
*/
const setInnerHTML = createMicrosoftUnsafeLocalFunction(function(
node: Element,
html: string,
html: string | TrustedValue,
): void {
// IE does not have innerHTML for SVG nodes, so instead we inject the
// new markup in a temp node and then move the child nodes across into
// the target node

if (node.namespaceURI === Namespaces.svg && !('innerHTML' in node)) {
reusableSVGContainer =
reusableSVGContainer || document.createElement('div');
reusableSVGContainer.innerHTML = '<svg>' + html + '</svg>';
const svgNode = reusableSVGContainer.firstChild;
while (node.firstChild) {
node.removeChild(node.firstChild);
if (node.namespaceURI === Namespaces.svg) {
if (__DEV__) {
warningWithoutStack(
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
// Trusted Types undergo a change where window.TrustedTypes was renamed to window.trustedTypes
// (https://github.com/WICG/trusted-types/pull/205).
// $FlowExpectedError - TrustedTypes are defined only in some browsers or with polyfill
typeof TrustedTypes === 'undefined' &&
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
// $FlowExpectedError - trustedTypes are defined only in some browsers or with polyfill
typeof trustedTypes === 'undefined',
"Using 'dangerouslySetInnerHTML' in an svg element with " +
'Trusted Types enabled in an Internet Explorer will cause ' +
'the trusted value to be converted to string. Assigning string ' +
"to 'innerHTML' will throw an error if Trusted Types are enforced. " +
"You can try to wrap your svg element inside a div and use 'dangerouslySetInnerHTML' " +
'on the enclosing div instead.',
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
);
}
while (svgNode.firstChild) {
node.appendChild(svgNode.firstChild);
if (!('innerHTML' in node)) {
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
reusableSVGContainer =
reusableSVGContainer || document.createElement('div');
reusableSVGContainer.innerHTML = '<svg>' + html.toString() + '</svg>';
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
const svgNode = reusableSVGContainer.firstChild;
while (node.firstChild) {
node.removeChild(node.firstChild);
}
while (svgNode.firstChild) {
node.appendChild(svgNode.firstChild);
}
}
} else {
node.innerHTML = html;
node.innerHTML = (html: any);
}
});

Expand Down
2 changes: 2 additions & 0 deletions scripts/rollup/validate/eslintrc.cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ module.exports = {
process: true,
setImmediate: true,
Buffer: true,
// Trusted Types
trustedTypes: true,

// Scheduler profiling
SharedArrayBuffer: true,
Expand Down
2 changes: 2 additions & 0 deletions scripts/rollup/validate/eslintrc.fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ module.exports = {
// Node.js Server Rendering
setImmediate: true,
Buffer: true,
// Trusted Types
trustedTypes: true,

// Scheduler profiling
SharedArrayBuffer: true,
Expand Down
2 changes: 2 additions & 0 deletions scripts/rollup/validate/eslintrc.rn.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ module.exports = {
// Fabric. See https://github.com/facebook/react/pull/15490
// for more information
nativeFabricUIManager: true,
// Trusted Types
trustedTypes: true,

// Scheduler profiling
SharedArrayBuffer: true,
Expand Down
2 changes: 2 additions & 0 deletions scripts/rollup/validate/eslintrc.umd.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ module.exports = {
define: true,
require: true,
global: true,
// Trusted Types
trustedTypes: true,

// Scheduler profiling
SharedArrayBuffer: true,
Expand Down