From 88011477e866bd1cb3ccc5878bc8ad69f58de251 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker <nate.hunzaker@gmail.com> Date: Fri, 8 Apr 2016 11:07:50 -0400 Subject: [PATCH] Strip whitelist. Move casing warnings to dev tools --- src/renderers/dom/shared/DOMProperty.js | 40 ++---- .../dom/shared/DOMPropertyOperations.js | 6 +- .../dom/shared/HTMLDOMPropertyConfig.js | 134 ------------------ src/renderers/dom/shared/ReactDOMComponent.js | 20 ++- .../dom/shared/SVGDOMPropertyConfig.js | 91 ------------ .../__tests__/DOMPropertyOperations-test.js | 4 +- .../ReactDOMUnknownPropertyDevtool.js | 38 ++--- 7 files changed, 49 insertions(+), 284 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index 814bf5045436b..793dc0ef5092c 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -17,6 +17,15 @@ function checkMask(value, bitmask) { return (value & bitmask) === bitmask; } +var reservedProps = { + key: true, + ref: true, + children: true, + innerHTML: true, + dangerouslySetInnerHTML: true, + suppressContentEditableWarning: true, +}; + var DOMPropertyInjection = { /** * Mapping from normalized, camelcased property names to a configuration that @@ -33,11 +42,6 @@ var DOMPropertyInjection = { * Inject some specialized knowledge about the DOM. This takes a config object * with the following properties: * - * isCustomAttribute: function that given an attribute name will return true - * if it can be inserted into the DOM verbatim. Useful for data-* or aria-* - * attributes where it's impossible to enumerate all of the possible - * attribute names, - * * Properties: object mapping DOM property name to one of the * DOMPropertyInjection constants or null. If your attribute isn't in here, * it won't get written to the DOM. @@ -65,12 +69,6 @@ var DOMPropertyInjection = { var DOMPropertyNames = domPropertyConfig.DOMPropertyNames || {}; var DOMMutationMethods = domPropertyConfig.DOMMutationMethods || {}; - if (domPropertyConfig.isCustomAttribute) { - DOMProperty._isCustomAttributeFunctions.push( - domPropertyConfig.isCustomAttribute - ); - } - for (var propName in Properties) { invariant( !DOMProperty.properties.hasOwnProperty(propName), @@ -211,25 +209,15 @@ var DOMProperty = { getPossibleStandardName: __DEV__ ? {} : null, /** - * All of the isCustomAttribute() functions that have been injected. + * Checks whether a property is a reserved word in react. + * @param {string} propName name of a property */ - _isCustomAttributeFunctions: [], - - /** - * Checks whether a property name is a custom attribute. - * @method - */ - isCustomAttribute: function(attributeName) { - for (var i = 0; i < DOMProperty._isCustomAttributeFunctions.length; i++) { - var isCustomAttributeFn = DOMProperty._isCustomAttributeFunctions[i]; - if (isCustomAttributeFn(attributeName)) { - return true; - } - } - return false; + isReservedProp: function (propName) { + return reservedProps.hasOwnProperty(propName) && reservedProps[propName]; }, injection: DOMPropertyInjection, + }; module.exports = DOMProperty; diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 6108e4dd079d4..c61ab1011a243 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -103,7 +103,7 @@ var DOMPropertyOperations = { return attributeName + '=""'; } return attributeName + '=' + quoteAttributeValueForBrowser(value); - } else if (DOMProperty.isCustomAttribute(name)) { + } else { if (value == null) { return ''; } @@ -171,7 +171,7 @@ var DOMPropertyOperations = { node.setAttribute(attributeName, '' + value); } } - } else if (DOMProperty.isCustomAttribute(name)) { + } else { DOMPropertyOperations.setValueForAttribute(node, name, value); } }, @@ -217,7 +217,7 @@ var DOMPropertyOperations = { } else { node.removeAttribute(propertyInfo.attributeName); } - } else if (DOMProperty.isCustomAttribute(name)) { + } else { node.removeAttribute(name); } }, diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index ce5e93cea6e1a..a87be8817da9a 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -23,183 +23,49 @@ var HAS_OVERLOADED_BOOLEAN_VALUE = DOMProperty.injection.HAS_OVERLOADED_BOOLEAN_VALUE; var HTMLDOMPropertyConfig = { - isCustomAttribute: RegExp.prototype.test.bind( - new RegExp('^(data|aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$') - ), Properties: { /** * Standard Properties */ - accept: 0, - acceptCharset: 0, - accessKey: 0, - action: 0, allowFullScreen: HAS_BOOLEAN_VALUE, - allowTransparency: 0, - alt: 0, async: HAS_BOOLEAN_VALUE, - autoComplete: 0, // autoFocus is polyfilled/normalized by AutoFocusUtils // autoFocus: HAS_BOOLEAN_VALUE, autoPlay: HAS_BOOLEAN_VALUE, capture: HAS_BOOLEAN_VALUE, - cellPadding: 0, - cellSpacing: 0, - charSet: 0, - challenge: 0, checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, - cite: 0, - classID: 0, className: 0, cols: HAS_POSITIVE_NUMERIC_VALUE, - colSpan: 0, - content: 0, - contentEditable: 0, - contextMenu: 0, controls: HAS_BOOLEAN_VALUE, - coords: 0, - crossOrigin: 0, - data: 0, // For `<object />` acts as `src`. - dateTime: 0, default: HAS_BOOLEAN_VALUE, defer: HAS_BOOLEAN_VALUE, - dir: 0, disabled: HAS_BOOLEAN_VALUE, download: HAS_OVERLOADED_BOOLEAN_VALUE, - draggable: 0, - encType: 0, - form: 0, - formAction: 0, - formEncType: 0, - formMethod: 0, formNoValidate: HAS_BOOLEAN_VALUE, - formTarget: 0, - frameBorder: 0, - headers: 0, - height: 0, hidden: HAS_BOOLEAN_VALUE, - high: 0, - href: 0, - hrefLang: 0, htmlFor: 0, httpEquiv: 0, - icon: 0, - id: 0, - inputMode: 0, - integrity: 0, - is: 0, - keyParams: 0, - keyType: 0, - kind: 0, - label: 0, - lang: 0, - list: 0, loop: HAS_BOOLEAN_VALUE, - low: 0, - manifest: 0, - marginHeight: 0, - marginWidth: 0, - max: 0, - maxLength: 0, - media: 0, - mediaGroup: 0, - method: 0, - min: 0, - minLength: 0, // Caution; `option.selected` is not updated if `select.multiple` is // disabled with `removeAttribute`. multiple: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, muted: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, - name: 0, - nonce: 0, noValidate: HAS_BOOLEAN_VALUE, open: HAS_BOOLEAN_VALUE, - optimum: 0, - pattern: 0, - placeholder: 0, - poster: 0, - preload: 0, - profile: 0, - radioGroup: 0, readOnly: HAS_BOOLEAN_VALUE, - rel: 0, required: HAS_BOOLEAN_VALUE, reversed: HAS_BOOLEAN_VALUE, - role: 0, rows: HAS_POSITIVE_NUMERIC_VALUE, rowSpan: HAS_NUMERIC_VALUE, - sandbox: 0, - scope: 0, scoped: HAS_BOOLEAN_VALUE, - scrolling: 0, seamless: HAS_BOOLEAN_VALUE, selected: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, - shape: 0, size: HAS_POSITIVE_NUMERIC_VALUE, - sizes: 0, span: HAS_POSITIVE_NUMERIC_VALUE, - spellCheck: 0, - src: 0, - srcDoc: 0, - srcLang: 0, - srcSet: 0, start: HAS_NUMERIC_VALUE, - step: 0, - style: 0, - summary: 0, - tabIndex: 0, - target: 0, - title: 0, // Setting .type throws on non-<input> tags - type: 0, - useMap: 0, value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS, - width: 0, - wmode: 0, - wrap: 0, - - /** - * RDFa Properties - */ - about: 0, - datatype: 0, - inlist: 0, - prefix: 0, - // property is also supported for OpenGraph in meta tags. - property: 0, - resource: 0, - typeof: 0, - vocab: 0, - - /** - * Non-standard Properties - */ - // autoCapitalize and autoCorrect are supported in Mobile Safari for - // keyboard hints. - autoCapitalize: 0, - autoCorrect: 0, - // autoSave allows WebKit/Blink to persist values of input fields on page reloads - autoSave: 0, - // color is for Safari mask-icon link - color: 0, - // itemProp, itemScope, itemType are for - // Microdata support. See http://schema.org/docs/gs.html - itemProp: 0, itemScope: HAS_BOOLEAN_VALUE, - itemType: 0, - // itemID and itemRef are for Microdata support as well but - // only specified in the WHATWG spec document. See - // https://html.spec.whatwg.org/multipage/microdata.html#microdata-dom-api - itemID: 0, - itemRef: 0, - // results show looking glass icon and recent searches on input - // search fields in WebKit/Blink - results: 0, - // IE-only attribute that specifies security restrictions on an iframe - // as an alternative to the sandbox attribute on IE<10 - security: 0, - // IE-only attribute that controls focus behavior - unselectable: 0, }, DOMAttributeNames: { acceptCharset: 'accept-charset', diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index 808706d62b8dc..50361e92ac6b8 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -622,13 +622,11 @@ ReactDOMComponent.Mixin = { var ret = '<' + this._currentElement.type; for (var propKey in props) { - if (!props.hasOwnProperty(propKey)) { + if (!props.hasOwnProperty(propKey) || props[propKey] == null || DOMProperty.isReservedProp(propKey)) { continue; } var propValue = props[propKey]; - if (propValue == null) { - continue; - } + if (registrationNameModules.hasOwnProperty(propKey)) { if (propValue) { enqueuePutListener(this, propKey, propValue, transaction); @@ -841,7 +839,8 @@ ReactDOMComponent.Mixin = { for (propKey in lastProps) { if (nextProps.hasOwnProperty(propKey) || !lastProps.hasOwnProperty(propKey) || - lastProps[propKey] == null) { + lastProps[propKey] == null || + DOMProperty.isReservedProp(propKey)) { continue; } if (propKey === STYLE) { @@ -860,9 +859,7 @@ ReactDOMComponent.Mixin = { // listener (e.g., onClick={null}) deleteListener(this, propKey); } - } else if ( - DOMProperty.properties[propKey] || - DOMProperty.isCustomAttribute(propKey)) { + } else { DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); } } @@ -873,7 +870,8 @@ ReactDOMComponent.Mixin = { lastProps != null ? lastProps[propKey] : undefined; if (!nextProps.hasOwnProperty(propKey) || nextProp === lastProp || - nextProp == null && lastProp == null) { + nextProp == null && lastProp == null || + DOMProperty.isReservedProp(propKey)) { continue; } if (propKey === STYLE) { @@ -925,9 +923,7 @@ ReactDOMComponent.Mixin = { nextProp ); } - } else if ( - DOMProperty.properties[propKey] || - DOMProperty.isCustomAttribute(propKey)) { + } else { var node = getNode(this); // If we're updating to null or undefined, we should remove the property // from the DOM node instead of inadvertently setting to a string. This diff --git a/src/renderers/dom/shared/SVGDOMPropertyConfig.js b/src/renderers/dom/shared/SVGDOMPropertyConfig.js index 0711446e9a0cf..0ed47f4394b5e 100644 --- a/src/renderers/dom/shared/SVGDOMPropertyConfig.js +++ b/src/renderers/dom/shared/SVGDOMPropertyConfig.js @@ -35,28 +35,17 @@ var NS = { // - width var ATTRS = { accentHeight: 'accent-height', - accumulate: 0, - additive: 0, alignmentBaseline: 'alignment-baseline', allowReorder: 'allowReorder', - alphabetic: 0, - amplitude: 0, arabicForm: 'arabic-form', - ascent: 0, attributeName: 'attributeName', attributeType: 'attributeType', autoReverse: 'autoReverse', - azimuth: 0, baseFrequency: 'baseFrequency', baseProfile: 'baseProfile', baselineShift: 'baseline-shift', - bbox: 0, - begin: 0, - bias: 0, - by: 0, calcMode: 'calcMode', capHeight: 'cap-height', - clip: 0, clipPath: 'clip-path', clipRule: 'clip-rule', clipPathUnits: 'clipPathUnits', @@ -66,35 +55,17 @@ var ATTRS = { colorRendering: 'color-rendering', contentScriptType: 'contentScriptType', contentStyleType: 'contentStyleType', - cursor: 0, - cx: 0, - cy: 0, - d: 0, - decelerate: 0, - descent: 0, diffuseConstant: 'diffuseConstant', - direction: 0, - display: 0, - divisor: 0, dominantBaseline: 'dominant-baseline', - dur: 0, - dx: 0, - dy: 0, edgeMode: 'edgeMode', - elevation: 0, enableBackground: 'enable-background', - end: 0, - exponent: 0, externalResourcesRequired: 'externalResourcesRequired', - fill: 0, fillOpacity: 'fill-opacity', fillRule: 'fill-rule', - filter: 0, filterRes: 'filterRes', filterUnits: 'filterUnits', floodColor: 'flood-color', floodOpacity: 'flood-opacity', - focusable: 0, fontFamily: 'font-family', fontSize: 'font-size', fontSizeAdjust: 'font-size-adjust', @@ -102,34 +73,17 @@ var ATTRS = { fontStyle: 'font-style', fontVariant: 'font-variant', fontWeight: 'font-weight', - format: 0, - from: 0, - fx: 0, - fy: 0, - g1: 0, - g2: 0, glyphName: 'glyph-name', glyphOrientationHorizontal: 'glyph-orientation-horizontal', glyphOrientationVertical: 'glyph-orientation-vertical', glyphRef: 'glyphRef', gradientTransform: 'gradientTransform', gradientUnits: 'gradientUnits', - hanging: 0, horizAdvX: 'horiz-adv-x', horizOriginX: 'horiz-origin-x', - ideographic: 0, imageRendering: 'image-rendering', - in: 0, - in2: 0, - intercept: 0, - k: 0, - k1: 0, - k2: 0, - k3: 0, - k4: 0, kernelMatrix: 'kernelMatrix', kernelUnitLength: 'kernelUnitLength', - kerning: 0, keyPoints: 'keyPoints', keySplines: 'keySplines', keyTimes: 'keyTimes', @@ -137,27 +91,15 @@ var ATTRS = { letterSpacing: 'letter-spacing', lightingColor: 'lighting-color', limitingConeAngle: 'limitingConeAngle', - local: 0, markerEnd: 'marker-end', markerMid: 'marker-mid', markerStart: 'marker-start', markerHeight: 'markerHeight', markerUnits: 'markerUnits', markerWidth: 'markerWidth', - mask: 0, maskContentUnits: 'maskContentUnits', maskUnits: 'maskUnits', - mathematical: 0, - mode: 0, numOctaves: 'numOctaves', - offset: 0, - opacity: 0, - operator: 0, - order: 0, - orient: 0, - orientation: 0, - origin: 0, - overflow: 0, overlinePosition: 'overline-position', overlineThickness: 'overline-thickness', paintOrder: 'paint-order', @@ -167,15 +109,12 @@ var ATTRS = { patternTransform: 'patternTransform', patternUnits: 'patternUnits', pointerEvents: 'pointer-events', - points: 0, pointsAtX: 'pointsAtX', pointsAtY: 'pointsAtY', pointsAtZ: 'pointsAtZ', preserveAlpha: 'preserveAlpha', preserveAspectRatio: 'preserveAspectRatio', primitiveUnits: 'primitiveUnits', - r: 0, - radius: 0, refX: 'refX', refY: 'refY', renderingIntent: 'rendering-intent', @@ -183,31 +122,17 @@ var ATTRS = { repeatDur: 'repeatDur', requiredExtensions: 'requiredExtensions', requiredFeatures: 'requiredFeatures', - restart: 0, - result: 0, - rotate: 0, - rx: 0, - ry: 0, - scale: 0, - seed: 0, shapeRendering: 'shape-rendering', - slope: 0, - spacing: 0, specularConstant: 'specularConstant', specularExponent: 'specularExponent', - speed: 0, spreadMethod: 'spreadMethod', startOffset: 'startOffset', stdDeviation: 'stdDeviation', - stemh: 0, - stemv: 0, stitchTiles: 'stitchTiles', stopColor: 'stop-color', stopOpacity: 'stop-opacity', strikethroughPosition: 'strikethrough-position', strikethroughThickness: 'strikethrough-thickness', - string: 0, - stroke: 0, strokeDasharray: 'stroke-dasharray', strokeDashoffset: 'stroke-dashoffset', strokeLinecap: 'stroke-linecap', @@ -224,13 +149,8 @@ var ATTRS = { textDecoration: 'text-decoration', textRendering: 'text-rendering', textLength: 'textLength', - to: 0, - transform: 0, - u1: 0, - u2: 0, underlinePosition: 'underline-position', underlineThickness: 'underline-thickness', - unicode: 0, unicodeBidi: 'unicode-bidi', unicodeRange: 'unicode-range', unitsPerEm: 'units-per-em', @@ -238,22 +158,15 @@ var ATTRS = { vHanging: 'v-hanging', vIdeographic: 'v-ideographic', vMathematical: 'v-mathematical', - values: 0, vectorEffect: 'vector-effect', - version: 0, vertAdvY: 'vert-adv-y', vertOriginX: 'vert-origin-x', vertOriginY: 'vert-origin-y', viewBox: 'viewBox', viewTarget: 'viewTarget', - visibility: 0, - widths: 0, wordSpacing: 'word-spacing', writingMode: 'writing-mode', - x: 0, xHeight: 'x-height', - x1: 0, - x2: 0, xChannelSelector: 'xChannelSelector', xlinkActuate: 'xlink:actuate', xlinkArcrole: 'xlink:arcrole', @@ -265,11 +178,7 @@ var ATTRS = { xmlBase: 'xml:base', xmlLang: 'xml:lang', xmlSpace: 'xml:space', - y: 0, - y1: 0, - y2: 0, yChannelSelector: 'yChannelSelector', - z: 0, zoomAndPan: 'zoomAndPan', }; diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index 14b72282ae26e..6ca79673aba92 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -394,13 +394,13 @@ describe('DOMPropertyOperations', function() { expect(DOMPropertyOperations.createMarkupForProperty( 'foobar', 'simple' - )).toBe(null); + )).toBe('foobar="simple"'); // foo-* does not exist yet expect(DOMPropertyOperations.createMarkupForProperty( 'foo-xyz', 'simple' - )).toBe(null); + )).toBe('foo-xyz="simple"'); // inject foobar DOM property DOMProperty.injection.injectDOMPropertyConfig({ diff --git a/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js b/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js index 5adc359063e1e..9d1ec68c94d2a 100644 --- a/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js +++ b/src/renderers/dom/shared/devtools/ReactDOMUnknownPropertyDevtool.js @@ -17,20 +17,31 @@ var EventPluginRegistry = require('EventPluginRegistry'); var warning = require('warning'); if (__DEV__) { - var reactProps = { - children: true, - dangerouslySetInnerHTML: true, - key: true, - ref: true, - }; + var additionalProps = [ + // Case sensitive properties not included in properties list + 'acceptCharset', 'accessKey', 'allowTransparency', 'autoComplete', + 'autoFocus', 'cellPadding', 'cellSpacing', 'charSet', 'classID', + 'colSpan', 'contentEditable', 'contextMenu', 'crossOrigin', + 'dateTime', 'encType', 'formAction', 'formEncType', 'formMethod', + 'formTarget', 'frameBorder', 'hrefLang', 'htmlFor', 'httpEquiv', + 'inputMode', 'keyParam', 'keyType', 'marginHeight', 'marginWidth', + 'maxLength', 'mediaGroup', 'minLength', 'radioGroup', + 'spellCheck', 'srcDoc', 'srcLang', 'srcSet', 'tabIndex', 'useMap', + 'autoCapitalize', 'autoCorrect', 'autoSave', 'itemProp', + 'itemType', 'itemID', 'itemRef' + ]; + + additionalProps.forEach(function(name) { + DOMProperty.getPossibleStandardName[name.toLowerCase()] = name; + }); + var warnedProperties = {}; var warnUnknownProperty = function(name) { - if (DOMProperty.properties.hasOwnProperty(name) || DOMProperty.isCustomAttribute(name)) { + if (DOMProperty.isReservedProp(name) || DOMProperty.properties.hasOwnProperty(name)) { return; } - if (reactProps.hasOwnProperty(name) && reactProps[name] || - warnedProperties.hasOwnProperty(name) && warnedProperties[name]) { + if (warnedProperties.hasOwnProperty(name) && warnedProperties[name]) { return; } @@ -38,13 +49,8 @@ if (__DEV__) { var lowerCasedName = name.toLowerCase(); // data-* attributes should be lowercase; suggest the lowercase version - var standardName = ( - DOMProperty.isCustomAttribute(lowerCasedName) ? - lowerCasedName : - DOMProperty.getPossibleStandardName.hasOwnProperty(lowerCasedName) ? - DOMProperty.getPossibleStandardName[lowerCasedName] : - null - ); + var standardName = DOMProperty.getPossibleStandardName.hasOwnProperty(name) ? + DOMProperty.getPossibleStandardName[name] : null; // For now, only warn when we have a suggested correction. This prevents // logging too much when using transferPropsTo.