Skip to content

Commit 89f9c44

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

File tree

5 files changed

+134
-3
lines changed

5 files changed

+134
-3
lines changed

src/browser/ui/ReactDOMComponent.js

Lines changed: 29 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,18 @@ function validateDangerousTag(tag) {
151154
}
152155
}
153156

157+
function validateDangerousAttributeName(attributeName) {
158+
if (!hasOwnProperty.call(validatedAttributeNameCache, attributeName)) {
159+
invariant(
160+
VALID_ATTRIBUTENAME_REGEX.test(attributeName),
161+
'Invalid attribute name: `%s`',
162+
attributeName
163+
);
164+
validatedAttributeNameCache[attributeName] = true;
165+
}
166+
return attributeName;
167+
}
168+
154169
/**
155170
* Creates a new React class that is idempotent and capable of containing other
156171
* React components. It accepts event listeners and DOM properties that are
@@ -234,8 +249,13 @@ ReactDOMComponent.Mixin = {
234249
}
235250
propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
236251
}
237-
var markup =
238-
DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
252+
var markup = null;
253+
if (this._tag != null && this._tag.indexOf('-') >= 0) {
254+
validateDangerousAttributeName(propKey);
255+
markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
256+
} else {
257+
markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
258+
}
239259
if (markup) {
240260
ret += ' ' + markup;
241261
}
@@ -398,6 +418,12 @@ ReactDOMComponent.Mixin = {
398418
}
399419
} else if (registrationNameModules.hasOwnProperty(propKey)) {
400420
putListener(this._rootNodeID, propKey, nextProp, transaction);
421+
} else if (this._tag.indexOf('-') >= 0) {
422+
BackendIDOperations.updateAttributeByID(
423+
this._rootNodeID,
424+
propKey,
425+
nextProp
426+
);
401427
} else if (
402428
DOMProperty.isStandardName[propKey] ||
403429
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: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,54 @@ 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+
'Invariant Violation: Invalid attribute name: ' +
155+
'`blah" onclick="beevil" noise="hi`'
156+
);
157+
});
158+
159+
it('should reject haxors on update', function() {
160+
var container = document.createElement('div');
161+
162+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
163+
React.render(beforeUpdate, container);
164+
165+
expect(function() {
166+
var afterUpdate = React.createElement(
167+
'x-foo-component',
168+
{'blah" onclick="beevil" noise="hi': 'selected'},
169+
null
170+
);
171+
React.render(afterUpdate, container);
172+
}).toThrow();
173+
});
174+
175+
it('should update arbitrary attributes for tags containnig dashes', function() {
176+
var container = document.createElement('div');
177+
178+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
179+
React.render(beforeUpdate, container);
180+
181+
var afterUpdate = React.createElement(
182+
'x-foo-component',
183+
{'myattr': 'myval'},
184+
null
185+
);
186+
React.render(afterUpdate, container);
187+
188+
expect(container.childNodes[0].getAttribute('myattr')).toBe('myval');
189+
});
190+
143191
it("should clear all the styles when removing 'style'", function() {
144192
var styles = {display: 'none', color: 'red'};
145193
var container = document.createElement('div');

src/browser/ui/dom/DOMPropertyOperations.js

Lines changed: 16 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,21 @@ 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+
if (value == null) {
125+
return '';
126+
}
127+
return name + '=' + quoteAttributeValueForBrowser(value);
128+
},
129+
114130
/**
115131
* Sets the value for a property on a node.
116132
*

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)