Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor[devtools]: forbid editing class instances in props #26522

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions fixtures/devtools/standalone/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,11 @@ <h1>List</h1>
},
});

class Foo {
flag = false;
object = {a: {b: {c: {d: 1}}}}
}

function UnserializableProps() {
return (
<ChildComponent
Expand All @@ -343,6 +348,7 @@ <h1>List</h1>
setOfSets={setOfSets}
typedArray={typedArray}
immutable={immutable}
classInstance={new Foo()}
/>
);
}
Expand Down
27 changes: 27 additions & 0 deletions packages/react-devtools-shared/src/__tests__/utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import {
getDisplayName,
getDisplayNameForReactElement,
isPlainObject,
} from 'react-devtools-shared/src/utils';
import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
import {
Expand Down Expand Up @@ -270,4 +271,30 @@ describe('utils', () => {
expect(gte('10.0.0', '9.0.0')).toBe(true);
});
});

describe('isPlainObject', () => {
it('should return true for plain objects', () => {
expect(isPlainObject({})).toBe(true);
expect(isPlainObject({a: 1})).toBe(true);
expect(isPlainObject({a: {b: {c: 123}}})).toBe(true);
});

it('should return false if object is a class instance', () => {
expect(isPlainObject(new (class C {})())).toBe(false);
});

it('should retun false for objects, which have not only Object in its prototype chain', () => {
expect(isPlainObject([])).toBe(false);
expect(isPlainObject(Symbol())).toBe(false);
});

it('should retun false for primitives', () => {
expect(isPlainObject(5)).toBe(false);
expect(isPlainObject(true)).toBe(false);
});

it('should return true for objects with no prototype', () => {
expect(isPlainObject(Object.create(null))).toBe(true);
});
});
});
41 changes: 36 additions & 5 deletions packages/react-devtools-shared/src/hydration.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export type Unserializable = {
size?: number,
type: string,
unserializable: boolean,
...
[string | number]: any,
};

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

case 'object':
isPathAllowedCheck = isPathAllowed(path);

if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) {
return createDehydrated(type, true, data, cleaned, path);
} else {
Expand All @@ -316,15 +316,46 @@ export function dehydrate(
return object;
}

case 'class_instance':
isPathAllowedCheck = isPathAllowed(path);

if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) {
return createDehydrated(type, true, data, cleaned, path);
}

const value: Unserializable = {
unserializable: true,
type,
readonly: true,
preview_short: formatDataForPreview(data, false),
preview_long: formatDataForPreview(data, true),
name: data.constructor.name,
};

getAllEnumerableKeys(data).forEach(key => {
const keyAsString = key.toString();

value[keyAsString] = dehydrate(
data[key],
cleaned,
unserializable,
path.concat([keyAsString]),
isPathAllowed,
isPathAllowedCheck ? 1 : level + 1,
);
});

unserializable.push(path);

return value;

case 'infinity':
case 'nan':
case 'undefined':
// Some values are lossy when sent through a WebSocket.
// We dehydrate+rehydrate them to preserve their type.
cleaned.push(path);
return {
type,
};
return {type};

default:
return data;
Expand Down
17 changes: 17 additions & 0 deletions packages/react-devtools-shared/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ export type DataType =
| 'array_buffer'
| 'bigint'
| 'boolean'
| 'class_instance'
| 'data_view'
| 'date'
| 'function'
Expand Down Expand Up @@ -620,6 +621,11 @@ export function getDataType(data: Object): DataType {
return 'html_all_collection';
}
}

if (!isPlainObject(data)) {
return 'class_instance';
}

return 'object';
case 'string':
return 'string';
Expand Down Expand Up @@ -835,6 +841,8 @@ export function formatDataForPreview(
}
case 'date':
return data.toString();
case 'class_instance':
return data.constructor.name;
case 'object':
if (showFormattedValue) {
const keys = Array.from(getAllEnumerableKeys(data)).sort(alphaSortKeys);
Expand Down Expand Up @@ -873,3 +881,12 @@ export function formatDataForPreview(
}
}
}

// Basically checking that the object only has Object in its prototype chain
export const isPlainObject = (object: Object): boolean => {
const objectPrototype = Object.getPrototypeOf(object);
if (!objectPrototype) return true;

const objectParentPrototype = Object.getPrototypeOf(objectPrototype);
return !objectParentPrototype;
};
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ const immutable = Immutable.fromJS({
});
const bigInt = BigInt(123); // eslint-disable-line no-undef

class Foo {
flag = false;
object: Object = {
a: {b: {c: {d: 1}}},
};
}

export default function UnserializableProps(): React.Node {
return (
<ChildComponent
Expand All @@ -45,6 +52,7 @@ export default function UnserializableProps(): React.Node {
typedArray={typedArray}
immutable={immutable}
bigInt={bigInt}
classInstance={new Foo()}
/>
);
}
Expand Down