Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to attribute whitelist logic #10564

Merged
merged 36 commits into from
Aug 31, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
e10694a
Remove HTMLPropertyConfig entries for non-boolean values
nhunzaker Aug 24, 2017
711bafe
Only HAS_BOOLEAN_VALUE attribute flag can assign booleans
nhunzaker Aug 29, 2017
4c5dfbb
Use a non-boolean attribute in object assignment tests
nhunzaker Aug 29, 2017
e086ff1
Add HAS_STRING_BOOLEAN_VALUE attribute flag
nhunzaker Aug 29, 2017
0410651
Fix boolean tests, add boolean warning.
nhunzaker Aug 29, 2017
ab75d9a
Reserved props should allow booleans
nhunzaker Aug 29, 2017
6077e0b
Remove outdated comments
gaearon Aug 29, 2017
6f913ed
Style tweaks
gaearon Aug 29, 2017
aca8d9c
Don't treat dashed SVG tags as custom elements
gaearon Aug 29, 2017
af7d035
SVG elements like font-face are not custom attributes
nhunzaker Aug 29, 2017
9c0751f
Move namespace check to isCustomAttribute. Add caveat for stack.
nhunzaker Aug 29, 2017
f8da44e
Remove unused namespace variable assignment
nhunzaker Aug 29, 2017
b93e093
Fix the DEV-only whitelist
gaearon Aug 29, 2017
fbcced1
Don't read property twice
gaearon Aug 29, 2017
a270e03
Ignore and warn about non-string `is` attribute
gaearon Aug 29, 2017
ed92af0
Blacklist "aria" and "data" attributes
gaearon Aug 29, 2017
5a339a1
Don't pass unknown on* attributes through
gaearon Aug 29, 2017
0b2ba65
Remove dead code
gaearon Aug 29, 2017
3f85316
Avoid accessing namespace when possible
nhunzaker Aug 29, 2017
76a6318
Drop .only in ReactDOMComponent-test
nhunzaker Aug 29, 2017
b29bb74
Make isCustomComponent logic more solid
gaearon Aug 29, 2017
0a2aec4
Do attribute name check earlier
gaearon Aug 29, 2017
501e86d
Fix fbjs import
gaearon Aug 29, 2017
8d1f487
Revert unintentional edit
gaearon Aug 29, 2017
72666fa
Re-allow "data" attribute
gaearon Aug 29, 2017
cb687ed
Use stricter check when attaching events
gaearon Aug 29, 2017
1d61379
Pass SVG boolean attributes with correct casing
gaearon Aug 29, 2017
2b0f61a
Fix the test
gaearon Aug 29, 2017
1590c2a
Undo the SVG dashed-name fix
gaearon Aug 30, 2017
cb883a6
Prettier
gaearon Aug 30, 2017
32f6321
Fix lint
gaearon Aug 30, 2017
6d9c0e0
Fix flow
gaearon Aug 30, 2017
10e0c09
Pass "aria" through but still warn
gaearon Aug 30, 2017
ba71ec1
Remove special cases for onfocusin, onfocusout
gaearon Aug 30, 2017
dc760af
Add a more specific warning for unknown events
gaearon Aug 31, 2017
10950c4
Pass badly cased React attributes through with warning
gaearon Aug 31, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Move namespace check to isCustomAttribute. Add caveat for stack.
  • Loading branch information
nhunzaker committed Aug 29, 2017
commit 9c0751fc356e7e797815e5dd18df01ee85764436
57 changes: 34 additions & 23 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ if (__DEV__) {
var ReactDOMUnknownPropertyHook = require('ReactDOMUnknownPropertyHook');
var {validateProperties: validateARIAProperties} = ReactDOMInvalidARIAHook;
var {
validateProperties: validateInputPropertes,
validateProperties: validateInputProperties,
} = ReactDOMNullInputValuePropHook;
var {
validateProperties: validateUnknownPropertes,
validateProperties: validateUnknownProperties,
} = ReactDOMUnknownPropertyHook;
}

Expand All @@ -70,10 +70,10 @@ if (__DEV__) {
time: true,
};

var validatePropertiesInDevelopment = function(type, props) {
validateARIAProperties(type, props);
validateInputPropertes(type, props);
validateUnknownPropertes(type, props);
var validatePropertiesInDevelopment = function(type, namespace, props) {
validateARIAProperties(type, props, namespace);
validateInputProperties(type, props);
validateUnknownProperties(type, props, namespace);
};

var warnForTextDifference = function(serverText: string, clientText: string) {
Expand Down Expand Up @@ -307,8 +307,7 @@ var ReactDOMFiberComponent = {
}
if (namespaceURI === HTML_NAMESPACE) {
if (__DEV__) {
var isCustomComponentTag =
isCustomComponent(type, props) && namespaceURI === HTML_NAMESPACE;
var isCustomComponentTag = isCustomComponent(type, props, namespaceURI);
// Should this check be gated by parent namespace? Not sure we want to
// allow <SVG> or <mATH>.
warning(
Expand Down Expand Up @@ -369,11 +368,13 @@ var ReactDOMFiberComponent = {
rawProps: Object,
rootContainerElement: Element | Document,
): void {
var isCustomComponentTag =
isCustomComponent(tag, rawProps) &&
domElement.namespaceURI === HTML_NAMESPACE;
var isCustomComponentTag = isCustomComponent(
tag,
rawProps,
domElement.namespaceURI,
);
if (__DEV__) {
validatePropertiesInDevelopment(tag, rawProps);
validatePropertiesInDevelopment(tag, domElement.namespaceURI, rawProps);
if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) {
warning(
false,
Expand Down Expand Up @@ -544,7 +545,11 @@ var ReactDOMFiberComponent = {
rootContainerElement: Element | Document,
): null | Array<mixed> {
if (__DEV__) {
validatePropertiesInDevelopment(tag, nextRawProps);
validatePropertiesInDevelopment(
tag,
domElement.namespaceURI,
nextRawProps,
);
}

var updatePayload: null | Array<any> = null;
Expand Down Expand Up @@ -741,12 +746,16 @@ var ReactDOMFiberComponent = {
lastRawProps: Object,
nextRawProps: Object,
): void {
var wasCustomComponentTag =
isCustomComponent(tag, lastRawProps) &&
domElement.namespaceURI === HTML_NAMESPACE;
var isCustomComponentTag =
isCustomComponent(tag, nextRawProps) &&
domElement.namespaceURI === HTML_NAMESPACE;
var wasCustomComponentTag = isCustomComponent(
tag,
lastRawProps,
domElement.namespaceURI,
);
var isCustomComponentTag = isCustomComponent(
tag,
nextRawProps,
domElement.namespaceURI,
);
// Apply the diff.
updateDOMProperties(
domElement,
Expand Down Expand Up @@ -786,10 +795,12 @@ var ReactDOMFiberComponent = {
rootContainerElement: Element | Document,
): null | Array<mixed> {
if (__DEV__) {
var isCustomComponentTag =
isCustomComponent(tag, rawProps) &&
domElement.namespaceURI === HTML_NAMESPACE;
validatePropertiesInDevelopment(tag, rawProps);
var isCustomComponentTag = isCustomComponent(
tag,
rawProps,
domElement.namespaceURI,
);
validatePropertiesInDevelopment(tag, domElement.namespaceURI, rawProps);
if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) {
warning(
false,
Expand Down
52 changes: 29 additions & 23 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2189,33 +2189,39 @@ describe('ReactDOMComponent', () => {
});
});

describe('Hyphenated SVG elements', function() {
it('the font-face element is not a custom element', function() {
spyOn(console, 'error');
var el = ReactTestUtils.renderIntoDocument(
<font-face x-height={false} />,
);
if (ReactDOMFeatureFlags.useFiber) {
describe('Hyphenated SVG elements', function() {
it('the font-face element is not a custom element', function() {
spyOn(console, 'error');
var el = ReactTestUtils.renderIntoDocument(
<svg><font-face x-height={false} /></svg>,
);

expect(el.hasAttribute('x-height')).toBe(false);
expect(el.querySelector('font-face').hasAttribute('x-height')).toBe(
false,
);

expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Invalid DOM property `x-height`. Did you mean `xHeight`',
);
});
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Invalid DOM property `x-height`. Did you mean `xHeight`',
);
});

it('the font-face element does not allow unknown boolean values', function() {
spyOn(console, 'error');
var el = ReactTestUtils.renderIntoDocument(
<font-face whatever={false} />,
);
it('the font-face element does not allow unknown boolean values', function() {
spyOn(console, 'error');
var el = ReactTestUtils.renderIntoDocument(
<svg><font-face whatever={false} /></svg>,
);

expect(el.hasAttribute('whatever')).toBe(false);
expect(el.querySelector('font-face').hasAttribute('whatever')).toBe(
false,
);

expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Received `false` for non-boolean attribute `whatever`.',
);
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'Warning: Received `false` for non-boolean attribute `whatever`.',
);
});
});
});
}
});
8 changes: 4 additions & 4 deletions src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ function warnInvalidARIAProps(type, props, debugID) {
}
}

function validateProperties(type, props, debugID /* Stack only */) {
if (isCustomComponent(type, props)) {
function validateProperties(type, props, namespace, debugID /* Stack only */) {
if (isCustomComponent(type, props, namespace)) {
return;
}
warnInvalidARIAProps(type, props, debugID);
Expand All @@ -158,12 +158,12 @@ var ReactDOMInvalidARIAHook = {
// Stack
onBeforeMountComponent(debugID, element) {
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
validateProperties(element.type, element.props, null, debugID);
}
},
onBeforeUpdateComponent(debugID, element) {
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
validateProperties(element.type, element.props, null, debugID);
}
},
};
Expand Down
9 changes: 5 additions & 4 deletions src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
var DOMProperty = require('DOMProperty');
var EventPluginRegistry = require('EventPluginRegistry');
var isCustomComponent = require('isCustomComponent');
var DOMNamespaces = require('DOMNamespaces');

if (__DEV__) {
var warning = require('fbjs/lib/warning');
Expand Down Expand Up @@ -208,8 +209,8 @@ var warnUnknownProperties = function(type, props, debugID) {
}
};

function validateProperties(type, props, debugID /* Stack only */) {
if (isCustomComponent(type, props)) {
function validateProperties(type, props, namespace, debugID /* Stack only */) {
if (isCustomComponent(type, props, namespace)) {
return;
}
warnUnknownProperties(type, props, debugID);
Expand All @@ -221,12 +222,12 @@ var ReactDOMUnknownPropertyHook = {
// Stack
onBeforeMountComponent(debugID, element) {
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
validateProperties(element.type, element.props, null, debugID);
}
},
onBeforeUpdateComponent(debugID, element) {
if (__DEV__ && element != null && typeof element.type === 'string') {
validateProperties(element.type, element.props, debugID);
validateProperties(element.type, element.props, null, debugID);
}
},
};
Expand Down
22 changes: 8 additions & 14 deletions src/renderers/dom/shared/utils/isCustomComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,17 @@

'use strict';

// https://www.w3.org/TR/SVG/eltindex.html
var DashedSVGElements = {
'color-profile': true,
'font-face': true,
'font-face-format': true,
'font-face-name': true,
'font-face-src': true,
'font-face-uri': true,
'missing-glyph': true,
};
var DOMNamespaces = require('DOMNamespaces');
var HTML_NAMESPACE = DOMNamespaces.Namespaces.html;

function isCustomComponent(tagName, props) {
if (DashedSVGElements[tagName]) {
return false;
function isCustomComponent(tagName, props, namespace) {
if (tagName.indexOf('-') >= 0 || props.is != null) {
// TODO: We always have a namespace with fiber. Drop the first
// check when Stack is removed.
return namespace == null || namespace === HTML_NAMESPACE;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaearon I moved the namespace check into this module. Stack validates properties before the namespace is assigned to the element, so we don't know if the element is SVG or HTML during initial validations. namespace == null can go away when stack is removed. This also preserves the custom element behavior for 15.x, if that matters.

}

return tagName.indexOf('-') >= 0 || props.is != null;
return false;
}

module.exports = isCustomComponent;
23 changes: 12 additions & 11 deletions src/renderers/dom/stack/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,11 @@ ReactDOMComponent.Mixin = {
namespaceURI = Namespaces.html;
}
if (__DEV__) {
var isCustomComponentTag =
isCustomComponent(this._tag, props) && namespaceURI === Namespaces.html;
var isCustomComponentTag = isCustomComponent(
this._tag,
props,
namespaceURI,
);
}
if (namespaceURI === Namespaces.html) {
if (__DEV__) {
Expand Down Expand Up @@ -699,8 +702,7 @@ ReactDOMComponent.Mixin = {
var markup = null;
if (
this._tag != null &&
isCustomComponent(this._tag, props) &&
this._namespaceURI === Namespaces.html
isCustomComponent(this._tag, props, this._namespaceURI)
) {
if (!DOMProperty.isReservedProp(propKey)) {
markup = DOMMarkupOperations.createMarkupForCustomAttribute(
Expand Down Expand Up @@ -884,9 +886,11 @@ ReactDOMComponent.Mixin = {
}

assertValidProps(this, nextProps);
var isCustomComponentTag =
isCustomComponent(this._tag, nextProps) &&
this._namespaceURI === Namespaces.html;
var isCustomComponentTag = isCustomComponent(
this._tag,
nextProps,
this._namespaceURI,
);
this._updateDOMProperties(
lastProps,
nextProps,
Expand Down Expand Up @@ -960,10 +964,7 @@ ReactDOMComponent.Mixin = {
} else if (registrationNameModules.hasOwnProperty(propKey)) {
// Do nothing for event names.
} else if (!DOMProperty.isReservedProp(propKey)) {
if (
isCustomComponent(this._tag, lastProps) &&
this._namespaceURI === Namespaces.html
) {
if (isCustomComponent(this._tag, lastProps, this._namespaceURI)) {
DOMPropertyOperations.deleteValueForAttribute(getNode(this), propKey);
} else {
DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey);
Expand Down
13 changes: 5 additions & 8 deletions src/renderers/shared/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ if (__DEV__) {
var {
validateProperties: validateUnknownPropertes,
} = require('ReactDOMUnknownPropertyHook');
var validatePropertiesInDevelopment = function(type, props) {
validateARIAProperties(type, props);
var validatePropertiesInDevelopment = function(type, namespace, props) {
validateARIAProperties(type, namespace, props);
validateInputPropertes(type, props);
validateUnknownPropertes(type, props);
validateUnknownPropertes(type, namespace, props);
};

var describeComponentFrame = require('describeComponentFrame');
Expand Down Expand Up @@ -278,10 +278,7 @@ function createOpenTagMarkup(
propValue = createMarkupForStyles(propValue, instForDebug);
}
var markup = null;
if (
isCustomComponent(tagLowercase, props) &&
namespace === Namespaces.html
) {
if (isCustomComponent(tagLowercase, props, namespace)) {
if (!RESERVED_PROPS.hasOwnProperty(propKey)) {
markup = DOMMarkupOperations.createMarkupForCustomAttribute(
propKey,
Expand Down Expand Up @@ -778,7 +775,7 @@ class ReactDOMServerRenderer {
}

if (__DEV__) {
validatePropertiesInDevelopment(tag, props);
validatePropertiesInDevelopment(tag, props, namespace);
}

assertValidProps(tag, props);
Expand Down