Skip to content

Commit

Permalink
Support arbitrary attributes on elements with dashes in the tag name.
Browse files Browse the repository at this point in the history
  • Loading branch information
jimfb committed Feb 10, 2015
1 parent c7e4f55 commit d70f53c
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 3 deletions.
30 changes: 27 additions & 3 deletions src/browser/ui/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)) {
Expand Down
26 changes: 26 additions & 0 deletions src/browser/ui/ReactDOMIDOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
45 changes: 45 additions & 0 deletions src/browser/ui/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
17 changes: 17 additions & 0 deletions src/browser/ui/dom/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
var DOMProperty = require('DOMProperty');

var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var invariant = require('invariant');
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
var warning = require('warning');

Expand Down Expand Up @@ -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.
*
Expand Down
15 changes: 15 additions & 0 deletions src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit d70f53c

Please sign in to comment.