Denial of Service (resource exhaustion)

Proof-of-concept (PoC) to illustrate the issue before and after the fix: // Before fix (conceptual): no threshold, full string is sent function renderDebugValueBeforeFix(value) { // No length check; always returns the full string return value; } // After fix (as implemented in the commit): threshold at 1,000,000 chars function renderDebugValueAfterFix(value) { if (typeof value === 'string' && value.length > 1000000) { return ( 'This string of length ' + value.length + ' has been omitted by React to avoid sending too much data from the server.' ); } return value; } // Demonstration with a 1,000,001-character string const big = 'x'.repeat(1000001); console.time('before'); console.log(renderDebugValueBeforeFix(big).length); console.timeEnd('before'); console.time('after'); console.log(renderDebugValueAfterFix(big).length); console.timeEnd('after'); Expected outcome: - Before: the length logged is 1000001 (full string processed). - After: the length logged is the length of the placeholder text (much smaller), illustrating the mitigation of main-thread work and potential DoS exposure. Additionally, the fix is exercised in tests that ensure large strings are replaced by placeholders during server replay of logs and in debug info paths.

diff --git a/fixtures/flight/src/App.js b/fixtures/flight/src/App.js index 9f8deed495d3..1ecfbbcd9cac 100644 --- a/fixtures/flight/src/App.js +++ b/fixtures/flight/src/App.js @@ -27,6 +27,7 @@ import {like, greet, increment} from './actions.js'; import {getServerState} from './ServerState.js'; import {sdkMethod} from './library.js'; +import FileReader from './FileReader.js'; const promisedText = new Promise(resolve => setTimeout(() => resolve('deferred text'), 50) @@ -243,6 +244,11 @@ export default async function App({prerender, noCache}) { {prerender ? null : ( // TODO: prerender is broken for large content for some reason. <React.Suspense fallback={null}> <LargeContent /> + {/* + This text prop is above the threshold, so in the debug info for + the element we'll see a placeholder instead of the actual value. + */} + <FileReader largeText={'a'.repeat(1000001)} /> </React.Suspense> )} </Container> diff --git a/fixtures/flight/src/FileReader.js b/fixtures/flight/src/FileReader.js new file mode 100644 index 000000000000..aeb104f26b45 --- /dev/null +++ b/fixtures/flight/src/FileReader.js @@ -0,0 +1,16 @@ +export default async function FileReader() { + // This debug string is below the threshold for debug string length, so its + // value is sent to the client as the awaited value. + await new Promise(resolve => { + setTimeout(() => resolve('o'.repeat(1000000)), 1); + }); + + // This debug string is above the threshold for debug string length, so the + // client receives a placeholder as the awaited value instead of the actual + // string. + await new Promise(resolve => { + setTimeout(() => resolve('x'.repeat(1000001)), 1); + }); + + return <p>FileReader</p>; +} diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 8736b0585f1b..95633283b2c7 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -3743,6 +3743,59 @@ describe('ReactFlight', () => { expect(cyclic2.cycle).toBe(cyclic2); }); + // @gate __DEV__ + it('replays logs with large strings replaced by a placeholder', async () => { + // This string exceeds the threshold for debug string length. Reconstructing + // a multi-megabyte string on the client when replaying the log would block + // the main thread for too long, so we omit it and send a placeholder + // instead. + const largeString = 'x'.repeat(1000001); + + function ServerComponent() { + console.log('large string:', largeString); + return null; + } + + function App() { + return ReactServer.createElement(ServerComponent); + } + + // These tests are specifically testing console.log. + // Assign to `mockConsoleLog` so we can still inspect it when `console.log` + // is overridden by the test modules. The original function will be restored + // after this test finishes by `jest.restoreAllMocks()`. + const mockConsoleLog = spyOnDevAndProd(console, 'log').mockImplementation( + () => {}, + ); + + // Reset the modules so that we get a new overridden console on top of the + // one installed by expect. This ensures that we still emit console.error + // calls. + jest.resetModules(); + jest.mock('react', () => require('react/react.react-server')); + ReactServer = require('react'); + ReactNoopFlightServer = require('react-noop-renderer/flight-server'); + const transport = ReactNoopFlightServer.render({ + root: ReactServer.createElement(App), + }); + + // The server logged the actual string synchronously while rendering. + expect(mockConsoleLog).toHaveBeenCalledTimes(1); + expect(mockConsoleLog.mock.calls[0][1]).toBe(largeString); + mockConsoleLog.mockClear(); + mockConsoleLog.mockImplementation(() => {}); + + await ReactNoopFlightClient.read(transport); + + // The replayed log received a placeholder instead of the actual string. + expect(mockConsoleLog).toHaveBeenCalledTimes(1); + expect(mockConsoleLog.mock.calls[0][0]).toBe('large string:'); + expect(mockConsoleLog.mock.calls[0][1]).toBe( + 'This string of length 1000001 has been omitted by React to avoid ' + + 'sending too much data from the server.', + ); + }); + // @gate !__DEV__ || enableComponentPerformanceTrack it('uses the server component debug info as the element owner in DEV', async () => { function Container({children}) { diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 6330e86dde60..ef6e9caac4b2 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -5160,6 +5160,17 @@ function renderDebugModel( } if (typeof value === 'string') { + if (value.length > 1000000) { + // Reconstructing a multi-megabyte string on the client blocks the main + // thread for too long. We omit the actual value and send a placeholder + // instead. + return ( + 'This string of length ' + + value.length + + ' has been omitted by React to avoid sending too much data from the ' + + 'server.' + ); + } if (value.length >= 1024) { // Large strings are counted towards the object limit. if (counter.objectLimit <= 0) { diff --git a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js index 5107fc0bfaea..f659c2bba839 100644 --- a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js +++ b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js @@ -3669,4 +3669,267 @@ describe('ReactFlightAsyncDebugInfo', () => { await finishLoadingStream(readable); }); + + it('omits large debug strings to avoid blocking the main thread when parsing', async () => { + async function Component() { + // This promise's value is expected to show up in the debug info below. + const small = await new Promise(resolve => { + setTimeout(() => resolve('hello'), 1); + }); + + // This promise's value exceeds the threshold for debug string length and + // is expected to show up as a placeholder in the debug info below. + // Reconstructing a multi-megabyte string on the client would block the + // main thread for too long. + const large = await new Promise(resolve => { + setTimeout(() => resolve('x'.repeat(1000001)), 1); + }); + + return small + ' ' + large.length; + } + + const stream = ReactServerDOMServer.renderToPipeableStream( + ReactServer.createElement(Component), + {}, + {filterStackFrame}, + ); + + const readable = new Stream.PassThrough(streamOptions); + + const result = ReactServerDOMClient.createFromNodeStream(readable, { + moduleMap: {}, + moduleLoading: {}, + }); + stream.pipe(readable); + + expect(await result).toBe('hello 1000001'); + + await finishLoadingStream(readable); + if ( + __DEV__ && + gate( + flags => + flags.enableComponentPerformanceTrack && flags.enableAsyncDebugInfo, + ) + ) { + expect(getDebugInfo(result)).toMatchInlineSnapshot(` + [ + { + "time": 0, + }, + { + "env": "Server", + "key": null, + "name": "Component", + "props": {}, + "stack": [ + [ + "Object.<anonymous>", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3692, + 19, + 3673, + 82, + ], + [ + "new Promise", + "", + 0, + 0, + 0, + 0, + ], + ], + }, + { + "time": 0, + }, + { + "awaited": { + "end": 0, + "env": "Server", + "name": "Component", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "props": {}, + "stack": [ + [ + "Object.<anonymous>", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3692, + 19, + 3673, + 82, + ], + [ + "new Promise", + "", + 0, + 0, + 0, + 0, + ], + ], + }, + "stack": [ + [ + "Component", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3676, + 25, + 3674, + 5, + ], + ], + "start": 0, + "value": { + "value": "hello", + }, + }, + "env": "Server", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "props": {}, + "stack": [ + [ + "Object.<anonymous>", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3692, + 19, + 3673, + 82, + ], + [ + "new Promise", + "", + 0, + 0, + 0, + 0, + ], + ], + }, + "stack": [ + [ + "Component", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3676, + 25, + 3674, + 5, + ], + ], + }, + { + "time": 0, + }, + { + "time": 0, + }, + { + "awaited": { + "end": 0, + "env": "Server", + "name": "Component", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "props": {}, + "stack": [ + [ + "Object.<anonymous>", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3692, + 19, + 3673, + 82, + ], + [ + "new Promise", + "", + 0, + 0, + 0, + 0, + ], + ], + }, + "stack": [ + [ + "Component", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3684, + 25, + 3674, + 5, + ], + ], + "start": 0, + "value": { + "value": "This string of length 1000001 has been omitted by React to avoid sending too much data from the server.", + }, + }, + "env": "Server", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "props": {}, + "stack": [ + [ + "Object.<anonymous>", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3692, + 19, + 3673, + 82, + ], + [ + "new Promise", + "", + 0, + 0, + 0, + 0, + ], + ], + }, + "stack": [ + [ + "Component", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 3684, + 25, + 3674, + 5, + ], + ], + }, + { + "time": 0, + }, + { + "time": 0, + }, + { + "awaited": { + "byteSize": 0, + "end": 0, + "name": "rsc stream", + "owner": null, + "start": 0, + "value": { + "value": "stream", + }, + }, + }, + ] + `); + } + }); }); diff --git a/packages/shared/ReactPerformanceTrackProperties.js b/packages/shared/ReactPerformanceTrackProperties.js index 29aba7282f04..72375a928e10 100644 --- a/packages/shared/ReactPerformanceTrackProperties.js +++ b/packages/shared/ReactPerformanceTrackProperties.js @@ -275,7 +275,11 @@ export function addValueToProperties( if (value === OMITTED_PROP_ERROR) { desc = '\u2026'; // ellipsis } else { - desc = JSON.stringify(value); + desc = JSON.stringify( + value.length >= 1024 + ? value.slice(0, 1023) + '\u2026' // ellipsis + : value, + ); } break; case 'undefined':