Skip to content

Commit

Permalink
de-generalize to specific use case and refactor tests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gnoff committed Apr 15, 2022
1 parent be5bc4d commit 7dd5cc7
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 146 deletions.
58 changes: 58 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2811,4 +2811,62 @@ describe('ReactDOMFizzServer', () => {
</ul>,
);
});

describe('bootstrapScriptContent escaping', () => {
it('the "S" in "</?[Ss]cript" strings are replaced with unicode escaped lowercase s or S depending on case, preserving case sensitivity of nearby characters', async () => {
window.__test_outlet = '';
let stringWithScriptsInIt =
'prescription pre<scription pre<Scription pre</scRipTion pre</ScripTion </script><script><!-- <script> -->';
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<div />, {
bootstrapScriptContent:
'window.__test_outlet = "This should have been replaced";var x = "' +
stringWithScriptsInIt +
'";\nwindow.__test_outlet = x;',
});
pipe(writable);
});
expect(window.__test_outlet).toMatch(stringWithScriptsInIt);
});

it('does not escape \\u2028, or \\u2029 characters', async () => {
// these characters are ignored in engines support https://github.com/tc39/proposal-json-superset
// in this test with JSDOM the characters are silently dropped and thus don't need to be encoded.
// if you send these characters to an older browser they could fail so it is a good idea to
// sanitize JSON input of these characters
window.__test_outlet = '';
const el = document.createElement('p');
el.textContent = '{"one":1,\u2028\u2029"two":2}';
let stringWithLSAndPSCharacters = el.textContent;
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<div />, {
bootstrapScriptContent:
'let x = ' +
stringWithLSAndPSCharacters +
'; window.__test_outlet = x;',
});
pipe(writable);
});
const outletString = JSON.stringify(window.__test_outlet);
expect(outletString).toBe(
stringWithLSAndPSCharacters.replace(/[\u2028\u2029]/g, ''),
);
});

it('does not escape <, >, or & characters', async () => {
// these characters valid javascript and may be necessary in scripts and won't be interpretted properly
// escaped outside of a string context within javascript
window.__test_outlet = null;
// this boolean expression will be cast to a number due to the bitwise &. we will look for a truthy value (1) below
let booleanLogicString = '1 < 2 & 3 > 1';
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<div />, {
bootstrapScriptContent:
'let x = ' + booleanLogicString + '; window.__test_outlet = x;',
});
pipe(writable);
});
expect(window.__test_outlet).toBe(1);
});
});
});
107 changes: 0 additions & 107 deletions packages/react-dom/src/__tests__/escapeScriptForBrowser-test.js

This file was deleted.

19 changes: 17 additions & 2 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -84,6 +83,22 @@ const startScriptSrc = stringToPrecomputedChunk('<script src="');
const startModuleSrc = stringToPrecomputedChunk('<script type="module" src="');
const endAsyncScript = stringToPrecomputedChunk('" async=""></script>');

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,
Expand All @@ -103,7 +118,7 @@ export function createResponseState(
if (bootstrapScriptContent !== undefined) {
bootstrapChunks.push(
inlineScriptWithNonce,
stringToChunk(escapeScriptForBrowser(bootstrapScriptContent)),
stringToChunk(escapeBootstrapScriptContent(bootstrapScriptContent)),
endInlineScript,
);
}
Expand Down
37 changes: 0 additions & 37 deletions packages/react-dom/src/server/escapeScriptForBrowser.js

This file was deleted.

0 comments on commit 7dd5cc7

Please sign in to comment.