Don't stringify DOM attributes

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

@@ -16,10 +16,7 @@ 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';
@@ -153,10 +150,8 @@ export function setValueForProperty(
       if (value === null) {
         node.removeAttribute(attributeName);
       } else {
-        node.setAttribute(
-          attributeName,
-          enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
-        );
+        // Node.setAttribute(NS) stringifies the value implicitly in all browsers apart from IE <= 9.
+        node.setAttribute(attributeName, (value: any));
       }
     }
     return;
@@ -186,15 +181,10 @@ 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) {
-        attributeValue = (value: any);
-      } else {
-        attributeValue = '' + (value: any);
-      }
+      // Node.setAttribute(NS) stringifies the value implicitly in all browsers apart from IE <= 9.
+      attributeValue = (value: any);
       if (propertyInfo.sanitizeURL) {
-        sanitizeURL(attributeValue.toString());
+        attributeValue = sanitizeURL(attributeValue);
       }
     }
     if (attributeNamespace) {

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 {

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;