diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index ad14b6498c83..d965dfd21d9f 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -9,7 +9,6 @@ import isAttributeNameSafe from '../shared/isAttributeNameSafe'; import { - enableTrustedTypesIntegration, enableCustomElementPropertySupport, } from 'shared/ReactFeatureFlags'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; @@ -135,7 +134,7 @@ export function setValueForAttribute( } node.setAttribute( name, - enableTrustedTypesIntegration ? (value: any) : '' + (value: any), + (value: any) ); } } @@ -163,7 +162,7 @@ export function setValueForKnownAttribute( } node.setAttribute( name, - enableTrustedTypesIntegration ? (value: any) : '' + (value: any), + (value: any) ); } @@ -192,7 +191,7 @@ export function setValueForNamespacedAttribute( node.setAttributeNS( namespace, name, - enableTrustedTypesIntegration ? (value: any) : '' + (value: any), + (value: any) ); } diff --git a/packages/react-dom-bindings/src/shared/sanitizeURL.js b/packages/react-dom-bindings/src/shared/sanitizeURL.js index b3de4657e915..7c60dfb38256 100644 --- a/packages/react-dom-bindings/src/shared/sanitizeURL.js +++ b/packages/react-dom-bindings/src/shared/sanitizeURL.js @@ -7,7 +7,10 @@ * @flow */ -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. @@ -28,6 +31,14 @@ function sanitizeURL(url: T): T | string { // We should never have symbols here because they get filtered out elsewhere. // eslint-disable-next-line react-internal/safe-string-coercion const stringifiedURL = '' + (url: any); + if ( + !enableTrustedTypesIntegration || + typeof trustedTypes === 'undefined' || + !trustedTypes.isScriptURL(url) + ) { + // Coerce to a string, unless we know it's an immutable TrustedScriptURL object. + url = stringifiedURL; + } if (disableJavaScriptURLs) { if (isJavaScriptProtocol.test(stringifiedURL)) { // Return a different javascript: url that doesn't cause any side-effects and just 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 777ef41756ce..64249658411e 100644 --- a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js +++ b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js @@ -17,10 +17,22 @@ describe('when Trusted Types are available in global object', () => { let ttObject1; let ttObject2; + 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 +40,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 = { @@ -152,6 +165,56 @@ 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 = { + // eslint-disable-next-line no-script-url + 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 {