facebook/react — pull request #1599 by syranide (2014-05-25 10:43:01+00:00)
Title: Split escapeTextForBrowser into escapeTextContentForBrowser and quoteAttributeValueForBrowser

IMHO the preferable solution to #1461, my comment from that PR:

Chrome only escapes `<`, `>` and `&` when setting `textContent`, it only escapes `&` and `"` when setting an attribute. Which I would say makes my suggestion above quite a lot less alien (to me at least).

It aligns with OWASPs stance that that *only* `"` can be used to break out of `"` for quoted attribute values.

It also aligns with OWASPs recommendation for text/html, `'` and `"` is unnecessary because we're dealing with plain text and `/` is unnecessary because it's a precaution against there being an unescaped `<` before the injected content.

Attribute names **cannot** be escaped, invalid chars invalidate the entire attribute name. Since we already very strictly filter against invalid attribute names we don't need to do anything further for attribute names.

Attribute names: discard invalid
Attribute values: `&` + `"`
Text content: `&` + `<` + `>`

With these rules we would generate the same HTML that browsers do, and also not clutter the HTML unnecessarily.

The *only* danger I see is if `dangerouslySetInnerHTML` is used with invalid HTML (you got way bigger problems then), if there's an unclosed tag at the end of it then you can now insert attributes into that element, but this is basically a non-issue in practice as `dangerouslySetInnerHTML` is always used on an existing element, which means that React will *always* close it of "properly" preventing injection of malicious attributes.