From 23d8c7b4e0833efbd01dd490850b7f1a440ce6b4 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sun, 18 Nov 2018 12:13:16 +0000 Subject: [PATCH] Update escaping of attribute and text HTML characters for better performance --- .../src/__tests__/ReactDOMComponent-test.js | 4 +- .../__tests__/escapeTextForBrowser-test.js | 11 +- .../quoteAttributeValueForBrowser-test.js | 16 +- .../src/server/ReactPartialRenderer.js | 10 +- .../src/server/escapeTextForBrowser.js | 141 +++++++++--------- .../server/quoteAttributeValueForBrowser.js | 4 +- 6 files changed, 90 insertions(+), 96 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index e695a845ff5..2e21299770b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -1599,9 +1599,7 @@ describe('ReactDOMComponent', () => { ), ), ).toBe( - '
' + - ''"<>&' + - '
', + `
&\" style=\"text-align:'"<>&\">'\"<>&
`, ); }); }); diff --git a/packages/react-dom/src/__tests__/escapeTextForBrowser-test.js b/packages/react-dom/src/__tests__/escapeTextForBrowser-test.js index 08d324daae3..eab3468a9fc 100644 --- a/packages/react-dom/src/__tests__/escapeTextForBrowser-test.js +++ b/packages/react-dom/src/__tests__/escapeTextForBrowser-test.js @@ -24,14 +24,14 @@ describe('escapeTextForBrowser', () => { expect(response).toMatch('&'); }); - it('double quote is escaped when passed as text content', () => { + it('double quote is not escaped when passed as text content', () => { const response = ReactDOMServer.renderToString({'"'}); - expect(response).toMatch('"'); + expect(response).toMatch('"'); }); - it('single quote is escaped when passed as text content', () => { + it('single quote is not escaped when passed as text content', () => { const response = ReactDOMServer.renderToString({"'"}); - expect(response).toMatch('''); + expect(response).toMatch(`'`); }); it('greater than entity is escaped when passed as text content', () => { @@ -59,8 +59,7 @@ describe('escapeTextForBrowser', () => { {''}, ); expect(response).toMatch( - '<script type='' ' + - 'src=""></script>', + `<script type='' src=\"\"></script>`, ); }); }); diff --git a/packages/react-dom/src/__tests__/quoteAttributeValueForBrowser-test.js b/packages/react-dom/src/__tests__/quoteAttributeValueForBrowser-test.js index fc2a711149f..b47bef812ca 100644 --- a/packages/react-dom/src/__tests__/quoteAttributeValueForBrowser-test.js +++ b/packages/react-dom/src/__tests__/quoteAttributeValueForBrowser-test.js @@ -29,19 +29,19 @@ describe('quoteAttributeValueForBrowser', () => { expect(response).toMatch(''); }); - it('single quote is escaped inside attributes', () => { + it('single quote is not escaped inside attributes', () => { const response = ReactDOMServer.renderToString(); - expect(response).toMatch(''); + expect(response).toMatch(``); }); - it('greater than entity is escaped inside attributes', () => { + it('greater than entity is not escaped inside attributes', () => { const response = ReactDOMServer.renderToString(); - expect(response).toMatch(''); + expect(response).toMatch(''); }); - it('lower than entity is escaped inside attributes', () => { + it('lower than entity is not escaped inside attributes', () => { const response = ReactDOMServer.renderToString(); - expect(response).toMatch(''); + expect(response).toMatch(''); }); it('number is escaped to string inside attributes', () => { @@ -67,9 +67,7 @@ describe('quoteAttributeValueForBrowser', () => { '} />, ); expect(response).toMatch( - '', + `\" data-reactroot=\"\"/>`, ); }); }); diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index dfc1aba8c88..0f44144a068 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -50,7 +50,7 @@ import { createMarkupForProperty, createMarkupForRoot, } from './DOMMarkupOperations'; -import escapeTextForBrowser from './escapeTextForBrowser'; +import {escapeText} from './escapeTextForBrowser'; import { prepareToUseHooks, finishHooks, @@ -280,7 +280,7 @@ function getNonChildrenInnerMarkup(props) { } else { const content = props.children; if (typeof content === 'string' || typeof content === 'number') { - return escapeTextForBrowser(content); + return escapeText(content); } } return null; @@ -886,13 +886,13 @@ class ReactDOMServerRenderer { return ''; } if (this.makeStaticMarkup) { - return escapeTextForBrowser(text); + return escapeText(text); } if (this.previousWasTextNode) { - return '' + escapeTextForBrowser(text); + return '' + escapeText(text); } this.previousWasTextNode = true; - return escapeTextForBrowser(text); + return escapeText(text); } else { let nextChild; ({child: nextChild, context} = resolve(child, context, this.threadID)); diff --git a/packages/react-dom/src/server/escapeTextForBrowser.js b/packages/react-dom/src/server/escapeTextForBrowser.js index 2a5a91aafde..679fd1695fa 100644 --- a/packages/react-dom/src/server/escapeTextForBrowser.js +++ b/packages/react-dom/src/server/escapeTextForBrowser.js @@ -30,82 +30,81 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -// code copied and modified from escape-html -/** - * Module variables. - * @private - */ - -const matchHtmlRegExp = /["'&<>]/; - -/** - * Escapes special characters and HTML entities in a given html string. - * - * @param {string} string HTML string to escape for later insertion - * @return {string} - * @public - */ - -function escapeHtml(string) { - const str = '' + string; - const match = matchHtmlRegExp.exec(str); - - if (!match) { - return str; - } - - let escape; - let html = ''; - let index; - let lastIndex = 0; - - for (index = match.index; index < str.length; index++) { - switch (str.charCodeAt(index)) { - case 34: // " - escape = '"'; - break; - case 38: // & - escape = '&'; - break; - case 39: // ' - escape = '''; // modified from escape-html; used to be ''' - break; - case 60: // < - escape = '<'; - break; - case 62: // > - escape = '>'; - break; - default: - continue; +// double quotes and single quotes are allowed in text nodes +export function escapeText(text: string | number): string { + if (typeof text === 'string') { + if ( + text.indexOf('&') === -1 && + text.indexOf('<') === -1 && + text.indexOf('>') === -1 + ) { + return text; } - if (lastIndex !== index) { - html += str.substring(lastIndex, index); + let result = text; + let start = 0; + let i = 0; + for (; i < text.length; ++i) { + let escape; + switch (text.charCodeAt(i)) { + case 38: // & + escape = '&'; + break; + case 60: // < + escape = '<'; + break; + case 62: // > + escape = '>'; + break; + default: + continue; + } + if (i > start) { + escape = text.slice(start, i) + escape; + } + result = start > 0 ? result + escape : escape; + start = i + 1; } - - lastIndex = index + 1; - html += escape; + if (i !== start) { + return result + text.slice(start, i); + } + return result; } - - return lastIndex !== index ? html + str.substring(lastIndex, index) : html; + return text.toString(); } -// end code copied and modified from escape-html -/** - * Escapes text to prevent scripting attacks. - * - * @param {*} text Text value to escape. - * @return {string} An escaped string. - */ -function escapeTextForBrowser(text) { - if (typeof text === 'boolean' || typeof text === 'number') { - // this shortcircuit helps perf for types that we know will never have - // special characters, especially given that this function is used often - // for numeric dom ids. - return '' + text; +// <, > and single quotes are allowed in attribute values +export function escapeAttributeValue(text: string | number): string { + if (typeof text === 'string') { + if (text.indexOf('"') === -1 && text.indexOf('&') === -1) { + return text; + } + + let result = text; + let start = 0; + let i = 0; + for (; i < text.length; ++i) { + let escape; + switch (text.charCodeAt(i)) { + case 34: // " + escape = '"'; + break; + case 38: // & + escape = '&'; + break; + default: + continue; + } + if (i > start) { + escape = text.slice(start, i) + escape; + } + result = start > 0 ? result + escape : escape; + start = i + 1; + } + if (i !== start) { + return result + text.slice(start, i); + } + return result; } - return escapeHtml(text); + return text.toString(); } - -export default escapeTextForBrowser; diff --git a/packages/react-dom/src/server/quoteAttributeValueForBrowser.js b/packages/react-dom/src/server/quoteAttributeValueForBrowser.js index b628155168c..bf96291b2a7 100644 --- a/packages/react-dom/src/server/quoteAttributeValueForBrowser.js +++ b/packages/react-dom/src/server/quoteAttributeValueForBrowser.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import escapeTextForBrowser from './escapeTextForBrowser'; +import {escapeAttributeValue} from './escapeTextForBrowser'; /** * Escapes attribute value to prevent scripting attacks. @@ -14,7 +14,7 @@ import escapeTextForBrowser from './escapeTextForBrowser'; * @return {string} An escaped string. */ function quoteAttributeValueForBrowser(value) { - return '"' + escapeTextForBrowser(value) + '"'; + return '"' + escapeAttributeValue(value) + '"'; } export default quoteAttributeValueForBrowser;