Skip to content

Commit b14f8da

Browse files
authored
refactor[devtools]: forbid editing class instances in props (#26522)
## Summary Fixes #24781 Restricting from editing props, which are class instances, because their internals should be opaque. Proposed changes: 1. Adding new data type `class_instance`: based on prototype chain of an object we will check if its plain or not. If not, then will be marked as `class_instance`. This should not affect `arrays`, ..., because we do this in the end of an `object` case in `getDataType` function. Important detail: this approach won't work for objects created with `Object.create`, because of the custom prototype. This can also be bypassed by manually deleting a prototype ¯\\\_(ツ)_/¯ I am not sure if there might be a better solution (which will cover all cases) to detect if object is a class instance. Initially I was trying to use `Object.getPrototypeOf(object) === Object.prototype`, but this won't work for cases when we are dealing with `iframe`. 2. Objects with a type `class_instance` will be marked as unserializable and read-only. ## Demo `person` is a class instance, `object` is a plain object https://user-images.githubusercontent.com/28902667/228914791-ebdc8ab0-eb5c-426d-8163-66d56b5e8790.mov
1 parent 7329ea8 commit b14f8da

File tree

5 files changed

+94
-5
lines changed

5 files changed

+94
-5
lines changed

fixtures/devtools/standalone/index.html

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,11 @@ <h1>List</h1>
334334
},
335335
});
336336

337+
class Foo {
338+
flag = false;
339+
object = {a: {b: {c: {d: 1}}}}
340+
}
341+
337342
function UnserializableProps() {
338343
return (
339344
<ChildComponent
@@ -343,6 +348,7 @@ <h1>List</h1>
343348
setOfSets={setOfSets}
344349
typedArray={typedArray}
345350
immutable={immutable}
351+
classInstance={new Foo()}
346352
/>
347353
);
348354
}

packages/react-devtools-shared/src/__tests__/utils-test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import {
1111
getDisplayName,
1212
getDisplayNameForReactElement,
13+
isPlainObject,
1314
} from 'react-devtools-shared/src/utils';
1415
import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
1516
import {
@@ -270,4 +271,30 @@ describe('utils', () => {
270271
expect(gte('10.0.0', '9.0.0')).toBe(true);
271272
});
272273
});
274+
275+
describe('isPlainObject', () => {
276+
it('should return true for plain objects', () => {
277+
expect(isPlainObject({})).toBe(true);
278+
expect(isPlainObject({a: 1})).toBe(true);
279+
expect(isPlainObject({a: {b: {c: 123}}})).toBe(true);
280+
});
281+
282+
it('should return false if object is a class instance', () => {
283+
expect(isPlainObject(new (class C {})())).toBe(false);
284+
});
285+
286+
it('should retun false for objects, which have not only Object in its prototype chain', () => {
287+
expect(isPlainObject([])).toBe(false);
288+
expect(isPlainObject(Symbol())).toBe(false);
289+
});
290+
291+
it('should retun false for primitives', () => {
292+
expect(isPlainObject(5)).toBe(false);
293+
expect(isPlainObject(true)).toBe(false);
294+
});
295+
296+
it('should return true for objects with no prototype', () => {
297+
expect(isPlainObject(Object.create(null))).toBe(true);
298+
});
299+
});
273300
});

packages/react-devtools-shared/src/hydration.js

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export type Unserializable = {
5252
size?: number,
5353
type: string,
5454
unserializable: boolean,
55-
...
55+
[string | number]: any,
5656
};
5757

5858
// This threshold determines the depth at which the bridge "dehydrates" nested data.
@@ -248,7 +248,6 @@ export function dehydrate(
248248
// Other types (e.g. typed arrays, Sets) will not spread correctly.
249249
Array.from(data).forEach(
250250
(item, i) =>
251-
// $FlowFixMe[prop-missing] Unserializable doesn't have an index signature
252251
(unserializableValue[i] = dehydrate(
253252
item,
254253
cleaned,
@@ -296,6 +295,7 @@ export function dehydrate(
296295

297296
case 'object':
298297
isPathAllowedCheck = isPathAllowed(path);
298+
299299
if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) {
300300
return createDehydrated(type, true, data, cleaned, path);
301301
} else {
@@ -316,15 +316,46 @@ export function dehydrate(
316316
return object;
317317
}
318318

319+
case 'class_instance':
320+
isPathAllowedCheck = isPathAllowed(path);
321+
322+
if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) {
323+
return createDehydrated(type, true, data, cleaned, path);
324+
}
325+
326+
const value: Unserializable = {
327+
unserializable: true,
328+
type,
329+
readonly: true,
330+
preview_short: formatDataForPreview(data, false),
331+
preview_long: formatDataForPreview(data, true),
332+
name: data.constructor.name,
333+
};
334+
335+
getAllEnumerableKeys(data).forEach(key => {
336+
const keyAsString = key.toString();
337+
338+
value[keyAsString] = dehydrate(
339+
data[key],
340+
cleaned,
341+
unserializable,
342+
path.concat([keyAsString]),
343+
isPathAllowed,
344+
isPathAllowedCheck ? 1 : level + 1,
345+
);
346+
});
347+
348+
unserializable.push(path);
349+
350+
return value;
351+
319352
case 'infinity':
320353
case 'nan':
321354
case 'undefined':
322355
// Some values are lossy when sent through a WebSocket.
323356
// We dehydrate+rehydrate them to preserve their type.
324357
cleaned.push(path);
325-
return {
326-
type,
327-
};
358+
return {type};
328359

329360
default:
330361
return data;

packages/react-devtools-shared/src/utils.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,7 @@ export type DataType =
534534
| 'array_buffer'
535535
| 'bigint'
536536
| 'boolean'
537+
| 'class_instance'
537538
| 'data_view'
538539
| 'date'
539540
| 'function'
@@ -620,6 +621,11 @@ export function getDataType(data: Object): DataType {
620621
return 'html_all_collection';
621622
}
622623
}
624+
625+
if (!isPlainObject(data)) {
626+
return 'class_instance';
627+
}
628+
623629
return 'object';
624630
case 'string':
625631
return 'string';
@@ -835,6 +841,8 @@ export function formatDataForPreview(
835841
}
836842
case 'date':
837843
return data.toString();
844+
case 'class_instance':
845+
return data.constructor.name;
838846
case 'object':
839847
if (showFormattedValue) {
840848
const keys = Array.from(getAllEnumerableKeys(data)).sort(alphaSortKeys);
@@ -873,3 +881,12 @@ export function formatDataForPreview(
873881
}
874882
}
875883
}
884+
885+
// Basically checking that the object only has Object in its prototype chain
886+
export const isPlainObject = (object: Object): boolean => {
887+
const objectPrototype = Object.getPrototypeOf(object);
888+
if (!objectPrototype) return true;
889+
890+
const objectParentPrototype = Object.getPrototypeOf(objectPrototype);
891+
return !objectParentPrototype;
892+
};

packages/react-devtools-shell/src/app/InspectableElements/UnserializableProps.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ const immutable = Immutable.fromJS({
3333
});
3434
const bigInt = BigInt(123); // eslint-disable-line no-undef
3535

36+
class Foo {
37+
flag = false;
38+
object: Object = {
39+
a: {b: {c: {d: 1}}},
40+
};
41+
}
42+
3643
export default function UnserializableProps(): React.Node {
3744
return (
3845
<ChildComponent
@@ -45,6 +52,7 @@ export default function UnserializableProps(): React.Node {
4552
typedArray={typedArray}
4653
immutable={immutable}
4754
bigInt={bigInt}
55+
classInstance={new Foo()}
4856
/>
4957
);
5058
}

0 commit comments

Comments
 (0)