From 748ab8a49727f92dac0bc353e821d40a73cda9a8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 11 Oct 2023 00:34:05 -0400 Subject: [PATCH 1/2] Enforce "simple object" rule in production --- .../src/ReactFlightReplyClient.js | 95 +++++++++++-------- .../src/__tests__/ReactFlight-test.js | 22 +++-- .../react-server/src/ReactFlightServer.js | 66 ++++++++----- packages/react/src/ReactTaint.js | 6 +- packages/shared/ReactSerializationErrors.js | 5 +- packages/shared/getPrototypeOf.js | 12 +++ scripts/error-codes/codes.json | 4 +- 7 files changed, 127 insertions(+), 83 deletions(-) create mode 100644 packages/shared/getPrototypeOf.js diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index 57989237481ed..ebfcf8f4f5476 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -29,6 +29,9 @@ import { } from 'shared/ReactSerializationErrors'; import isArray from 'shared/isArray'; +import getPrototypeOf from 'shared/getPrototypeOf'; + +const ObjectPrototype = Object.prototype; import {usedWithSSR} from './ReactFlightClientConfig'; @@ -263,54 +266,64 @@ export function processReply( formData.append(formFieldPrefix + setId, partJSON); return serializeSetID(setId); } - if (!isArray(value)) { - const iteratorFn = getIteratorFn(value); - if (iteratorFn) { - return Array.from((value: any)); - } + if (isArray(value)) { + // $FlowFixMe[incompatible-return] + return value; + } + const iteratorFn = getIteratorFn(value); + if (iteratorFn) { + return Array.from((value: any)); } + // Verify that this is a simple plain object. + const proto = getPrototypeOf(value); + if ( + proto !== ObjectPrototype && + (proto === null || getPrototypeOf(proto) !== null) + ) { + throw new Error( + 'Only plain objects, and a few built-ins, can be passed to Server Actions. ' + + 'Classes or null prototypes are not supported.', + ); + } if (__DEV__) { - if (value !== null && !isArray(value)) { - // Verify that this is a simple plain object. - if ((value: any).$$typeof === REACT_ELEMENT_TYPE) { - console.error( - 'React Element cannot be passed to Server Functions from the Client.%s', - describeObjectForErrorMessage(parent, key), - ); - } else if ((value: any).$$typeof === REACT_LAZY_TYPE) { - console.error( - 'React Lazy cannot be passed to Server Functions from the Client.%s', - describeObjectForErrorMessage(parent, key), - ); - } else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) { - console.error( - 'React Context Providers cannot be passed to Server Functions from the Client.%s', - describeObjectForErrorMessage(parent, key), - ); - } else if (objectName(value) !== 'Object') { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - '%s objects are not supported.%s', - objectName(value), - describeObjectForErrorMessage(parent, key), - ); - } else if (!isSimpleObject(value)) { + if ((value: any).$$typeof === REACT_ELEMENT_TYPE) { + console.error( + 'React Element cannot be passed to Server Functions from the Client.%s', + describeObjectForErrorMessage(parent, key), + ); + } else if ((value: any).$$typeof === REACT_LAZY_TYPE) { + console.error( + 'React Lazy cannot be passed to Server Functions from the Client.%s', + describeObjectForErrorMessage(parent, key), + ); + } else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) { + console.error( + 'React Context Providers cannot be passed to Server Functions from the Client.%s', + describeObjectForErrorMessage(parent, key), + ); + } else if (objectName(value) !== 'Object') { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + '%s objects are not supported.%s', + objectName(value), + describeObjectForErrorMessage(parent, key), + ); + } else if (!isSimpleObject(value)) { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + 'Classes or other objects with methods are not supported.%s', + describeObjectForErrorMessage(parent, key), + ); + } else if (Object.getOwnPropertySymbols) { + const symbols = Object.getOwnPropertySymbols(value); + if (symbols.length > 0) { console.error( 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Classes or other objects with methods are not supported.%s', + 'Objects with symbol properties like %s are not supported.%s', + symbols[0].description, describeObjectForErrorMessage(parent, key), ); - } else if (Object.getOwnPropertySymbols) { - const symbols = Object.getOwnPropertySymbols(value); - if (symbols.length > 0) { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Objects with symbol properties like %s are not supported.%s', - symbols[0].description, - describeObjectForErrorMessage(parent, key), - ); - } } } } diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index c0f3d2bbb0b04..c68630e5b23e5 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -988,19 +988,21 @@ describe('ReactFlight', () => { ReactNoopFlightClient.read(transport); }); - it('should warn in DEV if a class instance is passed to a host component', () => { + it('should error if a class instance is passed to a host component', () => { class Foo { method() {} } - expect(() => { - const transport = ReactNoopFlightServer.render( - , - ); - ReactNoopFlightClient.read(transport); - }).toErrorDev( - 'Only plain objects can be passed to Client Components from Server Components. ', - {withoutStack: true}, - ); + const errors = []; + ReactNoopFlightServer.render(, { + onError(x) { + errors.push(x.message); + }, + }); + + expect(errors).toEqual([ + 'Only plain objects, and a few built-ins, can be passed to Client Components ' + + 'from Server Components. Classes or null prototypes are not supported.', + ]); }); it('should warn in DEV if a a client reference is passed to useContext()', () => { diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index e8d120b952f1e..4bff817e4d5e3 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -111,10 +111,13 @@ import {getOrCreateServerContext} from 'shared/ReactServerContextRegistry'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import ReactServerSharedInternals from './ReactServerSharedInternals'; import isArray from 'shared/isArray'; +import getPrototypeOf from 'shared/getPrototypeOf'; import binaryToComparableString from 'shared/binaryToComparableString'; import {SuspenseException, getSuspendedThenable} from './ReactFlightThenable'; +const ObjectPrototype = Object.prototype; + type JSONValue = | string | boolean @@ -1107,39 +1110,50 @@ function resolveModelToJSON( } } - if (!isArray(value)) { - const iteratorFn = getIteratorFn(value); - if (iteratorFn) { - return Array.from((value: any)); - } + if (isArray(value)) { + // $FlowFixMe[incompatible-return] + return value; + } + + const iteratorFn = getIteratorFn(value); + if (iteratorFn) { + return Array.from((value: any)); } + // Verify that this is a simple plain object. + const proto = getPrototypeOf(value); + if ( + proto !== ObjectPrototype && + (proto === null || getPrototypeOf(proto) !== null) + ) { + throw new Error( + 'Only plain objects, and a few built-ins, can be passed to Client Components ' + + 'from Server Components. Classes or null prototypes are not supported.', + ); + } if (__DEV__) { - if (value !== null && !isArray(value)) { - // Verify that this is a simple plain object. - if (objectName(value) !== 'Object') { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - '%s objects are not supported.%s', - objectName(value), - describeObjectForErrorMessage(parent, key), - ); - } else if (!isSimpleObject(value)) { + if (objectName(value) !== 'Object') { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + '%s objects are not supported.%s', + objectName(value), + describeObjectForErrorMessage(parent, key), + ); + } else if (!isSimpleObject(value)) { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + 'Classes or other objects with methods are not supported.%s', + describeObjectForErrorMessage(parent, key), + ); + } else if (Object.getOwnPropertySymbols) { + const symbols = Object.getOwnPropertySymbols(value); + if (symbols.length > 0) { console.error( 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Classes or other objects with methods are not supported.%s', + 'Objects with symbol properties like %s are not supported.%s', + symbols[0].description, describeObjectForErrorMessage(parent, key), ); - } else if (Object.getOwnPropertySymbols) { - const symbols = Object.getOwnPropertySymbols(value); - if (symbols.length > 0) { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Objects with symbol properties like %s are not supported.%s', - symbols[0].description, - describeObjectForErrorMessage(parent, key), - ); - } } } } diff --git a/packages/react/src/ReactTaint.js b/packages/react/src/ReactTaint.js index d9e532737be6e..399acb8f01431 100644 --- a/packages/react/src/ReactTaint.js +++ b/packages/react/src/ReactTaint.js @@ -9,6 +9,8 @@ import {enableTaint, enableBinaryFlight} from 'shared/ReactFeatureFlags'; +import getPrototypeOf from 'shared/getPrototypeOf'; + import binaryToComparableString from 'shared/binaryToComparableString'; import ReactServerSharedInternals from './ReactServerSharedInternals'; @@ -22,9 +24,7 @@ const { interface Reference {} // This is the shared constructor of all typed arrays. -const TypedArrayConstructor = Object.getPrototypeOf( - Uint32Array.prototype, -).constructor; +const TypedArrayConstructor = getPrototypeOf(Uint32Array.prototype).constructor; const defaultMessage = 'A tainted value was attempted to be serialized to a Client Component or Action closure. ' + diff --git a/packages/shared/ReactSerializationErrors.js b/packages/shared/ReactSerializationErrors.js index a4875007791f5..e0def3e1c657d 100644 --- a/packages/shared/ReactSerializationErrors.js +++ b/packages/shared/ReactSerializationErrors.js @@ -19,6 +19,7 @@ import { import type {LazyComponent} from 'react/src/ReactLazy'; import isArray from 'shared/isArray'; +import getPrototypeOf from 'shared/getPrototypeOf'; // Used for DEV messages to keep track of which parent rendered some props, // in case they error. @@ -35,7 +36,7 @@ function isObjectPrototype(object: any): boolean { } // It might be an object from a different Realm which is // still just a plain simple object. - if (Object.getPrototypeOf(object)) { + if (getPrototypeOf(object)) { return false; } const names = Object.getOwnPropertyNames(object); @@ -48,7 +49,7 @@ function isObjectPrototype(object: any): boolean { } export function isSimpleObject(object: any): boolean { - if (!isObjectPrototype(Object.getPrototypeOf(object))) { + if (!isObjectPrototype(getPrototypeOf(object))) { return false; } const names = Object.getOwnPropertyNames(object); diff --git a/packages/shared/getPrototypeOf.js b/packages/shared/getPrototypeOf.js new file mode 100644 index 0000000000000..2137bdc1c3947 --- /dev/null +++ b/packages/shared/getPrototypeOf.js @@ -0,0 +1,12 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +const getPrototypeOf = Object.getPrototypeOf; + +export default getPrototypeOf; diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 851c058cf1b2d..b01d1150b2287 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -482,5 +482,7 @@ "494": "taintUniqueValue cannot taint objects or functions. Try taintObjectReference instead.", "495": "Cannot taint a %s because the value is too general and not unique enough to block globally.", "496": "Only objects or functions can be passed to taintObjectReference. Try taintUniqueValue instead.", - "497": "Only objects or functions can be passed to taintObjectReference." + "497": "Only objects or functions can be passed to taintObjectReference.", + "498": "Only plain objects, and a few built-ins, can be passed to Client Components from Server Components. Classes or null prototypes are not supported.", + "499": "Only plain objects, and a few built-ins, can be passed to Server Actions. Classes or null prototypes are not supported." } \ No newline at end of file From ff89f2b207157f86ccf08bcbe42f0bafe7756b9e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 11 Oct 2023 10:38:56 -0400 Subject: [PATCH 2/2] Move isArray check higher It's much more likely to be an array than Map or TypedArray. --- packages/react-client/src/ReactFlightReplyClient.js | 8 ++++---- packages/react-server/src/ReactFlightServer.js | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index ebfcf8f4f5476..dc20eb55d2c1b 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -230,6 +230,10 @@ export function processReply( ); return serializePromiseID(promiseId); } + if (isArray(value)) { + // $FlowFixMe[incompatible-return] + return value; + } // TODO: Should we the Object.prototype.toString.call() to test for cross-realm objects? if (value instanceof FormData) { if (formData === null) { @@ -266,10 +270,6 @@ export function processReply( formData.append(formFieldPrefix + setId, partJSON); return serializeSetID(setId); } - if (isArray(value)) { - // $FlowFixMe[incompatible-return] - return value; - } const iteratorFn = getIteratorFn(value); if (iteratorFn) { return Array.from((value: any)); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 4bff817e4d5e3..6c08dd9948838 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1049,6 +1049,11 @@ function resolveModelToJSON( return (undefined: any); } + if (isArray(value)) { + // $FlowFixMe[incompatible-return] + return value; + } + if (value instanceof Map) { return serializeMap(request, value); } @@ -1110,11 +1115,6 @@ function resolveModelToJSON( } } - if (isArray(value)) { - // $FlowFixMe[incompatible-return] - return value; - } - const iteratorFn = getIteratorFn(value); if (iteratorFn) { return Array.from((value: any));