From 76a6dbcb9ad93cf1e18606cb9280a1afc206b153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 10 Nov 2020 22:56:50 -0500 Subject: [PATCH] =?UTF-8?q?[Flight]=20Encode=20Symbols=20as=20special=20ro?= =?UTF-8?q?ws=20that=20can=20be=20referenced=20by=20models=20=E2=80=A6=20(?= =?UTF-8?q?#20171)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Encode Symbols as special rows that can be referenced by models If a symbol was extracted from Symbol.for(...) then we can reliably recreate the same symbol on the client. S123:"react.suspense" M456:{mySymbol: '$123'} This doesn't suffer from the XSS problem because you have to write actual code to create one of these symbols. That problem is only a problem because values pass through common other usages of JSON which are not secure. Since React encodes its built-ins as symbols, we can now use them as long as its props are serializable. Like Suspense. * Refactor resolution to avoid memo hack Going through createElement isn't quite equivalent for ref and key in props. * Reuse symbol ids that have already been written earlier in the stream --- .../react-client/src/ReactFlightClient.js | 18 ++++ .../src/ReactFlightClientStream.js | 27 +++--- .../src/__tests__/ReactFlight-test.js | 2 +- .../react-server/src/ReactFlightServer.js | 90 +++++++++++-------- .../src/ReactFlightServerConfigStream.js | 10 +++ .../src/ReactFlightDOMRelayClient.js | 4 + .../src/ReactFlightDOMRelayProtocol.js | 1 + .../ReactFlightDOMRelayServerHostConfig.js | 8 ++ .../ReactFlightDOMRelay-test.internal.js | 13 +-- .../src/__tests__/ReactFlightDOM-test.js | 17 ++-- .../src/ReactFlightNativeRelayClient.js | 4 + .../src/ReactFlightNativeRelayProtocol.js | 1 + .../ReactFlightNativeRelayServerHostConfig.js | 8 ++ scripts/error-codes/codes.json | 2 +- 14 files changed, 140 insertions(+), 65 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 67e6ee9bf374f..983e3edc93802 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -132,6 +132,13 @@ function createErrorChunk(response: Response, error: Error): ErroredChunk { return new Chunk(ERRORED, error, response); } +function createInitializedChunk( + response: Response, + value: T, +): InitializedChunk { + return new Chunk(INITIALIZED, value, response); +} + function wakeChunk(listeners: null | Array<() => mixed>) { if (listeners !== null) { for (let i = 0; i < listeners.length; i++) { @@ -373,6 +380,17 @@ export function resolveModule( } } +export function resolveSymbol( + response: Response, + id: number, + name: string, +): void { + const chunks = response._chunks; + // We assume that we'll always emit the symbol before anything references it + // to save a few bytes. + chunks.set(id, createInitializedChunk(response, Symbol.for(name))); +} + export function resolveError( response: Response, id: number, diff --git a/packages/react-client/src/ReactFlightClientStream.js b/packages/react-client/src/ReactFlightClientStream.js index 4bb493a66ddb1..9f07d8cc999ad 100644 --- a/packages/react-client/src/ReactFlightClientStream.js +++ b/packages/react-client/src/ReactFlightClientStream.js @@ -12,6 +12,7 @@ import type {Response} from './ReactFlightClientHostConfigStream'; import { resolveModule, resolveModel, + resolveSymbol, resolveError, createResponse as createResponseBase, parseModelString, @@ -32,26 +33,28 @@ function processFullRow(response: Response, row: string): void { return; } const tag = row[0]; + // When tags that are not text are added, check them here before + // parsing the row as text. + // switch (tag) { + // } + const colon = row.indexOf(':', 1); + const id = parseInt(row.substring(1, colon), 16); + const text = row.substring(colon + 1); switch (tag) { case 'J': { - const colon = row.indexOf(':', 1); - const id = parseInt(row.substring(1, colon), 16); - const json = row.substring(colon + 1); - resolveModel(response, id, json); + resolveModel(response, id, text); return; } case 'M': { - const colon = row.indexOf(':', 1); - const id = parseInt(row.substring(1, colon), 16); - const json = row.substring(colon + 1); - resolveModule(response, id, json); + resolveModule(response, id, text); + return; + } + case 'S': { + resolveSymbol(response, id, JSON.parse(text)); return; } case 'E': { - const colon = row.indexOf(':', 1); - const id = parseInt(row.substring(1, colon), 16); - const json = row.substring(colon + 1); - const errorInfo = JSON.parse(json); + const errorInfo = JSON.parse(text); resolveError(response, id, errorInfo.message, errorInfo.stack); return; } diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 3d4ccf5c459f1..e0947c68811d2 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -174,7 +174,7 @@ describe('ReactFlight', () => { - + diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 56842b7b2f40c..34404544efbdc 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -25,6 +25,7 @@ import { close, processModelChunk, processModuleChunk, + processSymbolChunk, processErrorChunk, resolveModuleMetaData, isModuleReference, @@ -32,21 +33,12 @@ import { import { REACT_ELEMENT_TYPE, - REACT_DEBUG_TRACING_MODE_TYPE, REACT_FORWARD_REF_TYPE, REACT_FRAGMENT_TYPE, REACT_LAZY_TYPE, - REACT_LEGACY_HIDDEN_TYPE, REACT_MEMO_TYPE, - REACT_OFFSCREEN_TYPE, - REACT_PROFILER_TYPE, - REACT_SCOPE_TYPE, - REACT_STRICT_MODE_TYPE, - REACT_SUSPENSE_TYPE, - REACT_SUSPENSE_LIST_TYPE, } from 'shared/ReactSymbols'; -import * as React from 'react'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -86,6 +78,7 @@ export type Request = { completedModuleChunks: Array, completedJSONChunks: Array, completedErrorChunks: Array, + writtenSymbols: Map, flowing: boolean, toJSON: (key: string, value: ReactModel) => ReactJSONValue, }; @@ -107,6 +100,7 @@ export function createRequest( completedModuleChunks: [], completedJSONChunks: [], completedErrorChunks: [], + writtenSymbols: new Map(), flowing: false, toJSON: function(key: string, value: ReactModel): ReactJSONValue { return resolveModelToJSON(request, this, key, value); @@ -118,10 +112,13 @@ export function createRequest( return request; } -function attemptResolveElement(element: React$Element): ReactModel { - const type = element.type; - const props = element.props; - if (element.ref !== null && element.ref !== undefined) { +function attemptResolveElement( + type: any, + key: null | React$Key, + ref: mixed, + props: any, +): ReactModel { + if (ref !== null && ref !== undefined) { // When the ref moves to the regular props object this will implicitly // throw for functions. We could probably relax it to a DEV warning for other // cases. @@ -135,25 +132,22 @@ function attemptResolveElement(element: React$Element): ReactModel { return type(props); } else if (typeof type === 'string') { // This is a host element. E.g. HTML. - return [REACT_ELEMENT_TYPE, type, element.key, element.props]; - } else if ( - type === REACT_FRAGMENT_TYPE || - type === REACT_STRICT_MODE_TYPE || - type === REACT_PROFILER_TYPE || - type === REACT_SCOPE_TYPE || - type === REACT_DEBUG_TRACING_MODE_TYPE || - type === REACT_LEGACY_HIDDEN_TYPE || - type === REACT_OFFSCREEN_TYPE || - // TODO: These are temporary shims - // and we'll want a different behavior. - type === REACT_SUSPENSE_TYPE || - type === REACT_SUSPENSE_LIST_TYPE - ) { - return element.props.children; + return [REACT_ELEMENT_TYPE, type, key, props]; + } else if (typeof type === 'symbol') { + if (type === REACT_FRAGMENT_TYPE) { + // For key-less fragments, we add a small optimization to avoid serializing + // it as a wrapper. + // TODO: If a key is specified, we should propagate its key to any children. + // Same as if a server component has a key. + return props.children; + } + // This might be a built-in React component. We'll let the client decide. + // Any built-in works as long as its props are serializable. + return [REACT_ELEMENT_TYPE, type, key, props]; } else if (type != null && typeof type === 'object') { if (isModuleReference(type)) { // This is a reference to a client component. - return [REACT_ELEMENT_TYPE, type, element.key, element.props]; + return [REACT_ELEMENT_TYPE, type, key, props]; } switch (type.$$typeof) { case REACT_FORWARD_REF_TYPE: { @@ -161,8 +155,7 @@ function attemptResolveElement(element: React$Element): ReactModel { return render(props, undefined); } case REACT_MEMO_TYPE: { - const nextChildren = React.createElement(type.type, element.props); - return attemptResolveElement(nextChildren); + return attemptResolveElement(type.type, key, ref, props); } } } @@ -399,7 +392,12 @@ export function resolveModelToJSON( const element: React$Element = (value: any); try { // Attempt to render the server component. - value = attemptResolveElement(element); + value = attemptResolveElement( + element.type, + element.key, + element.ref, + element.props, + ); } catch (x) { if (typeof x === 'object' && x !== null && typeof x.then === 'function') { // Something suspended, we'll need to create a new segment and resolve it later. @@ -526,14 +524,26 @@ export function resolveModelToJSON( } if (typeof value === 'symbol') { + const writtenSymbols = request.writtenSymbols; + const existingId = writtenSymbols.get(value); + if (existingId !== undefined) { + return serializeByValueID(existingId); + } + const name = value.description; invariant( - false, - 'Symbol values (%s) cannot be passed to client components. ' + + Symbol.for(name) === value, + 'Only global symbols received from Symbol.for(...) can be passed to client components. ' + + 'The symbol Symbol.for(%s) cannot be found among global symbols. ' + 'Remove %s from this object, or avoid the entire object: %s', value.description, describeKeyForErrorMessage(key), describeObjectForErrorMessage(parent), ); + request.pendingChunks++; + const symbolId = request.nextChunkId++; + emitSymbolChunk(request, symbolId, name); + writtenSymbols.set(value, symbolId); + return serializeByValueID(symbolId); } // $FlowFixMe: bigint isn't added to Flow yet. @@ -588,6 +598,11 @@ function emitModuleChunk( request.completedModuleChunks.push(processedChunk); } +function emitSymbolChunk(request: Request, id: number, name: string): void { + const processedChunk = processSymbolChunk(request, id, name); + request.completedModuleChunks.push(processedChunk); +} + function retrySegment(request: Request, segment: Segment): void { const query = segment.query; let value; @@ -604,7 +619,12 @@ function retrySegment(request: Request, segment: Segment): void { // Doing this here lets us reuse this same segment if the next component // also suspends. segment.query = () => value; - value = attemptResolveElement(element); + value = attemptResolveElement( + element.type, + element.key, + element.ref, + element.props, + ); } const processedChunk = processModelChunk(request, segment.id, value); request.completedJSONChunks.push(processedChunk); diff --git a/packages/react-server/src/ReactFlightServerConfigStream.js b/packages/react-server/src/ReactFlightServerConfigStream.js index 01c0dea4c0c0e..12241493e57fa 100644 --- a/packages/react-server/src/ReactFlightServerConfigStream.js +++ b/packages/react-server/src/ReactFlightServerConfigStream.js @@ -109,6 +109,16 @@ export function processModuleChunk( return convertStringToBuffer(row); } +export function processSymbolChunk( + request: Request, + id: number, + name: string, +): Chunk { + const json = stringify(name); + const row = serializeRowHeader('S', id) + json + '\n'; + return convertStringToBuffer(row); +} + export { scheduleWork, flushBuffered, diff --git a/packages/react-transport-dom-relay/src/ReactFlightDOMRelayClient.js b/packages/react-transport-dom-relay/src/ReactFlightDOMRelayClient.js index b42f52097f547..6ea0b861aa7a9 100644 --- a/packages/react-transport-dom-relay/src/ReactFlightDOMRelayClient.js +++ b/packages/react-transport-dom-relay/src/ReactFlightDOMRelayClient.js @@ -15,6 +15,7 @@ import { createResponse, resolveModel, resolveModule, + resolveSymbol, resolveError, close, } from 'react-client/src/ReactFlightClient'; @@ -26,6 +27,9 @@ export function resolveRow(response: Response, chunk: RowEncoding): void { resolveModel(response, chunk[1], chunk[2]); } else if (chunk[0] === 'M') { resolveModule(response, chunk[1], chunk[2]); + } else if (chunk[0] === 'S') { + // $FlowFixMe: Flow doesn't support disjoint unions on tuples. + resolveSymbol(response, chunk[1], chunk[2]); } else { // $FlowFixMe: Flow doesn't support disjoint unions on tuples. resolveError(response, chunk[1], chunk[2].message, chunk[2].stack); diff --git a/packages/react-transport-dom-relay/src/ReactFlightDOMRelayProtocol.js b/packages/react-transport-dom-relay/src/ReactFlightDOMRelayProtocol.js index 263161d53852d..94f73b0d7a4ac 100644 --- a/packages/react-transport-dom-relay/src/ReactFlightDOMRelayProtocol.js +++ b/packages/react-transport-dom-relay/src/ReactFlightDOMRelayProtocol.js @@ -20,6 +20,7 @@ export type JSONValue = export type RowEncoding = | ['J', number, JSONValue] | ['M', number, ModuleMetaData] + | ['S', number, string] | [ 'E', number, diff --git a/packages/react-transport-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js b/packages/react-transport-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js index d3fc134176a96..70c2e957596cf 100644 --- a/packages/react-transport-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js +++ b/packages/react-transport-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js @@ -111,6 +111,14 @@ export function processModuleChunk( return ['M', id, moduleMetaData]; } +export function processSymbolChunk( + request: Request, + id: number, + name: string, +): Chunk { + return ['S', id, name]; +} + export function scheduleWork(callback: () => void) { callback(); } diff --git a/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js b/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js index 092e12abac9d0..fd0d327fc8e68 100644 --- a/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js +++ b/packages/react-transport-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js @@ -153,11 +153,14 @@ describe('ReactFlightDOMRelay', () => { foo: { bar: (
- {'Fragment child'} - {'Profiler child'} - {'StrictMode child'} - {'Suspense child'} - {['SuspenseList row 1', 'SuspenseList row 2']} + Fragment child + Profiler child + StrictMode child + Suspense child + + {'SuspenseList row 1'} + {'SuspenseList row 2'} +
Hello world
), diff --git a/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 04910ff98e615..d8568c661c2af 100644 --- a/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-transport-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -282,10 +282,6 @@ describe('ReactFlightDOM', () => { ); } - function Placeholder({children, fallback}) { - return {children}; - } - // Model function Text({children}) { return children; @@ -347,22 +343,21 @@ describe('ReactFlightDOM', () => { } const MyErrorBoundaryClient = moduleReference(MyErrorBoundary); - const PlaceholderClient = moduleReference(Placeholder); function ProfileContent() { return ( <> :avatar:} /> - (loading sidebar)

}> + (loading sidebar)

}> :friends:} /> - - (loading posts)

}> + + (loading posts)

}> :posts:} /> - + - (loading games)

}> + (loading games)

}> :games:} /> - + ); diff --git a/packages/react-transport-native-relay/src/ReactFlightNativeRelayClient.js b/packages/react-transport-native-relay/src/ReactFlightNativeRelayClient.js index 94222c0ca7136..42b9efa73b471 100644 --- a/packages/react-transport-native-relay/src/ReactFlightNativeRelayClient.js +++ b/packages/react-transport-native-relay/src/ReactFlightNativeRelayClient.js @@ -15,6 +15,7 @@ import { createResponse, resolveModel, resolveModule, + resolveSymbol, resolveError, close, } from 'react-client/src/ReactFlightClient'; @@ -26,6 +27,9 @@ export function resolveRow(response: Response, chunk: RowEncoding): void { resolveModel(response, chunk[1], chunk[2]); } else if (chunk[0] === 'M') { resolveModule(response, chunk[1], chunk[2]); + } else if (chunk[0] === 'S') { + // $FlowFixMe: Flow doesn't support disjoint unions on tuples. + resolveSymbol(response, chunk[1], chunk[2]); } else { // $FlowFixMe: Flow doesn't support disjoint unions on tuples. resolveError(response, chunk[1], chunk[2].message, chunk[2].stack); diff --git a/packages/react-transport-native-relay/src/ReactFlightNativeRelayProtocol.js b/packages/react-transport-native-relay/src/ReactFlightNativeRelayProtocol.js index 5689f8c0f974c..75f6db8039ab0 100644 --- a/packages/react-transport-native-relay/src/ReactFlightNativeRelayProtocol.js +++ b/packages/react-transport-native-relay/src/ReactFlightNativeRelayProtocol.js @@ -20,6 +20,7 @@ export type JSONValue = export type RowEncoding = | ['J', number, JSONValue] | ['M', number, ModuleMetaData] + | ['S', number, string] | [ 'E', number, diff --git a/packages/react-transport-native-relay/src/ReactFlightNativeRelayServerHostConfig.js b/packages/react-transport-native-relay/src/ReactFlightNativeRelayServerHostConfig.js index bd28ee0337521..c8566c960f0de 100644 --- a/packages/react-transport-native-relay/src/ReactFlightNativeRelayServerHostConfig.js +++ b/packages/react-transport-native-relay/src/ReactFlightNativeRelayServerHostConfig.js @@ -111,6 +111,14 @@ export function processModuleChunk( return ['M', id, moduleMetaData]; } +export function processSymbolChunk( + request: Request, + id: number, + name: string, +): Chunk { + return ['S', id, name]; +} + export function scheduleWork(callback: () => void) { callback(); } diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index a86c8bc32812c..269b7d2787727 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -364,7 +364,7 @@ "373": "This Hook is not supported in Server Components.", "374": "Event handlers cannot be passed to client component props. Remove %s from these props if possible: %s\nIf you need interactivity, consider converting part of this to a client component.", "375": "Functions cannot be passed directly to client components because they're not serializable. Remove %s (%s) from this object, or avoid the entire object: %s", - "376": "Symbol values (%s) cannot be passed to client components. Remove %s from this object, or avoid the entire object: %s", + "376": "Only global symbols received from Symbol.for(...) can be passed to client components. The symbol Symbol.for(%s) cannot be found among global symbols. Remove %s from this object, or avoid the entire object: %s", "377": "BigInt (%s) is not yet supported in client component props. Remove %s from this object or use a plain number instead: %s", "378": "Type %s is not supported in client component props. Remove %s from this object, or avoid the entire object: %s", "379": "Refs cannot be used in server components, nor passed to client components."