diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index fbeee7e8fe62c..e08bef2dafd02 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -332,8 +332,12 @@ ReactDOMComponent.Mixin = { } propValue = CSSPropertyOperations.createMarkupForStyles(propValue); } - var markup = - DOMPropertyOperations.createMarkupForProperty(propKey, propValue); + var markup = null; + if (this._tag != null && this._tag.indexOf('-') >= 0) { + markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue); + } else { + markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue); + } if (markup) { ret += ' ' + markup; } @@ -534,6 +538,12 @@ ReactDOMComponent.Mixin = { } else if (lastProp) { deleteListener(this._rootNodeID, propKey); } + } else if (this._tag.indexOf('-') >= 0) { + BackendIDOperations.updateAttributeByID( + this._rootNodeID, + propKey, + nextProp + ); } else if ( DOMProperty.isStandardName[propKey] || DOMProperty.isCustomAttribute(propKey)) { diff --git a/src/browser/ui/ReactDOMIDOperations.js b/src/browser/ui/ReactDOMIDOperations.js index d7b5e19e7523d..a377e143924d3 100644 --- a/src/browser/ui/ReactDOMIDOperations.js +++ b/src/browser/ui/ReactDOMIDOperations.js @@ -68,6 +68,24 @@ var ReactDOMIDOperations = { } }, + /** + * Updates a DOM node with new property values. + * + * @param {string} id ID of the node to update. + * @param {string} name A valid property name. + * @param {*} value New value of the property. + * @internal + */ + updateAttributeByID: function(id, name, value) { + var node = ReactMount.getNode(id); + invariant( + !INVALID_PROPERTY_ERRORS.hasOwnProperty(name), + 'updatePropertyByID(...): %s', + INVALID_PROPERTY_ERRORS[name] + ); + DOMPropertyOperations.setValueForAttribute(node, name, value); + }, + /** * Updates a DOM node to remove a property. This should only be used to remove * DOM properties in `DOMProperty`. diff --git a/src/browser/ui/__tests__/ReactDOMComponent-test.js b/src/browser/ui/__tests__/ReactDOMComponent-test.js index e8c776a4803b3..598a213d94e24 100644 --- a/src/browser/ui/__tests__/ReactDOMComponent-test.js +++ b/src/browser/ui/__tests__/ReactDOMComponent-test.js @@ -23,6 +23,7 @@ describe('ReactDOMComponent', function() { var ReactTestUtils; beforeEach(function() { + require('mock-modules').dumpCache(); React = require('React'); ReactTestUtils = require('ReactTestUtils'); }); @@ -205,6 +206,61 @@ describe('ReactDOMComponent', function() { expect(stubStyle.color).toEqual('green'); }); + it('should reject haxors on initial markup', function() { + spyOn(console, 'error'); + for (var i = 0; i < 3; i++) + { + var container = document.createElement('div'); + var element = React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null + ); + React.render(element, container); + } + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toEqual( + 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`' + ); + }); + + it('should reject haxors on update', function() { + spyOn(console, 'error'); + for (var i = 0; i < 3; i++) + { + var container = document.createElement('div'); + var beforeUpdate = React.createElement('x-foo-component', {}, null); + React.render(beforeUpdate, container); + + var afterUpdate = React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null + ); + React.render(afterUpdate, container); + } + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toEqual( + 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`' + ); + }); + + it('should update arbitrary attributes for tags containnig dashes', function() { + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('x-foo-component', {}, null); + React.render(beforeUpdate, container); + + var afterUpdate = React.createElement( + 'x-foo-component', + {'myattr': 'myval'}, + null + ); + React.render(afterUpdate, container); + + expect(container.childNodes[0].getAttribute('myattr')).toBe('myval'); + }); + it("should clear all the styles when removing 'style'", function() { var styles = {display: 'none', color: 'red'}; var container = document.createElement('div'); diff --git a/src/browser/ui/dom/DOMPropertyOperations.js b/src/browser/ui/dom/DOMPropertyOperations.js index f049403184cba..d535cf2c87041 100644 --- a/src/browser/ui/dom/DOMPropertyOperations.js +++ b/src/browser/ui/dom/DOMPropertyOperations.js @@ -17,6 +17,30 @@ var DOMProperty = require('DOMProperty'); var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser'); var warning = require('warning'); +var VALID_ATTRIBUTE_NAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; // Simplified subset +var illegalAttributeNameCache = {}; +var validatedAttributeNameCache = {}; + +function isAttributeNameSafe(attributeName) { + if (validatedAttributeNameCache.hasOwnProperty(attributeName)) { + return true; + } + if (illegalAttributeNameCache.hasOwnProperty(attributeName)) { + return false; + } + if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) { + validatedAttributeNameCache[attributeName] = true; + return true; + } + illegalAttributeNameCache[attributeName] = true; + warning( + false, + 'Invalid attribute name: `%s`', + attributeName + ); + return false; +} + function shouldIgnoreValue(name, value) { return value == null || (DOMProperty.hasBooleanValue[name] && !value) || @@ -110,6 +134,23 @@ var DOMPropertyOperations = { return null; }, + /** + * Creates markup for a custom property. + * + * @param {string} name + * @param {*} value + * @return {?string} Markup string, or null if the property was invalid. + */ + createMarkupForCustomAttribute: function(name, value) { + if (!isAttributeNameSafe(name)) { + return ''; + } + if (value == null) { + return ''; + } + return name + '=' + quoteAttributeValueForBrowser(value); + }, + /** * Sets the value for a property on a node. * @@ -141,16 +182,23 @@ var DOMPropertyOperations = { } } } else if (DOMProperty.isCustomAttribute(name)) { - if (value == null) { - node.removeAttribute(name); - } else { - node.setAttribute(name, '' + value); - } + DOMPropertyOperations.setValueForAttribute(node, name, value); } else if (__DEV__) { warnUnknownProperty(name); } }, + setValueForAttribute: function(node, name, value) { + if (!isAttributeNameSafe(name)) { + return; + } + if (value == null) { + node.removeAttribute(name); + } else { + node.setAttribute(name, '' + value); + } + }, + /** * Deletes the value for a property on a node. * diff --git a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js index 8cadb40d1f9bd..48093c06ea5d6 100644 --- a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js @@ -179,6 +179,21 @@ describe('DOMPropertyOperations', function() { }); + describe('createMarkupForProperty', function() { + + it('should allow custom properties on web components', function() { + expect(DOMPropertyOperations.createMarkupForCustomAttribute( + 'awesomeness', + 5 + )).toBe('awesomeness="5"'); + + expect(DOMPropertyOperations.createMarkupForCustomAttribute( + 'dev', + 'jim' + )).toBe('dev="jim"'); + }); + }); + describe('setValueForProperty', function() { var stubNode;