Skip to content

Commit 2f442e6

Browse files
committed
Support arbitrary attributes on elements with dashes in the tag name.
1 parent c7e4f55 commit 2f442e6

File tree

5 files changed

+130
-3
lines changed

5 files changed

+130
-3
lines changed

src/browser/ui/ReactDOMComponent.js

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,12 @@ var omittedCloseTags = {
139139
// We accept any tag to be rendered but since this gets injected into abitrary
140140
// HTML, we want to make sure that it's a safe tag.
141141
// http://www.w3.org/TR/REC-xml/#NT-Name
142-
143142
var VALID_TAG_REGEX = /^[a-zA-Z][a-zA-Z:_\.\-\d]*$/; // Simplified subset
144143
var validatedTagCache = {};
144+
145+
var VALID_ATTRIBUTENAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/; // Simplified subset
146+
var validatedAttributeNameCache = {};
147+
145148
var hasOwnProperty = {}.hasOwnProperty;
146149

147150
function validateDangerousTag(tag) {
@@ -151,6 +154,16 @@ function validateDangerousTag(tag) {
151154
}
152155
}
153156

157+
function validateDangerousAttributeName(attributeName) {
158+
if (!hasOwnProperty.call(validatedAttributeNameCache, attributeName)) {
159+
invariant(VALID_ATTRIBUTENAME_REGEX.test(attributeName),
160+
'Invalid attribute name: `%s`',
161+
attributeName);
162+
validatedAttributeNameCache[attributeName] = true;
163+
}
164+
return attributeName;
165+
}
166+
154167
/**
155168
* Creates a new React class that is idempotent and capable of containing other
156169
* React components. It accepts event listeners and DOM properties that are
@@ -234,8 +247,13 @@ ReactDOMComponent.Mixin = {
234247
}
235248
propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
236249
}
237-
var markup =
238-
DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
250+
var markup = null;
251+
if(this._tag != null && this._tag.indexOf('-') >= 0) {
252+
validateDangerousAttributeName(propKey);
253+
markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
254+
} else {
255+
markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
256+
}
239257
if (markup) {
240258
ret += ' ' + markup;
241259
}
@@ -398,6 +416,12 @@ ReactDOMComponent.Mixin = {
398416
}
399417
} else if (registrationNameModules.hasOwnProperty(propKey)) {
400418
putListener(this._rootNodeID, propKey, nextProp, transaction);
419+
} else if (this._tag.indexOf('-') >= 0) {
420+
BackendIDOperations.updateAttributeByID(
421+
this._rootNodeID,
422+
propKey,
423+
nextProp
424+
);
401425
} else if (
402426
DOMProperty.isStandardName[propKey] ||
403427
DOMProperty.isCustomAttribute(propKey)) {

src/browser/ui/ReactDOMIDOperations.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,32 @@ var ReactDOMIDOperations = {
6868
}
6969
},
7070

71+
/**
72+
* Updates a DOM node with new property values.
73+
*
74+
* @param {string} id ID of the node to update.
75+
* @param {string} name A valid property name.
76+
* @param {*} value New value of the property.
77+
* @internal
78+
*/
79+
updateAttributeByID: function(id, name, value) {
80+
var node = ReactMount.getNode(id);
81+
invariant(
82+
!INVALID_PROPERTY_ERRORS.hasOwnProperty(name),
83+
'updatePropertyByID(...): %s',
84+
INVALID_PROPERTY_ERRORS[name]
85+
);
86+
87+
// If we're updating to null or undefined, we should remove the property
88+
// from the DOM node instead of inadvertantly setting to a string. This
89+
// brings us in line with the same behavior we have on initial render.
90+
if (value == null) {
91+
node.removeAttribute(name);
92+
} else {
93+
node.setAttribute(name, '' + value);
94+
}
95+
},
96+
7197
/**
7298
* Updates a DOM node to remove a property. This should only be used to remove
7399
* DOM properties in `DOMProperty`.

src/browser/ui/__tests__/ReactDOMComponent-test.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,51 @@ describe('ReactDOMComponent', function() {
140140
expect(stubStyle.color).toEqual('green');
141141
});
142142

143+
it('should reject haxors on initial markup', function() {
144+
var container = document.createElement('div');
145+
146+
expect(function() {
147+
var element = React.createElement(
148+
'x-foo-component',
149+
{'blah" onclick="beevil" noise="hi': 'selected'},
150+
null
151+
);
152+
React.render(element, container);
153+
}).toThrow();
154+
});
155+
156+
it('should reject haxors on update', function() {
157+
var container = document.createElement('div');
158+
159+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
160+
React.render(beforeUpdate, container);
161+
162+
expect(function() {
163+
var afterUpdate = React.createElement(
164+
'x-foo-component',
165+
{'blah" onclick="beevil" noise="hi': 'selected'},
166+
null
167+
);
168+
React.render(afterUpdate, container);
169+
}).toThrow();
170+
});
171+
172+
it('should update arbitrary attributes for tags containnig dashes', function() {
173+
var container = document.createElement('div');
174+
175+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
176+
React.render(beforeUpdate, container);
177+
178+
var afterUpdate = React.createElement(
179+
'x-foo-component',
180+
{'myattr': 'myval'},
181+
null
182+
);
183+
React.render(afterUpdate, container);
184+
185+
expect(container.childNodes[0].getAttribute('myattr')).toBe('myval');
186+
});
187+
143188
it("should clear all the styles when removing 'style'", function() {
144189
var styles = {display: 'none', color: 'red'};
145190
var container = document.createElement('div');

src/browser/ui/dom/DOMPropertyOperations.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
var DOMProperty = require('DOMProperty');
1616

1717
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
18+
var invariant = require('invariant');
1819
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
1920
var warning = require('warning');
2021

@@ -111,6 +112,22 @@ var DOMPropertyOperations = {
111112
return null;
112113
},
113114

115+
/**
116+
* Creates markup for a custom property.
117+
*
118+
* @param {string} tagName
119+
* @param {string} name
120+
* @param {*} value
121+
* @return {?string} Markup string, or null if the property was invalid.
122+
*/
123+
createMarkupForCustomAttribute: function(name, value)
124+
{
125+
if (value == null) {
126+
return '';
127+
}
128+
return name + '=' + quoteAttributeValueForBrowser(value);
129+
},
130+
114131
/**
115132
* Sets the value for a property on a node.
116133
*

src/browser/ui/dom/__tests__/DOMPropertyOperations-test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,21 @@ describe('DOMPropertyOperations', function() {
174174

175175
});
176176

177+
describe('createMarkupForProperty', function() {
178+
179+
it('should allow custom properties on web components', function() {
180+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
181+
'awesomeness',
182+
5
183+
)).toBe('awesomeness="5"');
184+
185+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
186+
'dev',
187+
'jim'
188+
)).toBe('dev="jim"');
189+
});
190+
});
191+
177192
describe('setValueForProperty', function() {
178193
var stubNode;
179194

0 commit comments

Comments
 (0)