Support arbitrary attributes on elements with dashes in the tag name.
Jim committed
Feb 9, 2015 at 15:02 UTC
b1db817dc54eb37e05e5dc1b8586efa8e93b0dac
5 files changed
+146
-7
src/renderers/dom/client/ReactDOMIDOperations.js
+18
@@ -66,6 +66,24 @@ var ReactDOMIDOperations = {
66
}
67
},
68
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
+
87
/**
88
* Updates a DOM node to remove a property. This should only be used to remove
89
* DOM properties in `DOMProperty`.
src/renderers/dom/shared/DOMPropertyOperations.js
+51
-5
@@ -17,6 +17,31 @@ var DOMProperty = require('DOMProperty');
17
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
18
var warning = require('warning');
19
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
+
45
function shouldIgnoreValue(name, value) {
46
return value == null ||
47
(DOMProperty.hasBooleanValue[name] && !value) ||
@@ -110,6 +135,20 @@ var DOMPropertyOperations = {
135
return null;
136
},
137
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
+
152
/**
153
* Sets the value for a property on a node.
154
*
@@ -147,16 +186,23 @@ var DOMPropertyOperations = {
186
}
187
}
188
} 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);
190
} else if (__DEV__) {
191
warnUnknownProperty(name);
192
}
193
},
194
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
+
206
/**
207
* Deletes the value for a property on a node.
208
*
src/renderers/dom/shared/ReactDOMComponent.js
+12
-2
@@ -333,8 +333,12 @@ ReactDOMComponent.Mixin = {
333
}
334
propValue = CSSPropertyOperations.createMarkupForStyles(propValue);
335
}
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
+ }
342
if (markup) {
343
ret += ' ' + markup;
344
}
@@ -535,6 +539,12 @@ ReactDOMComponent.Mixin = {
539
} else if (lastProp) {
540
deleteListener(this._rootNodeID, propKey);
541
}
542
+ } else if (this._tag.indexOf('-') >= 0) {
543
+ BackendIDOperations.updateAttributeByID(
544
+ this._rootNodeID,
545
+ propKey,
546
+ nextProp
547
+ );
548
} else if (
549
DOMProperty.isStandardName[propKey] ||
550
DOMProperty.isCustomAttribute(propKey)) {
src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js
+15
@@ -179,6 +179,21 @@ describe('DOMPropertyOperations', function() {
179
180
});
181
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
+
197
describe('setValueForProperty', function() {
198
var stubNode;
199
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+50
@@ -21,6 +21,7 @@ describe('ReactDOMComponent', function() {
21
var ReactTestUtils;
22
23
beforeEach(function() {
24
+ require('mock-modules').dumpCache();
25
React = require('React');
26
ReactTestUtils = require('ReactTestUtils');
27
});
@@ -203,6 +204,55 @@ describe('ReactDOMComponent', function() {
204
expect(stubStyle.color).toEqual('green');
205
});
206
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
+
256
it('should clear all the styles when removing `style`', function() {
257
var styles = {display: 'none', color: 'red'};
258
var container = document.createElement('div');