From 1be60390f550aa732200e1b26d9be8baddf82c73 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 9b4020e998..1988170779 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 c62945a025..ba6940fdfd 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 0c8abd0093..90407972ba 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 e3d5808dc1..1b1357b0e9 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 9932db14bd..ee6b151ead 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 d2f4d074bf..f48273bbad 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 4ed822e53b..282dd19318 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);