diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index bf3b4b135ef02..b6eec49689c26 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -139,9 +139,9 @@ 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 hasOwnProperty = {}.hasOwnProperty; function validateDangerousTag(tag) { @@ -195,7 +195,7 @@ ReactDOMComponent.Mixin = { assertValidProps(this._currentElement.props); var closeTag = omittedCloseTags[this._tag] ? '' : ''; return ( - this._createOpenTagMarkupAndPutListeners(transaction) + + this._createOpenTagMarkupAndPutListeners(transaction) + this._createContentMarkup(transaction, context) + closeTag ); @@ -235,7 +235,7 @@ ReactDOMComponent.Mixin = { propValue = CSSPropertyOperations.createMarkupForStyles(propValue); } var markup = - DOMPropertyOperations.createMarkupForProperty(propKey, propValue); + DOMPropertyOperations.createMarkupForProperty(this._tag, propKey, propValue); if (markup) { ret += ' ' + markup; } diff --git a/src/browser/ui/dom/DOMPropertyOperations.js b/src/browser/ui/dom/DOMPropertyOperations.js index f94703ea50282..1b687b91d820f 100644 --- a/src/browser/ui/dom/DOMPropertyOperations.js +++ b/src/browser/ui/dom/DOMPropertyOperations.js @@ -15,9 +15,17 @@ var DOMProperty = require('DOMProperty'); var escapeTextContentForBrowser = require('escapeTextContentForBrowser'); +var invariant = require('invariant'); var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser'); var warning = require('warning'); +// We accept any attribute name to be rendered but since this gets injected into abitrary +// HTML, we want to make sure that it's a safe attribute. +// Validate attribute names. We (temporarily) disallow colon until we decide the +// desired namespace semantics. For attribute name rules: http://www.w3.org/TR/xml/#NT-Name +var VALID_ATTRIBUTENAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; // Simplified subset +var validatedAttributeNameCache = {}; + function shouldIgnoreValue(name, value) { return value == null || (DOMProperty.hasBooleanValue[name] && !value) || @@ -26,6 +34,16 @@ function shouldIgnoreValue(name, value) { (DOMProperty.hasOverloadedBooleanValue[name] && value === false); } +function validateDangerousAttributeName(attributeName) { + if (!hasOwnProperty.call(validatedAttributeNameCache, attributeName)) { + invariant(VALID_ATTRIBUTENAME_REGEX.test(attributeName), + 'Invalid attribute name: `%s`', + attributeName); + validatedAttributeNameCache[attributeName] = true; + } + return attributeName; +} + if (__DEV__) { var reactProps = { children: true, @@ -84,11 +102,12 @@ var DOMPropertyOperations = { /** * Creates markup for a property. * + * @param {string} tagName * @param {string} name * @param {*} value * @return {?string} Markup string, or null if the property was invalid. */ - createMarkupForProperty: function(name, value) { + createMarkupForProperty: function(tagName, name, value) { if (DOMProperty.isStandardName.hasOwnProperty(name) && DOMProperty.isStandardName[name]) { if (shouldIgnoreValue(name, value)) { @@ -104,7 +123,12 @@ var DOMPropertyOperations = { if (value == null) { return ''; } - return name + '=' + quoteAttributeValueForBrowser(value); + return validateDangerousAttributeName(name) + '=' + quoteAttributeValueForBrowser(value); + } else if (tagName != null && tagName.indexOf('-') >= 0) { + if (value == null) { + return ''; + } + return validateDangerousAttributeName(name) + '=' + quoteAttributeValueForBrowser(value); } else if (__DEV__) { warnUnknownProperty(name); } @@ -119,7 +143,14 @@ var DOMPropertyOperations = { * @param {*} value */ setValueForProperty: function(node, name, value) { - if (DOMProperty.isStandardName.hasOwnProperty(name) && + if (node.tagName.indexOf('-') >= 0) { + if (value == null) { + node.removeAttribute(name); + } else { + node.setAttribute(name, '' + value); + } + } + else if (DOMProperty.isStandardName.hasOwnProperty(name) && DOMProperty.isStandardName[name]) { var mutationMethod = DOMProperty.getMutationMethod[name]; if (mutationMethod) { diff --git a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js index 8ff6c1aaaf0c4..80d00811de3ce 100644 --- a/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js @@ -32,16 +32,19 @@ describe('DOMPropertyOperations', function() { it('should create markup for simple properties', function() { expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'name', 'simple' )).toBe('name="simple"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'name', false )).toBe('name="false"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'name', null )).toBe(''); @@ -49,6 +52,7 @@ describe('DOMPropertyOperations', function() { it('should work with the id attribute', function() { expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'id', 'simple' )).toBe('id="simple"'); @@ -57,6 +61,7 @@ describe('DOMPropertyOperations', function() { it('should warn about incorrect casing', function() { spyOn(console, 'warn'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'tabindex', '1' )).toBe(null); @@ -67,6 +72,7 @@ describe('DOMPropertyOperations', function() { it('should warn about class', function() { spyOn(console, 'warn'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'class', 'muffins' )).toBe(null); @@ -76,16 +82,19 @@ describe('DOMPropertyOperations', function() { it('should create markup for boolean properties', function() { expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'checked', 'simple' )).toBe('checked'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'checked', true )).toBe('checked'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'checked', false )).toBe(''); @@ -93,41 +102,49 @@ describe('DOMPropertyOperations', function() { it('should create markup for booleanish properties', function() { expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', 'simple' )).toBe('download="simple"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', true )).toBe('download'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', 'true' )).toBe('download="true"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', false )).toBe(''); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', 'false' )).toBe('download="false"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', undefined )).toBe(''); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', null )).toBe(''); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'download', 0 )).toBe('download="0"'); @@ -135,16 +152,19 @@ describe('DOMPropertyOperations', function() { it('should create markup for custom attributes', function() { expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'aria-label', 'simple' )).toBe('aria-label="simple"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'aria-label', false )).toBe('aria-label="false"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'aria-label', null )).toBe(''); @@ -152,26 +172,54 @@ describe('DOMPropertyOperations', function() { it('should create markup for numeric properties', function() { expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'start', 5 )).toBe('start="5"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'start', 0 )).toBe('start="0"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'size', 0 )).toBe(''); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'size', 1 )).toBe('size="1"'); }); + it('should allow custom properties on web components', function() { + expect(DOMPropertyOperations.createMarkupForProperty( + 'x-my-webcomponent', + 'awesomeness', + 5 + )).toBe('awesomeness="5"'); + + expect(DOMPropertyOperations.createMarkupForProperty( + 'x-my-webcomponent', + 'dev', + 'jim' + )).toBe('dev="jim"'); + }); + + it('should reject haxors', function() { + expect(function(){ + DOMPropertyOperations.createMarkupForProperty( + 'x-my-webcomponent', + 'blah" onclick="beevil" noise="hi', + 5 + ); + }).toThrow('Invariant Violation: Invalid attribute name: `blah" onclick="beevil" noise="hi`'); + }); + }); describe('setValueForProperty', function() { @@ -289,19 +337,50 @@ describe('DOMPropertyOperations', function() { // some browsers) expect(stubNode.className).toBe(''); }); + + it('should work with tags containing dashes', function() { + var element = document.createElement('x-foo-component'); + expect(element.getAttribute('foobar')).toBe(null); + DOMPropertyOperations.setValueForProperty( + element, + 'foobar', + 'selected' + ); + expect(element.getAttribute('foobar')).toBe('selected'); + DOMPropertyOperations.setValueForProperty( + element, + 'foobar', + null + ); + expect(element.getAttribute('foobar')).toBe(null); + }); + + it('should reject haxors', function() { + var element = document.createElement('x-foo-component'); + + expect(function(){ + DOMPropertyOperations.setValueForProperty( + element, + 'blah" onclick="beevil" noise="hi', + 'selected' + ); + }).toThrow(); + }); }); describe('injectDOMPropertyConfig', function() { it('should support custom attributes', function() { // foobar does not exist yet expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'foobar', 'simple' )).toBe(null); // foo-* does not exist yet expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'foo-xyz', 'simple' )).toBe(null); @@ -316,22 +395,26 @@ describe('DOMPropertyOperations', function() { // Ensure old attributes still work expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'name', 'simple' )).toBe('name="simple"'); expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'data-name', 'simple' )).toBe('data-name="simple"'); // foobar should work expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'foobar', 'simple' )).toBe('foobar="simple"'); // foo-* should work expect(DOMPropertyOperations.createMarkupForProperty( + 'div', 'foo-xyz', 'simple' )).toBe('foo-xyz="simple"');