From d70f53c215d6cfa70ea810edfa516c229c317988 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. --- src/browser/ui/ReactDOMComponent.js | 30 +++++++++++-- src/browser/ui/ReactDOMIDOperations.js | 26 +++++++++++ .../ui/__tests__/ReactDOMComponent-test.js | 45 +++++++++++++++++++ src/browser/ui/dom/DOMPropertyOperations.js | 17 +++++++ .../__tests__/DOMPropertyOperations-test.js | 15 +++++++ 5 files changed, 130 insertions(+), 3 deletions(-) diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index bf3b4b135ef02..c49ff0357b465 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -139,9 +139,12 @@ var omittedCloseTags = { // We accept any tag to be rendered but since this gets injected into abitrary // HTML, we want to make sure that it's a safe tag. // http://www.w3.org/TR/REC-xml/#NT-Name - var VALID_TAG_REGEX = /^[a-zA-Z][a-zA-Z:_\.\-\d]*$/; // Simplified subset var validatedTagCache = {}; + +var VALID_ATTRIBUTENAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; // Simplified subset +var validatedAttributeNameCache = {}; + var hasOwnProperty = {}.hasOwnProperty; function validateDangerousTag(tag) { @@ -151,6 +154,16 @@ function validateDangerousTag(tag) { } } +function validateDangerousAttributeName(attributeName) { + if (!hasOwnProperty.call(validatedAttributeNameCache, attributeName)) { + invariant(VALID_ATTRIBUTENAME_REGEX.test(attributeName), + 'Invalid attribute name: `%s`', + attributeName); + validatedAttributeNameCache[attributeName] = true; + } + return attributeName; +} + /** * Creates a new React class that is idempotent and capable of containing other * React components. It accepts event listeners and DOM properties that are @@ -234,8 +247,13 @@ ReactDOMComponent.Mixin = { } propValue = CSSPropertyOperations.createMarkupForStyles(propValue); } - var markup = - DOMPropertyOperations.createMarkupForProperty(propKey, propValue); + var markup = null; + if (this._tag != null && this._tag.indexOf('-') >= 0) { + validateDangerousAttributeName(propKey); + markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue); + } else { + markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue); + } if (markup) { ret += ' ' + markup; } @@ -398,6 +416,12 @@ ReactDOMComponent.Mixin = { } } else if (registrationNameModules.hasOwnProperty(propKey)) { putListener(this._rootNodeID, propKey, nextProp, transaction); + } 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 2676b00aca551..9cdbf36858a7d 100644 --- a/src/browser/ui/ReactDOMIDOperations.js +++ b/src/browser/ui/ReactDOMIDOperations.js @@ -68,6 +68,32 @@ 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] + ); + + // If we're updating to null or undefined, we should remove the property + // from the DOM node instead of inadvertantly setting to a string. This + // brings us in line with the same behavior we have on initial render. + if (value == null) { + node.removeAttribute(name); + } else { + node.setAttribute(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 48e71ce40c5a0..f4f5704f1ac17 100644 --- a/src/browser/ui/__tests__/ReactDOMComponent-test.js +++ b/src/browser/ui/__tests__/ReactDOMComponent-test.js @@ -140,6 +140,51 @@ describe('ReactDOMComponent', function() { expect(stubStyle.color).toEqual('green'); }); + it('should reject haxors on initial markup', function() { + var container = document.createElement('div'); + + expect(function() { + var element = React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null + ); + React.render(element, container); + }).toThrow(); + }); + + it('should reject haxors on update', function() { + var container = document.createElement('div'); + + var beforeUpdate = React.createElement('x-foo-component', {}, null); + React.render(beforeUpdate, container); + + expect(function() { + var afterUpdate = React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null + ); + React.render(afterUpdate, container); + }).toThrow(); + }); + + 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 f94703ea50282..b7bfb207b9685 100644 --- a/src/browser/ui/dom/DOMPropertyOperations.js +++ b/src/browser/ui/dom/DOMPropertyOperations.js @@ -15,6 +15,7 @@ var DOMProperty = require('DOMProperty'); var escapeTextContentForBrowser = require('escapeTextContentForBrowser'); +var invariant = require('invariant'); var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser'); var warning = require('warning'); @@ -111,6 +112,22 @@ var DOMPropertyOperations = { return null; }, + /** + * Creates markup for a custom property. + * + * @param {string} tagName + * @param {string} name + * @param {*} value + * @return {?string} Markup string, or null if the property was invalid. + */ + createMarkupForCustomAttribute: function(name, value) + { + if (value == null) { + return ''; + } + return name + '=' + quoteAttributeValueForBrowser(value); + }, + /** * Sets 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 8ff6c1aaaf0c4..b99abf21bfd93 100644 --- a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js @@ -174,6 +174,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;