Let sanitizeURL return a sanitized value

Don't unnecessarily checkAttributeStringCoercion since these have already been checked by us treating symbols as null.
facebook · Mar 29, 2023 · 666589e · 666589e
1 parent 85de6fd
commit 666589e
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 38 deletions.
diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js
@@ -50,7 +50,7 @@ export function getValueForProperty(
  if (__DEV__) {
  checkAttributeStringCoercion(expected, name);
  }
- sanitizeURL('' + (expected: any));
+ sanitizeURL(expected);
  }
 
  const attributeName = propertyInfo.attributeName;
@@ -395,19 +395,25 @@ export function setValueForProperty(node: Element, name: string, value: mixed) {
  }
  break;
  default: {
+ if (__DEV__) {
+ checkAttributeStringCoercion(value, attributeName);
+ }
  let attributeValue;
  // `setAttribute` with objects becomes only `[object]` in IE8/9,
  // ('' + value) makes it output the correct toString()-value.
  if (enableTrustedTypesIntegration) {
- attributeValue = (value: any);
- } else {
- if (__DEV__) {
- checkAttributeStringCoercion(value, attributeName);
+ if (propertyInfo.sanitizeURL) {
+  attributeValue = (sanitizeURL(value): any);
+ } else {
+ attributeValue = (value: any);
  }
+ } else {
+ // We have already verified this above.
+ // eslint-disable-next-line react-internal/safe-string-coercion
  attributeValue = '' + (value: any);
- }
- if (propertyInfo.sanitizeURL) {
- sanitizeURL(attributeValue.toString());
+  if (propertyInfo.sanitizeURL) {
+  attributeValue = sanitizeURL(attributeValue);
+ }
  }
  const attributeNamespace = propertyInfo.attributeNamespace;
  if (attributeNamespace) {

diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
@@ -736,12 +736,13 @@ function pushAttribute(
  }
  break;
  default:
+ if (__DEV__) {
+ checkAttributeStringCoercion(value, attributeName);
+ }
  if (propertyInfo.sanitizeURL) {
- if (__DEV__) {
- checkAttributeStringCoercion(value, attributeName);
- }
- value = '' + (value: any);
- sanitizeURL(value);
+ // We've already checked above.
+ // eslint-disable-next-line react-internal/safe-string-coercion
+ value = sanitizeURL('' + (value: any));
  }
  target.push(
  attributeSeparator,
@@ -3844,15 +3845,12 @@ function writeStyleResourceDependencyHrefOnlyInJS(
 
 function writeStyleResourceDependencyInJS(
  destination: Destination,
- href: string,
- precedence: string,
+ href: mixed,
+ precedence: mixed,
  props: Object,
 ) {
- if (__DEV__) {
- checkAttributeStringCoercion(href, 'href');
- }
- const coercedHref = '' + (href: any);
- sanitizeURL(coercedHref);
+ // eslint-disable-next-line react-internal/safe-string-coercion
+ const coercedHref = sanitizeURL('' + (href: any));
  writeChunk(
  destination,
  stringToChunk(escapeJSObjectForInstructionScripts(coercedHref)),
@@ -3936,11 +3934,7 @@ function writeStyleResourceAttributeInJS(
  // Santized URLs
  case 'src':
  case 'href': {
- if (__DEV__) {
- checkAttributeStringCoercion(value, attributeName);
- }
- attributeValue = '' + (value: any);
- sanitizeURL(attributeValue);
+ value = sanitizeURL(value);
  break;
  }
  default: {
@@ -4041,15 +4035,12 @@ function writeStyleResourceDependencyHrefOnlyInAttr(
 
 function writeStyleResourceDependencyInAttr(
  destination: Destination,
- href: string,
- precedence: string,
+ href: mixed,
+ precedence: mixed,
  props: Object,
 ) {
- if (__DEV__) {
- checkAttributeStringCoercion(href, 'href');
- }
- const coercedHref = '' + (href: any);
- sanitizeURL(coercedHref);
+ // eslint-disable-next-line react-internal/safe-string-coercion
+ const coercedHref = sanitizeURL('' + (href: any));
  writeChunk(
  destination,
  stringToChunk(escapeTextForBrowser(JSON.stringify(coercedHref))),
@@ -4136,8 +4127,7 @@ function writeStyleResourceAttributeInAttr(
  if (__DEV__) {
  checkAttributeStringCoercion(value, attributeName);
  }
- attributeValue = '' + (value: any);
- sanitizeURL(attributeValue);
+ value = sanitizeURL(value);
  break;
  }
  default: {

diff --git a/packages/react-dom-bindings/src/shared/sanitizeURL.js b/packages/react-dom-bindings/src/shared/sanitizeURL.js
@@ -24,24 +24,28 @@ const isJavaScriptProtocol =
 
 let didWarn = false;
 
-function sanitizeURL(url: string) {
+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 (disableJavaScriptURLs) {
- if (isJavaScriptProtocol.test(url)) {
+ if (isJavaScriptProtocol.test(stringifiedURL)) {
  throw new Error(
  'React has blocked a javascript: URL as a security precaution.',
  );
  }
  } else if (__DEV__) {
- if (!didWarn && isJavaScriptProtocol.test(url)) {
+ if (!didWarn && isJavaScriptProtocol.test(stringifiedURL)) {
  didWarn = true;
  console.error(
  'A future version of React will block javascript: URLs as a security precaution. ' +
  'Use event handlers instead if you can. If you need to generate unsafe HTML try ' +
  'using dangerouslySetInnerHTML instead. React was passed %s.',
- JSON.stringify(url),
+ JSON.stringify(stringifiedURL),
  );
  }
  }
+ return url;
 }
 
 export default sanitizeURL;