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

Summary: This is a resubmit of facebook#19588 since it was never merged and closed in error. However, the issue it fixes is still relevant and will unblock rollout of the TT feature flag internally. Copying the original PR message here:

-----

Instead of using Trusted Types feature flag, assume that the browser would stringify the attributes, so that React-DOM doesn't have to (making interpolation into node attributes "just work" regardless of the Trusted Types enforcement and availability) . Currently only IE<=9 does not stringify attributes. This PR implicitly drops the support for IE 9.

For attributes undergoing sanitizeURL, the value is stringified in sanitizeURL function, unless enableTrustedTypesIntegration is true and the value is an immutable TrustedScriptURL value. This ascertains that objects with custom toString() function cannot be used to bypass the sanitization (now that DOMPropertyOperations don't stringify on their own).

Fixes facebook#19587.

## Summary
The PR decouples the attribute stringification from the Trusted Types logic, dropping the former completely. It was only used as a workaround for a IE <=9 browser bug. A small improvement for Trusted Types would be that it moves the most important functionality from behind the flag - i.e. allows most React apps to pass trusted types into DOM sinks without enabling the feature flag.

Some rare functionality and dev warnings are still gated on the flag, but those are unrelated to the stringification issue.

## Test Plan
Appropriate tests are added.

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

@@ -16,23 +16,37 @@ 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) {
           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 +166,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 {