diff --git a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js index d63b71ed7d577..c2abd6f4f5674 100644 --- a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js @@ -7,9 +7,10 @@ import {shorthandToLonghand} from './CSSShorthandProperty'; -import dangerousStyleValue from './dangerousStyleValue'; import hyphenateStyleName from '../shared/hyphenateStyleName'; import warnValidStyle from '../shared/warnValidStyle'; +import {isUnitlessNumber} from '../shared/CSSProperty'; +import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion'; /** * Operations for dealing with CSS properties. @@ -29,19 +30,36 @@ export function createDangerousStringForStyles(styles) { if (!styles.hasOwnProperty(styleName)) { continue; } - const styleValue = styles[styleName]; - if (styleValue != null) { + const value = styles[styleName]; + if (value != null && typeof value !== 'boolean' && value !== '') { const isCustomProperty = styleName.indexOf('--') === 0; - serialized += - delimiter + - (isCustomProperty ? styleName : hyphenateStyleName(styleName)) + - ':'; - serialized += dangerousStyleValue( - styleName, - styleValue, - isCustomProperty, - ); - + if (isCustomProperty) { + if (__DEV__) { + checkCSSPropertyStringCoercion(value, styleName); + } + serialized += delimiter + styleName + ':' + ('' + value).trim(); + } else { + if ( + typeof value === 'number' && + value !== 0 && + !( + isUnitlessNumber.hasOwnProperty(styleName) && + isUnitlessNumber[styleName] + ) + ) { + serialized += + delimiter + hyphenateStyleName(styleName) + ':' + value + 'px'; + } else { + if (__DEV__) { + checkCSSPropertyStringCoercion(value, styleName); + } + serialized += + delimiter + + hyphenateStyleName(styleName) + + ':' + + ('' + value).trim(); + } + } delimiter = ';'; } } @@ -58,28 +76,46 @@ export function createDangerousStringForStyles(styles) { */ export function setValueForStyles(node, styles) { const style = node.style; - for (let styleName in styles) { + for (const styleName in styles) { if (!styles.hasOwnProperty(styleName)) { continue; } + const value = styles[styleName]; const isCustomProperty = styleName.indexOf('--') === 0; if (__DEV__) { if (!isCustomProperty) { - warnValidStyle(styleName, styles[styleName]); + warnValidStyle(styleName, value); } } - const styleValue = dangerousStyleValue( - styleName, - styles[styleName], - isCustomProperty, - ); - if (styleName === 'float') { - styleName = 'cssFloat'; - } - if (isCustomProperty) { - style.setProperty(styleName, styleValue); + + if (value == null || typeof value === 'boolean' || value === '') { + if (isCustomProperty) { + style.setProperty(styleName, ''); + } else if (styleName === 'float') { + style.cssFloat = ''; + } else { + style[styleName] = ''; + } + } else if (isCustomProperty) { + style.setProperty(styleName, value); + } else if ( + typeof value === 'number' && + value !== 0 && + !( + isUnitlessNumber.hasOwnProperty(styleName) && + isUnitlessNumber[styleName] + ) + ) { + style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers } else { - style[styleName] = styleValue; + if (styleName === 'float') { + style.cssFloat = value; + } else { + if (__DEV__) { + checkCSSPropertyStringCoercion(value, styleName); + } + style[styleName] = ('' + value).trim(); + } } } } diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js index fdd4484f714e6..d7d6bc78c1285 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js @@ -70,7 +70,6 @@ import { DOCUMENT_TYPE_NODE, DOCUMENT_FRAGMENT_NODE, } from './HTMLNodeType'; -import dangerousStyleValue from './dangerousStyleValue'; import {retryIfBlockedOn} from '../events/ReactDOMEventReplaying'; @@ -750,7 +749,12 @@ export function unhideInstance(instance: Instance, props: Props): void { styleProp.hasOwnProperty('display') ? styleProp.display : null; - instance.style.display = dangerousStyleValue('display', display); + instance.style.display = + display == null || typeof display === 'boolean' + ? '' + : // The value would've errored already if it wasn't safe. + // eslint-disable-next-line react-internal/safe-string-coercion + ('' + display).trim(); } export function unhideTextInstance( diff --git a/packages/react-dom-bindings/src/client/dangerousStyleValue.js b/packages/react-dom-bindings/src/client/dangerousStyleValue.js deleted file mode 100644 index bef3e41b062e0..0000000000000 --- a/packages/react-dom-bindings/src/client/dangerousStyleValue.js +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {isUnitlessNumber} from '../shared/CSSProperty'; -import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion'; - -/** - * Convert a value into the proper css writable value. The style name `name` - * should be logical (no hyphens), as specified - * in `CSSProperty.isUnitlessNumber`. - * - * @param {string} name CSS property name such as `topMargin`. - * @param {*} value CSS property value such as `10px`. - * @return {string} Normalized style value with dimensions applied. - */ -function dangerousStyleValue(name, value, isCustomProperty) { - // Note that we've removed escapeTextForBrowser() calls here since the - // whole string will be escaped when the attribute is injected into - // the markup. If you provide unsafe user data here they can inject - // arbitrary CSS which may be problematic (I couldn't repro this): - // https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet - // http://www.thespanner.co.uk/2007/11/26/ultimate-xss-css-injection/ - // This is not an XSS hole but instead a potential CSS injection issue - // which has lead to a greater discussion about how we're going to - // trust URLs moving forward. See #2115901 - - const isEmpty = value == null || typeof value === 'boolean' || value === ''; - if (isEmpty) { - return ''; - } - - if ( - !isCustomProperty && - typeof value === 'number' && - value !== 0 && - !(isUnitlessNumber.hasOwnProperty(name) && isUnitlessNumber[name]) - ) { - return value + 'px'; // Presumes implicit 'px' suffix for unitless numbers - } - - if (__DEV__) { - checkCSSPropertyStringCoercion(value, name); - } - return ('' + value).trim(); -} - -export default dangerousStyleValue;