diff --git a/fixtures/dom/src/components/Header.js b/fixtures/dom/src/components/Header.js index fe9709af00318..9e0c24e41e170 100644 --- a/fixtures/dom/src/components/Header.js +++ b/fixtures/dom/src/components/Header.js @@ -89,6 +89,9 @@ class Header extends React.Component { Selection Events Suspense Form State + + Attribute Stringification + diff --git a/fixtures/dom/src/components/fixtures/attribute-stringification/AttributeStringificationTestCase.js b/fixtures/dom/src/components/fixtures/attribute-stringification/AttributeStringificationTestCase.js new file mode 100644 index 0000000000000..a0bffe9864bca --- /dev/null +++ b/fixtures/dom/src/components/fixtures/attribute-stringification/AttributeStringificationTestCase.js @@ -0,0 +1,36 @@ +import Fixture from '../../Fixture'; + +const React = window.React; + +class AttributeStringificationTestCase extends React.Component { + state = { + title: { + prop: 'if you see this, the test failed', + toString: () => 'stringified', + }, + }; + constructor(props) { + super(props); + this.input = React.createRef(); + } + componentDidMount() { + this.setState( + Object.assign(this.state, { + titleRead: this.input.current.getAttribute('title'), + }) + ); + } + + render() { + return ( + + + + Attribute Value: {JSON.stringify(this.state.titleRead)} + + + ); + } +} + +export default AttributeStringificationTestCase; diff --git a/fixtures/dom/src/components/fixtures/attribute-stringification/index.js b/fixtures/dom/src/components/fixtures/attribute-stringification/index.js new file mode 100644 index 0000000000000..2a5e5de97834e --- /dev/null +++ b/fixtures/dom/src/components/fixtures/attribute-stringification/index.js @@ -0,0 +1,28 @@ +import FixtureSet from '../../FixtureSet'; +import TestCase from '../../TestCase'; +import AttributeStringificationTestCase from './AttributeStringificationTestCase'; + +const React = window.React; + +function AttributeStringification() { + return ( + + + + The Attribute value displayed below the input field is "stringified". + The value is not "[object]". + + + + + + ); +} + +export default AttributeStringification; diff --git a/fixtures/dom/src/polyfills.js b/fixtures/dom/src/polyfills.js index ed6e08f59beb5..52146913f9244 100644 --- a/fixtures/dom/src/polyfills.js +++ b/fixtures/dom/src/polyfills.js @@ -2,6 +2,7 @@ import 'core-js/es6/symbol'; import 'core-js/es6/promise'; import 'core-js/es6/set'; import 'core-js/es6/map'; +import 'core-js/features/array/fill'; // http://paulirish.com/2011/requestanimationframe-for-smart-animating/ // http://my.opera.com/emoller/blog/2011/12/20/requestanimationframe-for-smart-er-animating diff --git a/packages/react-dom/src/client/DOMPropertyOperations.js b/packages/react-dom/src/client/DOMPropertyOperations.js index 2dcb1ace8ab9f..cafd72e0c3b8d 100644 --- a/packages/react-dom/src/client/DOMPropertyOperations.js +++ b/packages/react-dom/src/client/DOMPropertyOperations.js @@ -16,14 +16,32 @@ import { OVERLOADED_BOOLEAN, } from '../shared/DOMProperty'; import sanitizeURL from '../shared/sanitizeURL'; -import { - disableJavaScriptURLs, - enableTrustedTypesIntegration, -} from 'shared/ReactFeatureFlags'; +import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags'; import {isOpaqueHydratingObject} from './ReactDOMHostConfig'; import type {PropertyInfo} from '../shared/DOMProperty'; +/** + * Cached result of detectStringification() function. + * Should become true in all environments but IE<=9. + */ +let setAttributeCanStringify = undefined; + +/** + * Detect if Element.setAttribute stringifies attribute values. + * Should return true for all environments but IE <= 9. + * @param {DOMElement} node + */ +function detectStringification(node: Element) { + const obj: any = { + toString: () => 'foo', + }; + const attrName = 'reacttest'; + const el = node.ownerDocument.createElement('p'); + el.setAttribute(attrName, obj); + return el.getAttribute(attrName) === 'foo'; +} + /** * Get the value for a property on a node. Only used in DEV for SSR validation. * The "expected" argument is used as a hint of what the expected value is. @@ -146,6 +164,9 @@ export function setValueForProperty( if (shouldRemoveAttribute(name, value, propertyInfo, isCustomComponentTag)) { value = null; } + if (setAttributeCanStringify === undefined) { + setAttributeCanStringify = detectStringification(node); + } // If the prop isn't in the special list, treat it as a simple attribute. if (isCustomComponentTag || propertyInfo === null) { if (isAttributeNameSafe(name)) { @@ -155,7 +176,7 @@ export function setValueForProperty( } else { node.setAttribute( attributeName, - enableTrustedTypesIntegration ? (value: any) : '' + (value: any), + setAttributeCanStringify ? (value: any) : '' + (value: any), ); } } @@ -186,15 +207,15 @@ export function setValueForProperty( // 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. - if (enableTrustedTypesIntegration) { + if (setAttributeCanStringify) { attributeValue = (value: any); } else { + // As `setAttribute` does not stringify the value itself. + // ('' + value) makes it output the correct toString()-value. attributeValue = '' + (value: any); } if (propertyInfo.sanitizeURL) { - sanitizeURL(attributeValue.toString()); + attributeValue = sanitizeURL(attributeValue); } } if (attributeNamespace) { 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 f74186f87dc60..e5d00188bb9bb 100644 --- a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js +++ b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js @@ -16,11 +16,24 @@ describe('when Trusted Types are available in global object', () => { let container; let ttObject1; let ttObject2; + let fakeTTObjects; + + const expectToReject = fn => { + let msg; + try { + fn(); + } catch (x) { + msg = x.message; + } + expect(msg).toContain( + 'React has blocked a javascript: URL as a security precaution.', + ); + }; beforeEach(() => { jest.resetModules(); container = document.createElement('div'); - const fakeTTObjects = new Set(); + fakeTTObjects = new Set(); window.trustedTypes = { isHTML: function(value) { if (this !== window.trustedTypes) { @@ -28,11 +41,12 @@ describe('when Trusted Types are available in global object', () => { } return fakeTTObjects.has(value); }, - isScript: () => false, - isScriptURL: () => false, + isScript: (value) => fakeTTObjects.has(value), + isScriptURL: (value) => fakeTTObjects.has(value), }; ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.enableTrustedTypesIntegration = true; + ReactFeatureFlags.disableJavaScriptURLs = true; React = require('react'); ReactDOM = require('react-dom'); ttObject1 = { @@ -47,6 +61,9 @@ describe('when Trusted Types are available in global object', () => { }; fakeTTObjects.add(ttObject1); fakeTTObjects.add(ttObject2); + // Run setAttributeCanStringify detection first to simplify counting + // setAttribute calls later. + ReactDOM.render(, container); }); afterEach(() => { @@ -152,6 +169,55 @@ describe('when Trusted Types are available in global object', () => { } }); + it('should not stringify attributes that go through sanitizeURL', () => { + const setAttribute = Element.prototype.setAttribute; + try { + const setAttributeCalls = []; + Element.prototype.setAttribute = function(name, value) { + setAttributeCalls.push([this, name.toLowerCase(), value]); + return setAttribute.apply(this, arguments); + }; + const trustedScriptUrlHttps = { + toString: () => 'https://ok.example', + }; + fakeTTObjects.add(trustedScriptUrlHttps); + // It's not a matching type (under Trusted Types a.href a string will do), + // but a.href undergoes URL and we're only testing if the value was + // passed unmodified to setAttribute. + ReactDOM.render(, container); + expect(setAttributeCalls.length).toBe(1); + expect(setAttributeCalls[0][0]).toBe(container.firstChild); + expect(setAttributeCalls[0][1]).toBe('href'); + // Ensure it didn't get stringified when passed to a DOM sink: + expect(setAttributeCalls[0][2]).toBe(trustedScriptUrlHttps); + } finally { + Element.prototype.setAttribute = setAttribute; + } + }); + + it('should sanitize attributes even if they are Trusted Types', () => { + const setAttribute = Element.prototype.setAttribute; + try { + const setAttributeCalls = []; + Element.prototype.setAttribute = function(name, value) { + setAttributeCalls.push([this, name.toLowerCase(), value]); + return setAttribute.apply(this, arguments); + }; + const trustedScriptUrlJavascript = { + toString: () => 'javascript:notfine', + }; + fakeTTObjects.add(trustedScriptUrlJavascript); + // Assert that the URL sanitization will correctly unwrap and verify the + // value. + expectToReject(() => { + ReactDOM.render(, container) + }); + expect(setAttributeCalls.length).toBe(0); + } finally { + Element.prototype.setAttribute = setAttribute; + } + }); + it('should not stringify trusted values for setAttributeNS', () => { const setAttributeNS = Element.prototype.setAttributeNS; try { diff --git a/packages/react-dom/src/shared/sanitizeURL.js b/packages/react-dom/src/shared/sanitizeURL.js index 81d54478a9d76..faa9d9e55e0ed 100644 --- a/packages/react-dom/src/shared/sanitizeURL.js +++ b/packages/react-dom/src/shared/sanitizeURL.js @@ -8,7 +8,10 @@ */ import invariant from 'shared/invariant'; -import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags'; +import { + disableJavaScriptURLs, + enableTrustedTypesIntegration, +} from 'shared/ReactFeatureFlags'; // A javascript: URL can contain leading C0 control or \u0020 SPACE, // and any newline or tab are filtered out as if they're not part of the URL. @@ -24,7 +27,15 @@ const isJavaScriptProtocol = /^[\u0000-\u001F ]*j[\r\n\t]*a[\r\n\t]*v[\r\n\t]*a[ let didWarn = false; -function sanitizeURL(url: string) { +function sanitizeURL(url: any): any { + if ( + !enableTrustedTypesIntegration || + typeof trustedTypes === 'undefined' || + !trustedTypes.isScriptURL(url) + ) { + // Coerce to a string, unless we know it's an immutable TrustedScriptURL object. + url = '' + url; + } if (disableJavaScriptURLs) { invariant( !isJavaScriptProtocol.test(url), @@ -41,6 +52,7 @@ function sanitizeURL(url: string) { ); } } + return url; } export default sanitizeURL;
Attribute Value: {JSON.stringify(this.state.titleRead)}