From 76bb96ef21b7c665e1b51b6bb90e64ec40c0f16a Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Wed, 25 Mar 2015 16:43:41 -0700 Subject: [PATCH] Validate only against problematic tag nesting Turns out lots of people write invalid HTML all the time and no one, including your browser or React, cares. Most invalid HTML combinations don't cause magic reparented nodes; only some do. The HTML5 parsing spec (https://html.spec.whatwg.org/multipage/syntax.html) specifies which tag combinations cause strange parsing behavior. I did my best to encode the logic here. It's more lenient than before in some cases, but more strict in others (before we didn't look at the whole stack of tags; now we warn with deeply nested `p` or `form` or `a` tags). --- .../__tests__/validateDOMNesting-test.js | 79 ++++ src/browser/ui/ReactDOMComponent.js | 8 +- src/browser/ui/ReactDOMTextComponent.js | 4 +- src/browser/ui/ReactMount.js | 4 +- src/browser/validateDOMNesting.js | 398 +++++++++++------- 5 files changed, 345 insertions(+), 148 deletions(-) create mode 100644 src/browser/__tests__/validateDOMNesting-test.js diff --git a/src/browser/__tests__/validateDOMNesting-test.js b/src/browser/__tests__/validateDOMNesting-test.js new file mode 100644 index 0000000000..4aaafeb243 --- /dev/null +++ b/src/browser/__tests__/validateDOMNesting-test.js @@ -0,0 +1,79 @@ +/** + * Copyright 2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var isTagValidInContext; + +// https://html.spec.whatwg.org/multipage/syntax.html#special +var specialTags = [ + 'address', 'applet', 'area', 'article', 'aside', 'base', 'basefont', + 'bgsound', 'blockquote', 'body', 'br', 'button', 'caption', 'center', 'col', + 'colgroup', 'dd', 'details', 'dir', 'div', 'dl', 'dt', 'embed', 'fieldset', + 'figcaption', 'figure', 'footer', 'form', 'frame', 'frameset', 'h1', 'h2', + 'h3', 'h4', 'h5', 'h6', 'head', 'header', 'hgroup', 'hr', 'html', 'iframe', + 'img', 'input', 'isindex', 'li', 'link', 'listing', 'main', 'marquee', 'menu', + 'menuitem', 'meta', 'nav', 'noembed', 'noframes', 'noscript', 'object', 'ol', + 'p', 'param', 'plaintext', 'pre', 'script', 'section', 'select', 'source', + 'style', 'summary', 'table', 'tbody', 'td', 'template', 'textarea', 'tfoot', + 'th', 'thead', 'title', 'tr', 'track', 'ul', 'wbr', 'xmp' +]; + +// https://html.spec.whatwg.org/multipage/syntax.html#formatting +var formattingTags = [ + 'a', 'b', 'big', 'code', 'em', 'font', 'i', 'nobr', 's', 'small', 'strike', + 'strong', 'tt', 'u' +]; + +function isTagStackValid(stack) { + for (var i = 0; i < stack.length; i++) { + if (!isTagValidInContext(stack[i], stack.slice(0, i))) { + //console.log('invalid', stack[i], stack.slice(0, i)); + return false; + } + } + return true; +} + +describe('ReactContextValidator', function() { + beforeEach(function() { + require('mock-modules').dumpCache(); + + isTagValidInContext = require('validateDOMNesting').isTagValidInContext; + }); + + it('allows any tag with no context', function() { + // With renderToString (for example), we don't know where we're mounting the + // tag so we must err on the side of leniency. + specialTags.concat(formattingTags, ['mysterytag']).forEach(function(tag) { + expect(isTagValidInContext(tag, [])).toBe(true); + }); + }); + + it('allows valid nestings', function() { + expect(isTagStackValid(['table', 'tbody', 'tr', 'td', 'b'])).toBe(true); + expect(isTagStackValid(['body', 'datalist', 'option'])).toBe(true); + expect(isTagStackValid(['div', 'a', 'object', 'a'])).toBe(true); + expect(isTagStackValid(['div', 'p', 'button', 'p'])).toBe(true); + expect(isTagStackValid(['p', 'svg', 'foreignObject', 'p'])).toBe(true); + + // Invalid, but not changed by browser parsing so we allow them + expect(isTagStackValid(['div', 'ul', 'ul', 'li'])).toBe(true); + expect(isTagStackValid(['div', 'label', 'div'])).toBe(true); + }); + + it('prevents problematic nestings', function() { + expect(isTagStackValid(['a', 'a'])).toBe(false); + expect(isTagStackValid(['form', 'form'])).toBe(false); + expect(isTagStackValid(['p', 'p'])).toBe(false); + expect(isTagStackValid(['table', 'tr'])).toBe(false); + }); +}); diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 4e45365732..fc3a1e447b 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -177,7 +177,8 @@ function processChildContext(context, tagName) { if (__DEV__) { // Pass down our tag name to child components for validation purposes context = assign({}, context); - context[validateDOMNesting.parentTagContextKey] = tagName; + var stack = context[validateDOMNesting.tagStackContextKey] || []; + context[validateDOMNesting.tagStackContextKey] = stack.concat([tagName]); } return context; } @@ -227,9 +228,9 @@ ReactDOMComponent.Mixin = { assertValidProps(this, this._currentElement.props); if (__DEV__) { - if (context[validateDOMNesting.parentTagContextKey]) { + if (context[validateDOMNesting.tagStackContextKey]) { validateDOMNesting( - context[validateDOMNesting.parentTagContextKey], + context[validateDOMNesting.tagStackContextKey], this._tag, this._currentElement ); @@ -318,6 +319,7 @@ ReactDOMComponent.Mixin = { CONTENT_TYPES[typeof props.children] ? props.children : null; var childrenToUse = contentToUse != null ? null : props.children; if (contentToUse != null) { + // TODO: Validate that text is allowed as a child of this node ret = escapeTextContentForBrowser(contentToUse); } else if (childrenToUse != null) { var mountImages = this.mountChildren( diff --git a/src/browser/ui/ReactDOMTextComponent.js b/src/browser/ui/ReactDOMTextComponent.js index af6369df97..7dbcb524b9 100644 --- a/src/browser/ui/ReactDOMTextComponent.js +++ b/src/browser/ui/ReactDOMTextComponent.js @@ -67,9 +67,9 @@ assign(ReactDOMTextComponent.prototype, { */ mountComponent: function(rootID, transaction, context) { if (__DEV__) { - if (context[validateDOMNesting.parentTagContextKey]) { + if (context[validateDOMNesting.tagStackContextKey]) { validateDOMNesting( - context[validateDOMNesting.parentTagContextKey], + context[validateDOMNesting.tagStackContextKey], 'span', null ); diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js index e3240c1941..9e64fd86d0 100644 --- a/src/browser/ui/ReactMount.js +++ b/src/browser/ui/ReactMount.js @@ -266,8 +266,8 @@ function mountComponentIntoNode( var context = emptyObject; if (__DEV__) { context = {}; - context[validateDOMNesting.parentTagContextKey] = - container.nodeName.toLowerCase(); + context[validateDOMNesting.tagStackContextKey] = + [container.nodeName.toLowerCase()]; } var markup = ReactReconciler.mountComponent( componentInstance, rootID, transaction, context diff --git a/src/browser/validateDOMNesting.js b/src/browser/validateDOMNesting.js index 831923ac7b..e759b3eba6 100644 --- a/src/browser/validateDOMNesting.js +++ b/src/browser/validateDOMNesting.js @@ -17,154 +17,267 @@ var warning = require('warning'); var validateDOMNesting = emptyFunction; if (__DEV__) { - // The below rules were created from the HTML5 spec and using - // https://github.com/facebook/xhp-lib/blob/1.6.0/src/html.php + // This validation code was written based on the HTML5 parsing spec: + // https://html.spec.whatwg.org/multipage/syntax.html#has-an-element-in-scope + // + // Note: this does not catch all invalid nesting, nor does it try to (as it's + // not clear what practical benefit doing so provides); instead, we warn only + // for cases where the parser will give a parse tree differing from what React + // intended. For example,
is invalid but we don't warn + // because it still parses correctly; we do warn for other cases like nested + //

tags where the beginning of the second element implicitly closes the + // first, causing a confusing mess. - // Flow elements are block or inline elements that can appear in a

- var flow = [ - 'a', 'abbr', 'address', 'area', 'article', 'aside', 'audio', 'b', 'bdi', - 'bdo', 'blockquote', 'br', 'button', 'canvas', 'cite', 'code', 'data', - 'datalist', 'del', 'details', 'dfn', 'div', 'dl', 'em', 'embed', - 'fieldset', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', - 'header', 'hr', 'i', 'iframe', 'img', 'input', 'ins', 'kbd', 'keygen', - 'label', 'link', 'main', 'map', 'mark', 'menu', 'meta', 'meter', 'nav', - 'noscript', 'object', 'ol', 'output', 'p', 'pre', 'progress', 'q', 'ruby', - 's', 'samp', 'script', 'section', 'select', 'small', 'span', 'strong', - 'style', 'sub', 'sup', 'svg', 'table', 'textarea', 'time', 'u', 'ul', - 'var', 'video', 'wbr', '#text' + // https://html.spec.whatwg.org/multipage/syntax.html#special + var specialTags = [ + 'address', 'applet', 'area', 'article', 'aside', 'base', 'basefont', + 'bgsound', 'blockquote', 'body', 'br', 'button', 'caption', 'center', 'col', + 'colgroup', 'dd', 'details', 'dir', 'div', 'dl', 'dt', 'embed', 'fieldset', + 'figcaption', 'figure', 'footer', 'form', 'frame', 'frameset', 'h1', 'h2', + 'h3', 'h4', 'h5', 'h6', 'head', 'header', 'hgroup', 'hr', 'html', 'iframe', + 'img', 'input', 'isindex', 'li', 'link', 'listing', 'main', 'marquee', + 'menu', 'menuitem', 'meta', 'nav', 'noembed', 'noframes', 'noscript', + 'object', 'ol', 'p', 'param', 'plaintext', 'pre', 'script', 'section', + 'select', 'source', 'style', 'summary', 'table', 'tbody', 'td', 'template', + 'textarea', 'tfoot', 'th', 'thead', 'title', 'tr', 'track', 'ul', 'wbr', + 'xmp' ]; - // Phrasing elements are inline elements that can appear in a - var phrase = [ - 'a', 'abbr', 'area', 'audio', 'b', 'bdi', 'bdo', 'br', 'button', 'canvas', - 'cite', 'code', 'data', 'datalist', 'del', 'dfn', 'em', 'embed', 'i', - 'iframe', 'img', 'input', 'ins', 'kbd', 'keygen', 'label', 'link', 'map', - 'mark', 'meta', 'meter', 'noscript', 'object', 'output', 'progress', 'q', - 'ruby', 's', 'samp', 'script', 'select', 'small', 'span', 'strong', 'sub', - 'sup', 'svg', 'textarea', 'time', 'u', 'var', 'video', 'wbr', '#text' - ]; - - // Metadata elements can appear in - var metadata = [ - 'base', 'link', 'meta', 'noscript', 'script', 'style', 'title' - ]; - - // By default, we assume that flow elements can contain other flow elements - // and phrasing elements can contain other phrasing elements. Here are the - // exceptions: - var allowedChildren = { - '#document': ['html'], - - 'a': flow, - 'audio': ['source', 'track'].concat(flow), - 'body': flow, - 'button': phrase, - 'caption': flow, - 'canvas': flow, - 'colgroup': ['col'], - 'dd': flow, - 'del': flow, - 'details': ['summary'].concat(flow), - 'dl': ['dt', 'dd'], - 'dt': flow, - 'fieldset': flow, - 'figcaption': flow, - 'figure': ['figcaption'].concat(flow), - 'h1': phrase, - 'h2': phrase, - 'h3': phrase, - 'h4': phrase, - 'h5': phrase, - 'h6': phrase, - 'head': metadata, - 'hgroup': ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'], - 'html': ['body', 'head'], - 'iframe': [], - 'ins': flow, - 'label': phrase, - 'legend': phrase, - 'li': flow, - 'map': flow, - 'menu': ['li', 'menuitem'].concat(flow), - 'noscript': '*', - 'object': ['param'].concat(flow), - 'ol': ['li'], - 'optgroup': ['option'], - 'p': phrase, - 'pre': phrase, - 'rp': phrase, - 'rt': phrase, - 'ruby': ['rp', 'rt', '#text'], - 'script': ['#text'], - 'select': ['option', 'optgroup'], - 'style': ['#text'], - 'summary': phrase, - 'table': ['caption', 'colgroup', 'tbody', 'tfoot', 'thead'], - 'tbody': ['tr'], - 'td': flow, - 'textarea': ['#text'], - 'tfoot': ['tr'], - 'th': flow, - 'thead': ['tr'], - 'title': ['#text'], - 'tr': ['td', 'th'], - 'ul': ['li'], - 'video': ['source', 'track'].concat(flow), - - // SVG - // TODO: Validate nesting of all svg elements - 'svg': [ - 'circle', 'defs', 'g', 'line', 'linearGradient', 'path', 'polygon', - 'polyline', 'radialGradient', 'rect', 'stop', 'text' - ], - - // Self-closing tags - 'area': [], - 'base': [], - 'br': [], - 'col': [], - 'embed': [], - 'hr': [], - 'img': [], - 'input': [], - 'keygen': [], - 'link': [], - 'menuitem': [], - 'meta': [], - 'param': [], - 'source': [], - 'track': [], - 'wbr': [] + /** + * Return whether `stack` contains `tag` and the last occurrence of `tag` is + * deeper than any element in the `scope` array. + * + * https://html.spec.whatwg.org/multipage/syntax.html#has-an-element-in-the-specific-scope + * + * Examples: + * stackHasTagInSpecificScope(['p', 'quote'], 'p', ['button']) is true + * stackHasTagInSpecificScope(['p', 'button'], 'p', ['button']) is false + * + * @param {Array} stack + * @param {string} tag + * @param {Array} scope + */ + var stackHasTagInSpecificScope = function(stack, tag, scope) { + for (var i = stack.length - 1; i >= 0; i--) { + if (stack[i] === tag) { + return true; + } + if (scope.indexOf(stack[i]) !== -1) { + return false; + } + } + return false; }; - var i, l; - var allowedChildrenMap = {}; - for (i = 0, l = flow.length; i < l; i++) { - allowedChildrenMap[flow[i]] = flow; - } - for (i = 0, l = phrase.length; i < l; i++) { - allowedChildrenMap[phrase[i]] = flow; - } - for (var el in allowedChildren) { - if (allowedChildren.hasOwnProperty(el)) { - allowedChildrenMap[el] = allowedChildren[el]; - } - } + // https://html.spec.whatwg.org/multipage/syntax.html#has-an-element-in-scope + var inScopeTags = [ + 'applet', 'caption', 'html', 'table', 'td', 'th', 'marquee', 'object', + 'template', - var nodeCanContainNode = function(parentTag, childTag) { - var allowed = allowedChildrenMap[parentTag]; - if (!allowed || !allowedChildrenMap[childTag]) { - // We don't recognize one of the tags; err on the side of not warning - return true; - } - - var result = allowed === '*' || allowed.indexOf(childTag) !== -1; - return !!result; + // https://html.spec.whatwg.org/multipage/syntax.html#html-integration-point + // TODO: Distinguish by namespace here + 'foreignObject', 'desc', 'title' + ]; + var stackHasTagInScope = function(stack, tag) { + return stackHasTagInSpecificScope(stack, tag, inScopeTags); }; - validateDOMNesting = function(parentTag, childTag, element) { - if (!nodeCanContainNode(parentTag, childTag)) { + // https://html.spec.whatwg.org/multipage/syntax.html#has-an-element-in-button-scope + var buttonScopeTags = inScopeTags.concat(['button']); + var stackHasTagInButtonScope = function(stack, tag) { + return stackHasTagInSpecificScope(stack, tag, buttonScopeTags); + }; + + // See rules for 'li', 'dd', 'dt' start tags in + // https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody + var listItemTagAllowed = function(tags, stack) { + // tags is ['li'] or ['dd, 'dt'] + for (var i = stack.length - 1; i >= 0; i--) { + if (tags.indexOf(stack[i]) !== -1) { + return false; + } else if ( + specialTags.indexOf(stack[i]) !== -1 && + stack[i] !== 'address' && stack[i] !== 'div' && stack[i] !== 'p' + ) { + return true; + } + } + return true; + }; + + // https://html.spec.whatwg.org/multipage/syntax.html#generate-implied-end-tags + var impliedEndTags = + ['dd', 'dt', 'li', 'option', 'optgroup', 'p', 'rp', 'rt']; + + /** + * Returns whether we allow putting `tag` in the document if the current stack + * of open tags is `openTagStack`. + * + * Examples: + * isTagValidInContext('tr', [..., 'table', 'tbody']) is true + * isTagValidInContext('tr', [..., 'table']) is false + * + * @param {string} tag Lowercase HTML tag name or node name like '#text' + * @param {Array} openTagStack + */ + var isTagValidInContext = function(tag, openTagStack) { + var currentTag = openTagStack[openTagStack.length - 1]; + + // First, let's check if we're in an unusual parsing mode... + switch (currentTag) { + // https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inselect + case 'select': + return tag === 'option' || tag === 'optgroup' || tag === '#text'; + case 'optgroup': + return tag === 'option' || tag === '#text'; + // Strictly speaking, seeing an