From b1db817dc54eb37e05e5dc1b8586efa8e93b0dac Mon Sep 17 00:00:00 2001 From: Jim Date: Mon, 9 Feb 2015 15:02:40 -0800 Subject: [PATCH] Support arbitrary attributes on elements with dashes in the tag name. --- .../dom/client/ReactDOMIDOperations.js | 18 ++++++ .../dom/shared/DOMPropertyOperations.js | 56 +++++++++++++++++-- src/renderers/dom/shared/ReactDOMComponent.js | 14 ++++- .../__tests__/DOMPropertyOperations-test.js | 15 +++++ .../__tests__/ReactDOMComponent-test.js | 50 +++++++++++++++++ 5 files changed, 146 insertions(+), 7 deletions(-) diff --git a/src/renderers/dom/client/ReactDOMIDOperations.js b/src/renderers/dom/client/ReactDOMIDOperations.js index cd29a50742d88..34a15afe345e2 100644 --- a/src/renderers/dom/client/ReactDOMIDOperations.js +++ b/src/renderers/dom/client/ReactDOMIDOperations.js @@ -66,6 +66,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/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index b25f62c01dd0f..0c2e1eb3ed337 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -17,6 +17,31 @@ var DOMProperty = require('DOMProperty'); var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser'); var warning = require('warning'); +// Simplified subset +var VALID_ATTRIBUTE_NAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; +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 +135,20 @@ var DOMPropertyOperations = { return null; }, + /** + * Creates markup for a custom property. + * + * @param {string} name + * @param {*} value + * @return {string} Markup string, or empty string if the property was invalid. + */ + createMarkupForCustomAttribute: function(name, value) { + if (!isAttributeNameSafe(name) || value == null) { + return ''; + } + return name + '=' + quoteAttributeValueForBrowser(value); + }, + /** * Sets the value for a property on a node. * @@ -147,16 +186,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/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index a8742274f734a..e7fe7ca1b6225 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -333,8 +333,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; } @@ -535,6 +539,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/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index c9711ee77e0f5..2e5ee224d5a48 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__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; diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index dda078a441d3c..5f4e0b5f5c6dc 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -21,6 +21,7 @@ describe('ReactDOMComponent', function() { var ReactTestUtils; beforeEach(function() { + require('mock-modules').dumpCache(); React = require('React'); ReactTestUtils = require('ReactTestUtils'); }); @@ -203,6 +204,55 @@ describe('ReactDOMComponent', function() { expect(stubStyle.color).toEqual('green'); }); + it('should reject attribute key injection attack on 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 attribute key injection attack 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 containing dashes', function() { + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('x-foo-component', {}, null); + React.render(beforeUpdate, container); + + var afterUpdate = ; + 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');