Skip to content

Commit b1db817

Browse files
committed
Support arbitrary attributes on elements with dashes in the tag name.
1 parent db82ed0 commit b1db817

File tree

5 files changed

+146
-7
lines changed

5 files changed

+146
-7
lines changed

src/renderers/dom/client/ReactDOMIDOperations.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@ var ReactDOMIDOperations = {
6666
}
6767
},
6868

69+
/**
70+
* Updates a DOM node with new property values.
71+
*
72+
* @param {string} id ID of the node to update.
73+
* @param {string} name A valid property name.
74+
* @param {*} value New value of the property.
75+
* @internal
76+
*/
77+
updateAttributeByID: function(id, name, value) {
78+
var node = ReactMount.getNode(id);
79+
invariant(
80+
!INVALID_PROPERTY_ERRORS.hasOwnProperty(name),
81+
'updatePropertyByID(...): %s',
82+
INVALID_PROPERTY_ERRORS[name]
83+
);
84+
DOMPropertyOperations.setValueForAttribute(node, name, value);
85+
},
86+
6987
/**
7088
* Updates a DOM node to remove a property. This should only be used to remove
7189
* DOM properties in `DOMProperty`.

src/renderers/dom/shared/DOMPropertyOperations.js

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,31 @@ var DOMProperty = require('DOMProperty');
1717
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
1818
var warning = require('warning');
1919

20+
// Simplified subset
21+
var VALID_ATTRIBUTE_NAME_REGEX = /^[a-zA-Z_][a-zA-Z_\.\-\d]*$/;
22+
var illegalAttributeNameCache = {};
23+
var validatedAttributeNameCache = {};
24+
25+
function isAttributeNameSafe(attributeName) {
26+
if (validatedAttributeNameCache.hasOwnProperty(attributeName)) {
27+
return true;
28+
}
29+
if (illegalAttributeNameCache.hasOwnProperty(attributeName)) {
30+
return false;
31+
}
32+
if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) {
33+
validatedAttributeNameCache[attributeName] = true;
34+
return true;
35+
}
36+
illegalAttributeNameCache[attributeName] = true;
37+
warning(
38+
false,
39+
'Invalid attribute name: `%s`',
40+
attributeName
41+
);
42+
return false;
43+
}
44+
2045
function shouldIgnoreValue(name, value) {
2146
return value == null ||
2247
(DOMProperty.hasBooleanValue[name] && !value) ||
@@ -110,6 +135,20 @@ var DOMPropertyOperations = {
110135
return null;
111136
},
112137

138+
/**
139+
* Creates markup for a custom property.
140+
*
141+
* @param {string} name
142+
* @param {*} value
143+
* @return {string} Markup string, or empty string if the property was invalid.
144+
*/
145+
createMarkupForCustomAttribute: function(name, value) {
146+
if (!isAttributeNameSafe(name) || value == null) {
147+
return '';
148+
}
149+
return name + '=' + quoteAttributeValueForBrowser(value);
150+
},
151+
113152
/**
114153
* Sets the value for a property on a node.
115154
*
@@ -147,16 +186,23 @@ var DOMPropertyOperations = {
147186
}
148187
}
149188
} else if (DOMProperty.isCustomAttribute(name)) {
150-
if (value == null) {
151-
node.removeAttribute(name);
152-
} else {
153-
node.setAttribute(name, '' + value);
154-
}
189+
DOMPropertyOperations.setValueForAttribute(node, name, value);
155190
} else if (__DEV__) {
156191
warnUnknownProperty(name);
157192
}
158193
},
159194

195+
setValueForAttribute: function(node, name, value) {
196+
if (!isAttributeNameSafe(name)) {
197+
return;
198+
}
199+
if (value == null) {
200+
node.removeAttribute(name);
201+
} else {
202+
node.setAttribute(name, '' + value);
203+
}
204+
},
205+
160206
/**
161207
* Deletes the value for a property on a node.
162208
*

src/renderers/dom/shared/ReactDOMComponent.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,12 @@ ReactDOMComponent.Mixin = {
333333
}
334334
propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
335335
}
336-
var markup =
337-
DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
336+
var markup = null;
337+
if (this._tag != null && this._tag.indexOf('-') >= 0) {
338+
markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
339+
} else {
340+
markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
341+
}
338342
if (markup) {
339343
ret += ' ' + markup;
340344
}
@@ -535,6 +539,12 @@ ReactDOMComponent.Mixin = {
535539
} else if (lastProp) {
536540
deleteListener(this._rootNodeID, propKey);
537541
}
542+
} else if (this._tag.indexOf('-') >= 0) {
543+
BackendIDOperations.updateAttributeByID(
544+
this._rootNodeID,
545+
propKey,
546+
nextProp
547+
);
538548
} else if (
539549
DOMProperty.isStandardName[propKey] ||
540550
DOMProperty.isCustomAttribute(propKey)) {

src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js

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

180180
});
181181

182+
describe('createMarkupForProperty', function() {
183+
184+
it('should allow custom properties on web components', function() {
185+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
186+
'awesomeness',
187+
5
188+
)).toBe('awesomeness="5"');
189+
190+
expect(DOMPropertyOperations.createMarkupForCustomAttribute(
191+
'dev',
192+
'jim'
193+
)).toBe('dev="jim"');
194+
});
195+
});
196+
182197
describe('setValueForProperty', function() {
183198
var stubNode;
184199

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ describe('ReactDOMComponent', function() {
2121
var ReactTestUtils;
2222

2323
beforeEach(function() {
24+
require('mock-modules').dumpCache();
2425
React = require('React');
2526
ReactTestUtils = require('ReactTestUtils');
2627
});
@@ -203,6 +204,55 @@ describe('ReactDOMComponent', function() {
203204
expect(stubStyle.color).toEqual('green');
204205
});
205206

207+
it('should reject attribute key injection attack on markup', function() {
208+
spyOn(console, 'error');
209+
for (var i = 0; i < 3; i++) {
210+
var container = document.createElement('div');
211+
var element = React.createElement(
212+
'x-foo-component',
213+
{'blah" onclick="beevil" noise="hi': 'selected'},
214+
null
215+
);
216+
React.render(element, container);
217+
}
218+
expect(console.error.argsForCall.length).toBe(1);
219+
expect(console.error.argsForCall[0][0]).toEqual(
220+
'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`'
221+
);
222+
});
223+
224+
it('should reject attribute key injection attack on update', function() {
225+
spyOn(console, 'error');
226+
for (var i = 0; i < 3; i++) {
227+
var container = document.createElement('div');
228+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
229+
React.render(beforeUpdate, container);
230+
231+
var afterUpdate = React.createElement(
232+
'x-foo-component',
233+
{'blah" onclick="beevil" noise="hi': 'selected'},
234+
null
235+
);
236+
React.render(afterUpdate, container);
237+
}
238+
expect(console.error.argsForCall.length).toBe(1);
239+
expect(console.error.argsForCall[0][0]).toEqual(
240+
'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`'
241+
);
242+
});
243+
244+
it('should update arbitrary attributes for tags containing dashes', function() {
245+
var container = document.createElement('div');
246+
247+
var beforeUpdate = React.createElement('x-foo-component', {}, null);
248+
React.render(beforeUpdate, container);
249+
250+
var afterUpdate = <x-foo-component myattr="myval" />;
251+
React.render(afterUpdate, container);
252+
253+
expect(container.childNodes[0].getAttribute('myattr')).toBe('myval');
254+
});
255+
206256
it('should clear all the styles when removing `style`', function() {
207257
var styles = {display: 'none', color: 'red'};
208258
var container = document.createElement('div');

0 commit comments

Comments
 (0)