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
},
};
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);
}
}
}
24 changes: 22 additions & 2 deletions packages/react-dom/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,16 @@ 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';
import {
enableFlareAPI,
enableTrustedTypesIntegration,
} from 'shared/ReactFeatureFlags';

let didWarnInvalidHydration = false;
let didWarnShadyDOM = false;
let didWarnScriptTags = false;

const DANGEROUSLY_SET_INNER_HTML = 'dangerouslySetInnerHTML';
const SUPPRESS_CONTENT_EDITABLE_WARNING = 'suppressContentEditableWarning';
Expand Down Expand Up @@ -422,6 +427,18 @@ export function createElement(
// 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__) {
if (enableTrustedTypesIntegration && !didWarnScriptTags) {
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
warning(
false,
'Encountered a script tag while rendering React component. ' +
'Scripts inside React components are never executed when rendering ' +
'on the client. Consider using template tag instead ' +
'(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).',
);
didWarnScriptTags = true;
}
}
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 +793,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
44 changes: 44 additions & 0 deletions packages/react-dom/src/client/ToStringValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @flow
*/

import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags';

export opaque type ToStringValue =
| boolean
| number
Expand Down Expand Up @@ -35,3 +37,45 @@ 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 (enableTrustedTypesIntegration && typeof trustedTypes !== 'undefined') {
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, valueOf(): string} = {
toString(): string,
valueOf(): 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 {
if (
enableTrustedTypesIntegration &&
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
// fast-path string values as it's most frequent usage of the function
typeof value !== 'string' &&
isTrustedTypesValue(value)
) {
return value;
} else {
return '' + value;
}
}
105 changes: 105 additions & 0 deletions packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
describe('when Trusted Types are available in global object', () => {
let React;
let ReactDOM;
let ReactFeatureFlags;
let container;

beforeEach(() => {
container = document.createElement('div');
window.trustedTypes = {
isHTML: () => true,
isScript: () => false,
isScriptURL: () => false,
};
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.enableTrustedTypesIntegration = true;
React = require('react');
ReactDOM = require('react-dom');
});

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

it('should not stringify trusted values', () => {
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 instance = ReactDOM.render(<Component />, container);
instance.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 {
render() {
return <svg dangerouslySetInnerHTML={{__html: 'unsafe html'}} />;
}
}
expect(() => {
ReactDOM.render(<Component />, container);
}).toWarnDev(
"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.',
{withoutStack: true},
);
});
});

it('should warn once when rendering script tag in jsx on client', () => {
expect(() => {
ReactDOM.render(<script>alert("I am not executed")</script>, container);
}).toWarnDev(
'Warning: Encountered a script tag while rendering React component. ' +
'Scripts inside React components are never executed when rendering ' +
'on the client. Consider using template tag instead ' +
'(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).\n' +
' in script (at **)',
);

const spy = spyOnDev(console, 'error');
ReactDOM.render(<script>alert("I am not executed")</script>, container);
if (__DEV__) {
Siegrift marked this conversation as resolved.
Show resolved Hide resolved
expect(spy).toHaveBeenCalledTimes(0);
}
});
});
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));
}
41 changes: 29 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,9 @@

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

// SVG temp container for IE lacking innerHTML
let reusableSVGContainer;
Expand All @@ -22,25 +25,39 @@ 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 (enableTrustedTypesIntegration && __DEV__) {
warningWithoutStack(
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.valueOf().toString() + '</svg>';
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 packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,5 @@ export const warnAboutStringRefs = false;
export const disableLegacyContext = false;

export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;

export const enableTrustedTypesIntegration = false;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const warnAboutDefaultPropsOnFunctionComponents = false;
export const warnAboutStringRefs = false;
export const disableLegacyContext = false;
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const warnAboutDefaultPropsOnFunctionComponents = false;
export const warnAboutStringRefs = false;
export const disableLegacyContext = false;
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const warnAboutDefaultPropsOnFunctionComponents = false;
export const warnAboutStringRefs = false;
export const disableLegacyContext = false;
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const warnAboutDefaultPropsOnFunctionComponents = false;
export const warnAboutStringRefs = false;
export const disableLegacyContext = false;
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
Loading