From f54c1e08e4b473c7fcf0d4cbee7fa8cb9be028f3 Mon Sep 17 00:00:00 2001 From: Emanuel Tesar Date: Tue, 16 Jul 2019 16:20:54 +0200 Subject: [PATCH 01/11] Add trusted types to react on client side --- .../src/client/DOMPropertyOperations.js | 9 +- .../react-dom/src/client/ReactDOMComponent.js | 16 ++-- .../react-dom/src/client/ToStringValue.js | 3 +- .../src/client/__tests__/trustedTypes-test.js | 85 +++++++++++++++++++ packages/react-dom/src/client/setInnerHTML.js | 13 ++- .../src/shared/trustedTypesAwareToString.js | 42 +++++++++ 6 files changed, 153 insertions(+), 15 deletions(-) create mode 100644 packages/react-dom/src/client/__tests__/trustedTypes-test.js create mode 100644 packages/react-dom/src/shared/trustedTypesAwareToString.js diff --git a/packages/react-dom/src/client/DOMPropertyOperations.js b/packages/react-dom/src/client/DOMPropertyOperations.js index 6a2d0f7d244a6..aef9630c9adad 100644 --- a/packages/react-dom/src/client/DOMPropertyOperations.js +++ b/packages/react-dom/src/client/DOMPropertyOperations.js @@ -16,6 +16,7 @@ import { OVERLOADED_BOOLEAN, } from '../shared/DOMProperty'; import sanitizeURL from '../shared/sanitizeURL'; +import trustedTypesAwareToString from '../shared/trustedTypesAwareToString'; import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags'; import type {PropertyInfo} from '../shared/DOMProperty'; @@ -142,7 +143,7 @@ export function setValueForProperty( if (value === null) { node.removeAttribute(attributeName); } else { - node.setAttribute(attributeName, '' + (value: any)); + node.setAttribute(attributeName, trustedTypesAwareToString(value)); } } return; @@ -168,13 +169,15 @@ 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. - attributeValue = '' + (value: any); + attributeValue = trustedTypesAwareToString(value); if (propertyInfo.sanitizeURL) { - sanitizeURL(attributeValue); + sanitizeURL('' + attributeValue); } } if (attributeNamespace) { diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index af580be510bee..3c1918173d324 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -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 trustedTypesAwareToString from '../shared/trustedTypesAwareToString'; import {enableFlareAPI} from 'shared/ReactFeatureFlags'; @@ -416,15 +417,7 @@ export function createElement( ); } - 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'); - div.innerHTML = ', 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 tamplate tag instead ' + + '(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).', + ); - 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.', - ); - } - }); + const spy = spyOnDev(console, 'error'); + ReactDOM.render(, container); + if (__DEV__) { + expect(spy).toHaveBeenCalledTimes(0); + } }); }); diff --git a/packages/react-dom/src/client/setInnerHTML.js b/packages/react-dom/src/client/setInnerHTML.js index 7cc699003a9a0..1a84701922988 100644 --- a/packages/react-dom/src/client/setInnerHTML.js +++ b/packages/react-dom/src/client/setInnerHTML.js @@ -11,6 +11,7 @@ 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; @@ -30,14 +31,10 @@ const setInnerHTML = createMicrosoftUnsafeLocalFunction(function( // new markup in a temp node and then move the child nodes across into // the target node if (node.namespaceURI === Namespaces.svg) { - if (__DEV__) { + if (enableTrustedTypesIntegration && __DEV__) { warningWithoutStack( - // 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' && - // $FlowExpectedError - trustedTypes are defined only in some browsers or with polyfill - typeof trustedTypes === 'undefined', + // $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 ' + diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 3b64033a6e2ff..17f5026449e7c 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -97,3 +97,5 @@ export const warnAboutStringRefs = false; export const disableLegacyContext = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; + +export const enableTrustedTypesIntegration = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 9dbed7ab7f987..d06b1b2bddca9 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -42,6 +42,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() { diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 4201b41bd266d..88975a5dd4b9c 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -37,6 +37,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() { diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js index 23d936b7eb50c..feff0ed82713a 100644 --- a/packages/shared/forks/ReactFeatureFlags.persistent.js +++ b/packages/shared/forks/ReactFeatureFlags.persistent.js @@ -37,6 +37,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() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index c308b949db368..1d12b604af27f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -37,6 +37,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() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index b452e018ef45b..4f945dde90615 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -35,6 +35,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() { diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 8965f4d42155f..8d49c0355d1a9 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -22,6 +22,7 @@ export const { enableUserBlockingEvents, disableLegacyContext, disableSchedulerTimeoutBasedOnReactExpirationTime, + enableTrustedTypesIntegration, } = require('ReactFeatureFlags'); // In www, we have experimental support for gathering data From 1e5b0bfeff663fe53ac1a91c7c8e19a7e5ed2a07 Mon Sep 17 00:00:00 2001 From: Emanuel Tesar Date: Wed, 11 Sep 2019 19:28:19 +0200 Subject: [PATCH 07/11] Move comment --- packages/react-dom/src/client/ToStringValue.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/client/ToStringValue.js b/packages/react-dom/src/client/ToStringValue.js index 6c5ed066ec0f2..4ab78cb56df36 100644 --- a/packages/react-dom/src/client/ToStringValue.js +++ b/packages/react-dom/src/client/ToStringValue.js @@ -65,9 +65,9 @@ export opaque type TrustedValue: {toString(): string} = {toString(): string}; * 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 ( enableTrustedTypesIntegration && + // fast-path string values as it's most frequent usage of the function typeof value !== 'string' && isTrustedTypesValue(value) ) { From 245d89b10980887b6cc9b8f36df741b5d15485f4 Mon Sep 17 00:00:00 2001 From: Emanuel Tesar Date: Wed, 11 Sep 2019 22:29:21 +0200 Subject: [PATCH 08/11] Fix PR comments --- .../react-dom/src/client/ReactDOMComponent.js | 21 +++++++++++-------- .../react-dom/src/client/ToStringValue.js | 2 +- .../__tests__/trustedTypes-test.internal.js | 11 +++++----- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 7f4d527f55bb6..48b52eb195ea9 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -422,19 +422,22 @@ 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__ && enableTrustedTypesIntegration && !didWarnScriptTags) { - warning( - false, - 'Encountered a script tag while rendering React component. ' + - 'Scripts inside React components are never executed when rendering' + - 'on the client. Consider using tamplate tag instead ' + - '(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).', - ); - didWarnScriptTags = true; + if (__DEV__) { + if (enableTrustedTypesIntegration && !didWarnScriptTags) { + 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; + } } div.innerHTML = ', 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 tamplate tag instead ' + - '(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).', + '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'); From b1d6e1943f0a5fc7651c2a3bb1abaa2541aa8f59 Mon Sep 17 00:00:00 2001 From: Emanuel Tesar Date: Fri, 13 Sep 2019 12:39:24 +0200 Subject: [PATCH 09/11] Fix html toString concatenation --- packages/react-dom/src/client/ToStringValue.js | 5 ++++- packages/react-dom/src/client/setInnerHTML.js | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/client/ToStringValue.js b/packages/react-dom/src/client/ToStringValue.js index 73bf5589ecfac..1cb8f60f0b5e7 100644 --- a/packages/react-dom/src/client/ToStringValue.js +++ b/packages/react-dom/src/client/ToStringValue.js @@ -55,7 +55,10 @@ if (enableTrustedTypesIntegration && typeof trustedTypes !== 'undefined') { } /** Trusted value is a wrapper for "safe" values which can be assigned to DOM execution sinks. */ -export opaque type TrustedValue: {toString(): string} = {toString(): string}; +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 diff --git a/packages/react-dom/src/client/setInnerHTML.js b/packages/react-dom/src/client/setInnerHTML.js index 1a84701922988..3313c7f9a70c6 100644 --- a/packages/react-dom/src/client/setInnerHTML.js +++ b/packages/react-dom/src/client/setInnerHTML.js @@ -46,7 +46,8 @@ const setInnerHTML = createMicrosoftUnsafeLocalFunction(function( if (!('innerHTML' in node)) { reusableSVGContainer = reusableSVGContainer || document.createElement('div'); - reusableSVGContainer.innerHTML = '' + html.toString() + ''; + reusableSVGContainer.innerHTML = + '' + html.valueOf().toString() + ''; const svgNode = reusableSVGContainer.firstChild; while (node.firstChild) { node.removeChild(node.firstChild); From 57e405fa847ebd5b0a5a55e9933a1ca8d2b6cea1 Mon Sep 17 00:00:00 2001 From: Emanuel Tesar Date: Fri, 13 Sep 2019 13:41:25 +0200 Subject: [PATCH 10/11] Fix forgotten else branch --- packages/react-dom/src/client/setInnerHTML.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-dom/src/client/setInnerHTML.js b/packages/react-dom/src/client/setInnerHTML.js index 3313c7f9a70c6..4845185f1a4a5 100644 --- a/packages/react-dom/src/client/setInnerHTML.js +++ b/packages/react-dom/src/client/setInnerHTML.js @@ -55,6 +55,8 @@ const setInnerHTML = createMicrosoftUnsafeLocalFunction(function( while (svgNode.firstChild) { node.appendChild(svgNode.firstChild); } + } else { + node.innerHTML = (html: any); } } else { node.innerHTML = (html: any); From 941ec849621e2bdf84fe6323de099a20a9a96839 Mon Sep 17 00:00:00 2001 From: Emanuel Tesar Date: Mon, 16 Sep 2019 10:57:34 +0200 Subject: [PATCH 11/11] Fix PR comments --- .../src/client/__tests__/trustedTypes-test.internal.js | 6 +----- packages/react-dom/src/client/setInnerHTML.js | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js index c2d251d197dde..3a9eb4c15d16c 100644 --- a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js +++ b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js @@ -80,7 +80,6 @@ describe('when Trusted Types are available in global object', () => { "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}, ); }); }); @@ -96,10 +95,7 @@ describe('when Trusted Types are available in global object', () => { ' in script (at **)', ); - const spy = spyOnDev(console, 'error'); + // check that the warning is print only once ReactDOM.render(, container); - if (__DEV__) { - expect(spy).toHaveBeenCalledTimes(0); - } }); }); diff --git a/packages/react-dom/src/client/setInnerHTML.js b/packages/react-dom/src/client/setInnerHTML.js index 4845185f1a4a5..4fa94a4e95c92 100644 --- a/packages/react-dom/src/client/setInnerHTML.js +++ b/packages/react-dom/src/client/setInnerHTML.js @@ -9,7 +9,7 @@ import {Namespaces} from '../shared/DOMNamespaces'; import createMicrosoftUnsafeLocalFunction from '../shared/createMicrosoftUnsafeLocalFunction'; -import warningWithoutStack from 'shared/warningWithoutStack'; +import warning from 'shared/warning'; import type {TrustedValue} from './ToStringValue'; import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; @@ -32,7 +32,7 @@ const setInnerHTML = createMicrosoftUnsafeLocalFunction(function( // the target node if (node.namespaceURI === Namespaces.svg) { if (enableTrustedTypesIntegration && __DEV__) { - warningWithoutStack( + warning( // $FlowExpectedError - trustedTypes are defined only in some browsers or with polyfill typeof trustedTypes === 'undefined', "Using 'dangerouslySetInnerHTML' in an svg element with " +