From 934177598e583672ac12fbc63288848c2f2930bc Mon Sep 17 00:00:00 2001 From: Josh Story Date: Sat, 22 Oct 2022 09:56:40 -0700 Subject: [PATCH] fix transposed escape functions (#25534) escapeTextForBrowser accepts any type so flow did not identify that we were escaping a Chunk rather than a string. It's tricky because we sometimes want to be able to escape non strings. I've also updated the types for `Chunk` and `escapeTextForBrowser` so that we should be able to catch this statically in the future. The reason this did not show up in tests is almost all of our tests of float (the areas affected by transpositions) are tested using the Node runtime where a chunk type is a string. It may be wise to run these tests in every runtime in the future or at least make sure there is broad representation of resources in each specific runtime test suite. --- .../server/ReactDOMLegacyServerStreamConfig.js | 4 ++-- .../src/server/ReactDOMServerFormatConfig.js | 6 +++--- .../src/server/escapeTextForBrowser.js | 6 ++++-- .../ReactDOMFizzServerBrowser-test.js | 18 ++++++++++++++++++ .../src/ReactServerStreamConfigFB.js | 4 ++-- .../src/ReactServerStreamConfigBrowser.js | 2 +- .../src/ReactServerStreamConfigNode.js | 2 +- 7 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js b/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js index 9b4020e998c5e..1988170779e4c 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js @@ -12,8 +12,8 @@ export interface Destination { destroy(error: Error): mixed; } -export type PrecomputedChunk = string; -export type Chunk = string; +export opaque type PrecomputedChunk = string; +export opaque type Chunk = string; export function scheduleWork(callback: () => void) { callback(); diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index c62945a02520a..ba6940fdfd7a4 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -2385,7 +2385,7 @@ export function writeInitialResources( } else { target.push( precedencePlaceholderStart, - escapeTextForBrowser(stringToChunk(precedence)), + stringToChunk(escapeTextForBrowser(precedence)), precedencePlaceholderEnd, ); } @@ -2417,7 +2417,7 @@ export function writeInitialResources( case 'title': { pushStartTitleImpl(target, r.props, responseState); if (typeof r.props.children === 'string') { - target.push(escapeTextForBrowser(stringToChunk(r.props.children))); + target.push(stringToChunk(escapeTextForBrowser(r.props.children))); } pushEndInstance(target, target, 'title', r.props); break; @@ -2518,7 +2518,7 @@ export function writeImmediateResources( case 'title': { pushStartTitleImpl(target, r.props, responseState); if (typeof r.props.children === 'string') { - target.push(escapeTextForBrowser(stringToChunk(r.props.children))); + target.push(stringToChunk(escapeTextForBrowser(r.props.children))); } pushEndInstance(target, target, 'title', r.props); break; diff --git a/packages/react-dom-bindings/src/server/escapeTextForBrowser.js b/packages/react-dom-bindings/src/server/escapeTextForBrowser.js index 0c8abd00938c5..90407972ba02a 100644 --- a/packages/react-dom-bindings/src/server/escapeTextForBrowser.js +++ b/packages/react-dom-bindings/src/server/escapeTextForBrowser.js @@ -28,6 +28,8 @@ * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + * + * @flow */ // code copied and modified from escape-html @@ -103,12 +105,12 @@ function escapeHtml(string) { * @param {*} text Text value to escape. * @return {string} An escaped string. */ -function escapeTextForBrowser(text) { +function escapeTextForBrowser(text: string | number | boolean): string { 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; + return '' + (text: any); } return escapeHtml(text); } diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index e3d5808dc1080..1b1357b0e9cea 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -484,4 +484,22 @@ describe('ReactDOMFizzServerBrowser', () => { expect(errors).toEqual(['uh oh', 'uh oh']); }); + + // https://github.com/facebook/react/pull/25534/files - fix transposed escape functions + // @gate enableFloat + it('should encode title properly', async () => { + const stream = await ReactDOMFizzServer.renderToReadableStream( + + + foo + + bar + , + ); + + const result = await readResult(stream); + expect(result).toEqual( + 'foobar', + ); + }); }); diff --git a/packages/react-server-dom-relay/src/ReactServerStreamConfigFB.js b/packages/react-server-dom-relay/src/ReactServerStreamConfigFB.js index 9932db14bd87c..ee6b151ead541 100644 --- a/packages/react-server-dom-relay/src/ReactServerStreamConfigFB.js +++ b/packages/react-server-dom-relay/src/ReactServerStreamConfigFB.js @@ -14,8 +14,8 @@ export type Destination = { error: mixed, }; -export type PrecomputedChunk = string; -export type Chunk = string; +export opaque type PrecomputedChunk = string; +export opaque type Chunk = string; export function scheduleWork(callback: () => void) { // We don't schedule work in this model, and instead expect performWork to always be called repeatedly. diff --git a/packages/react-server/src/ReactServerStreamConfigBrowser.js b/packages/react-server/src/ReactServerStreamConfigBrowser.js index d2f4d074bf2df..f48273bbadfac 100644 --- a/packages/react-server/src/ReactServerStreamConfigBrowser.js +++ b/packages/react-server/src/ReactServerStreamConfigBrowser.js @@ -10,7 +10,7 @@ export type Destination = ReadableStreamController; export type PrecomputedChunk = Uint8Array; -export type Chunk = Uint8Array; +export opaque type Chunk = Uint8Array; export function scheduleWork(callback: () => void) { callback(); diff --git a/packages/react-server/src/ReactServerStreamConfigNode.js b/packages/react-server/src/ReactServerStreamConfigNode.js index 4ed822e53b381..282dd193182b3 100644 --- a/packages/react-server/src/ReactServerStreamConfigNode.js +++ b/packages/react-server/src/ReactServerStreamConfigNode.js @@ -17,7 +17,7 @@ interface MightBeFlushable { export type Destination = Writable & MightBeFlushable; export type PrecomputedChunk = Uint8Array; -export type Chunk = string; +export opaque type Chunk = string; export function scheduleWork(callback: () => void) { setImmediate(callback);