[Trusted Types] Don't stringify DOM attributes (facebook#19588 redo)

Summary: This is a resubmit of facebook#19588.

Fixes facebook#19587.

Author: onionymous

packages/react-dom-bindings/src/client/DOMPropertyOperations.js

@@ -8,10 +8,7 @@
  */
 
 import isAttributeNameSafe from '../shared/isAttributeNameSafe';
-import {
-  enableTrustedTypesIntegration,
-  enableCustomElementPropertySupport,
-} from 'shared/ReactFeatureFlags';
+import {enableCustomElementPropertySupport} from 'shared/ReactFeatureFlags';
 import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
 import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';
 
@@ -133,10 +130,7 @@ export function setValueForAttribute(
     if (__DEV__) {
       checkAttributeStringCoercion(value, name);
     }
-    node.setAttribute(
-      name,
-      enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
-    );
+    node.setAttribute(name, (value: any));
   }
 }
 
@@ -161,10 +155,7 @@ export function setValueForKnownAttribute(
   if (__DEV__) {
     checkAttributeStringCoercion(value, name);
   }
-  node.setAttribute(
-    name,
-    enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
-  );
+  node.setAttribute(name, (value: any));
 }
 
 export function setValueForNamespacedAttribute(
@@ -189,11 +180,7 @@ export function setValueForNamespacedAttribute(
   if (__DEV__) {
     checkAttributeStringCoercion(value, name);
   }
-  node.setAttributeNS(
-    namespace,
-    name,
-    enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
-  );
+  node.setAttributeNS(namespace, name, (value: any));
 }
 
 export function setValueForPropertyOnCustomComponent(

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<T>(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

packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js

@@ -17,22 +17,35 @@ 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) {
           throw new Error(this);
         }
         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(<a href={trustedScriptUrlHttps} />, 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(<a href={trustedScriptUrlJavascript} />, 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 {