From 52786bf04ee811ba6903232cf3696f4258972666 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 15 Apr 2022 15:14:39 -0700 Subject: [PATCH] de-generalize to specific use case and refactor tests the escaping of this function does is tailored to the specific use case of how bootstrapScriptContent is currently set up and having it be a module suggests it is meant for a more general than it has been considered for. Additionally the tests were redone to focus on practical implications for what is and is not escaped --- .../src/__tests__/ReactDOMFizzServer-test.js | 58 ++++++++++ .../__tests__/escapeScriptForBrowser-test.js | 107 ------------------ .../src/server/ReactDOMServerFormatConfig.js | 19 +++- .../src/server/escapeScriptForBrowser.js | 37 ------ 4 files changed, 75 insertions(+), 146 deletions(-) delete mode 100644 packages/react-dom/src/__tests__/escapeScriptForBrowser-test.js delete mode 100644 packages/react-dom/src/server/escapeScriptForBrowser.js diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 295e99b26a691..77b40c3998793 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2811,4 +2811,62 @@ describe('ReactDOMFizzServer', () => { , ); }); + + describe('bootstrapScriptContent escaping', () => { + it('the "S" in " { + window.__test_outlet = ''; + const stringWithScriptsInIt = + 'prescription pre', - ); - }); - - it('" { - const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
, { - bootstrapScriptContent: - '"prescription pre
', - ); - }); - - it('"[Ss]cript", "/[Ss]cript", "<[Ss]crip", " { - const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
, { - bootstrapScriptContent: - '"Script script /Script /script
', - ); - }); - - it('matches case insensitively', () => { - const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
, { - bootstrapScriptContent: '"', - ); - }); - - it('does not escape <, >, &, \\u2028, or \\u2029 characters', () => { - const {writable, output} = getTestWritable(); - const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
, { - bootstrapScriptContent: '"<, >, &, \u2028, or \u2029"', - }); - pipe(writable); - jest.runAllTimers(); - expect(output.result).toMatch( - '
', - ); - }); -}); diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index 7b618fd601573..b0bcba20618dc 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -52,7 +52,6 @@ import {validateProperties as validateUnknownProperties} from '../shared/ReactDO import warnValidStyle from '../shared/warnValidStyle'; import escapeTextForBrowser from './escapeTextForBrowser'; -import escapeScriptForBrowser from './escapeScriptForBrowser'; import hyphenateStyleName from '../shared/hyphenateStyleName'; import hasOwnProperty from 'shared/hasOwnProperty'; import sanitizeURL from '../shared/sanitizeURL'; @@ -84,6 +83,22 @@ const startScriptSrc = stringToPrecomputedChunk(''); +const scriptRegex = /(<\/|<)(s)(cript)/gi; +const substitutions = { + s: '\\u0073', + S: '\\u0053', +}; + +function escapeBootstrapScriptContent(scriptText) { + if (__DEV__) { + checkHtmlStringCoercion(scriptText); + } + return ('' + scriptText).replace( + scriptRegex, + (match, prefix, s, suffix) => `${prefix}${substitutions[s]}${suffix}`, + ); +} + // Allows us to keep track of what we've already written so we can refer back to it. export function createResponseState( identifierPrefix: string | void, @@ -103,7 +118,7 @@ export function createResponseState( if (bootstrapScriptContent !== undefined) { bootstrapChunks.push( inlineScriptWithNonce, - stringToChunk(escapeScriptForBrowser(bootstrapScriptContent)), + stringToChunk(escapeBootstrapScriptContent(bootstrapScriptContent)), endInlineScript, ); } diff --git a/packages/react-dom/src/server/escapeScriptForBrowser.js b/packages/react-dom/src/server/escapeScriptForBrowser.js deleted file mode 100644 index 5827072c73dd7..0000000000000 --- a/packages/react-dom/src/server/escapeScriptForBrowser.js +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {checkHtmlStringCoercion} from 'shared/CheckStringCoercion'; - -const scriptRegex = /(<\/|<)(s)(cript)/gi; -const scriptReplacer = (match, prefix, s, suffix) => - `${prefix}${substitutions[s]}${suffix}`; -const substitutions = { - s: '\\u0073', - S: '\\u0053', -}; - -/** - * Escapes javascript for embedding into HTML. - * - * @param {*} scriptText Text value to escape. - * @return {string} An escaped string. - */ -function escapeScriptForBrowser(scriptText) { - if (typeof scriptText === 'boolean' || typeof scriptText === '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 '' + scriptText; - } - if (__DEV__) { - checkHtmlStringCoercion(scriptText); - } - return ('' + scriptText).replace(scriptRegex, scriptReplacer); -} - -export default escapeScriptForBrowser;