Skip to content

Commit 7e2ab87

Browse files
author
Brian Vaughn
authored
DevTools: Replaced unsafe hasOwnProperty() calls (facebook#17768)
DevTools previously called in several places with user-defined values. This could lead to runtime errors if those values had an overriden attribute. This commit replaces those callse with instead. New test cases have been added.
1 parent 5d3d71b commit 7e2ab87

File tree

10 files changed

+266
-45
lines changed

10 files changed

+266
-45
lines changed

‎packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -510,11 +510,6 @@ exports[`InspectedElementContext should support complex data types: 1: Inspected
510510
"object_of_objects": {
511511
"inner": {}
512512
},
513-
"object_with_null_proto": {
514-
"string": "abc",
515-
"number": 123,
516-
"boolean": true
517-
},
518513
"react_element": {},
519514
"regexp": {},
520515
"set": {
@@ -552,6 +547,39 @@ exports[`InspectedElementContext should support custom objects with enumerable p
552547
}
553548
`;
554549

550+
exports[`InspectedElementContext should support objects with no prototype: 1: Inspected element 2 1`] = `
551+
{
552+
"id": 2,
553+
"owners": null,
554+
"context": null,
555+
"hooks": null,
556+
"props": {
557+
"object": {
558+
"string": "abc",
559+
"number": 123,
560+
"boolean": true
561+
}
562+
},
563+
"state": null
564+
}
565+
`;
566+
567+
exports[`InspectedElementContext should support objects with overridden hasOwnProperty: 1: Inspected element 2 1`] = `
568+
{
569+
"id": 2,
570+
"owners": null,
571+
"context": null,
572+
"hooks": null,
573+
"props": {
574+
"object": {
575+
"name": "blah",
576+
"hasOwnProperty": true
577+
}
578+
},
579+
"state": null
580+
}
581+
`;
582+
555583
exports[`InspectedElementContext should support simple data types: 1: Initial inspection 1`] = `
556584
{
557585
"id": 2,

‎packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js‎

Lines changed: 95 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -537,10 +537,6 @@ describe('InspectedElementContext', () => {
537537
xyz: 1,
538538
},
539539
});
540-
const objectWithNullProto = Object.create(null);
541-
objectWithNullProto.string = 'abc';
542-
objectWithNullProto.number = 123;
543-
objectWithNullProto.boolean = true;
544540

545541
const container = document.createElement('div');
546542
await utils.actAsync(() =>
@@ -558,7 +554,6 @@ describe('InspectedElementContext', () => {
558554
map={mapShallow}
559555
map_of_maps={mapOfMaps}
560556
object_of_objects={objectOfObjects}
561-
object_with_null_proto={objectWithNullProto}
562557
react_element={<span />}
563558
regexp={/abc/giu}
564559
set={setShallow}
@@ -609,7 +604,6 @@ describe('InspectedElementContext', () => {
609604
map,
610605
map_of_maps,
611606
object_of_objects,
612-
object_with_null_proto,
613607
react_element,
614608
regexp,
615609
set,
@@ -701,12 +695,6 @@ describe('InspectedElementContext', () => {
701695
);
702696
expect(object_of_objects.inner[meta.preview_short]).toBe('{…}');
703697

704-
expect(object_with_null_proto).toEqual({
705-
boolean: true,
706-
number: 123,
707-
string: 'abc',
708-
});
709-
710698
expect(react_element[meta.inspectable]).toBe(false);
711699
expect(react_element[meta.name]).toBe('span');
712700
expect(react_element[meta.type]).toBe('react_element');
@@ -753,6 +741,101 @@ describe('InspectedElementContext', () => {
753741
done();
754742
});
755743

744+
it('should support objects with no prototype', async done => {
745+
const Example = () => null;
746+
747+
const object = Object.create(null);
748+
object.string = 'abc';
749+
object.number = 123;
750+
object.boolean = true;
751+
752+
const container = document.createElement('div');
753+
await utils.actAsync(() =>
754+
ReactDOM.render(<Example object={object} />, container),
755+
);
756+
757+
const id = ((store.getElementIDAtIndex(0): any): number);
758+
759+
let inspectedElement = null;
760+
761+
function Suspender({target}) {
762+
const {getInspectedElement} = React.useContext(InspectedElementContext);
763+
inspectedElement = getInspectedElement(id);
764+
return null;
765+
}
766+
767+
await utils.actAsync(
768+
() =>
769+
TestRenderer.create(
770+
<Contexts
771+
defaultSelectedElementID={id}
772+
defaultSelectedElementIndex={0}>
773+
<React.Suspense fallback={null}>
774+
<Suspender target={id} />
775+
</React.Suspense>
776+
</Contexts>,
777+
),
778+
false,
779+
);
780+
781+
expect(inspectedElement).not.toBeNull();
782+
expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`);
783+
expect(inspectedElement.props.object).toEqual({
784+
boolean: true,
785+
number: 123,
786+
string: 'abc',
787+
});
788+
789+
done();
790+
});
791+
792+
it('should support objects with overridden hasOwnProperty', async done => {
793+
const Example = () => null;
794+
795+
const object = {
796+
name: 'blah',
797+
hasOwnProperty: true,
798+
};
799+
800+
const container = document.createElement('div');
801+
await utils.actAsync(() =>
802+
ReactDOM.render(<Example object={object} />, container),
803+
);
804+
805+
const id = ((store.getElementIDAtIndex(0): any): number);
806+
807+
let inspectedElement = null;
808+
809+
function Suspender({target}) {
810+
const {getInspectedElement} = React.useContext(InspectedElementContext);
811+
inspectedElement = getInspectedElement(id);
812+
return null;
813+
}
814+
815+
await utils.actAsync(
816+
() =>
817+
TestRenderer.create(
818+
<Contexts
819+
defaultSelectedElementID={id}
820+
defaultSelectedElementIndex={0}>
821+
<React.Suspense fallback={null}>
822+
<Suspender target={id} />
823+
</React.Suspense>
824+
</Contexts>,
825+
),
826+
false,
827+
);
828+
829+
expect(inspectedElement).not.toBeNull();
830+
expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`);
831+
expect(inspectedElement.props.object).toEqual({
832+
name: 'blah',
833+
hasOwnProperty: true,
834+
});
835+
836+
done();
837+
});
838+
756839
it('should support custom objects with enumerable properties and getters', async done => {
757840
class CustomData {
758841
_number = 42;

‎packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,6 @@ Object {
151151
"object_of_objects": {
152152
"inner": {}
153153
},
154-
"object_with_null_proto": {
155-
"string": "abc",
156-
"number": 123,
157-
"boolean": true
158-
},
159154
"react_element": {},
160155
"regexp": {},
161156
"set": {
@@ -198,6 +193,47 @@ Object {
198193
}
199194
`;
200195

196+
exports[`InspectedElementContext should support objects with no prototype: 1: Initial inspection 1`] = `
197+
Object {
198+
"id": 2,
199+
"type": "full-data",
200+
"value": {
201+
"id": 2,
202+
"owners": null,
203+
"context": {},
204+
"hooks": null,
205+
"props": {
206+
"object": {
207+
"string": "abc",
208+
"number": 123,
209+
"boolean": true
210+
}
211+
},
212+
"state": null
213+
},
214+
}
215+
`;
216+
217+
exports[`InspectedElementContext should support objects with overridden hasOwnProperty: 1: Initial inspection 1`] = `
218+
Object {
219+
"id": 2,
220+
"type": "full-data",
221+
"value": {
222+
"id": 2,
223+
"owners": null,
224+
"context": {},
225+
"hooks": null,
226+
"props": {
227+
"object": {
228+
"name": "blah",
229+
"hasOwnProperty": true
230+
}
231+
},
232+
"state": null
233+
},
234+
}
235+
`;
236+
201237
exports[`InspectedElementContext should support simple data types: 1: Initial inspection 1`] = `
202238
Object {
203239
"id": 2,

‎packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js‎

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,6 @@ describe('InspectedElementContext', () => {
168168
xyz: 1,
169169
},
170170
});
171-
const objectWithNullProto = Object.create(null);
172-
objectWithNullProto.string = 'abc';
173-
objectWithNullProto.number = 123;
174-
objectWithNullProto.boolean = true;
175171

176172
act(() =>
177173
ReactDOM.render(
@@ -188,7 +184,6 @@ describe('InspectedElementContext', () => {
188184
map={mapShallow}
189185
map_of_maps={mapOfMaps}
190186
object_of_objects={objectOfObjects}
191-
object_with_null_proto={objectWithNullProto}
192187
react_element={<span />}
193188
regexp={/abc/giu}
194189
set={setShallow}
@@ -217,7 +212,6 @@ describe('InspectedElementContext', () => {
217212
map,
218213
map_of_maps,
219214
object_of_objects,
220-
object_with_null_proto,
221215
react_element,
222216
regexp,
223217
set,
@@ -283,12 +277,6 @@ describe('InspectedElementContext', () => {
283277
);
284278
expect(object_of_objects.inner[meta.preview_short]).toBe('{…}');
285279

286-
expect(object_with_null_proto).toEqual({
287-
boolean: true,
288-
number: 123,
289-
string: 'abc',
290-
});
291-
292280
expect(react_element[meta.inspectable]).toBe(false);
293281
expect(react_element[meta.name]).toBe('span');
294282
expect(react_element[meta.type]).toBe('react_element');
@@ -325,6 +313,61 @@ describe('InspectedElementContext', () => {
325313
done();
326314
});
327315

316+
it('should support objects with no prototype', async done => {
317+
const Example = () => null;
318+
319+
const object = Object.create(null);
320+
object.string = 'abc';
321+
object.number = 123;
322+
object.boolean = true;
323+
324+
act(() =>
325+
ReactDOM.render(
326+
<Example object={object} />,
327+
document.createElement('div'),
328+
),
329+
);
330+
331+
const id = ((store.getElementIDAtIndex(0): any): number);
332+
const inspectedElement = await read(id);
333+
334+
expect(inspectedElement).toMatchSnapshot('1: Initial inspection');
335+
expect(inspectedElement.value.props.object).toEqual({
336+
boolean: true,
337+
number: 123,
338+
string: 'abc',
339+
});
340+
341+
done();
342+
});
343+
344+
it('should support objects with overridden hasOwnProperty', async done => {
345+
const Example = () => null;
346+
347+
const object = {
348+
name: 'blah',
349+
hasOwnProperty: true,
350+
};
351+
352+
act(() =>
353+
ReactDOM.render(
354+
<Example object={object} />,
355+
document.createElement('div'),
356+
),
357+
);
358+
359+
const id = ((store.getElementIDAtIndex(0): any): number);
360+
const inspectedElement = await read(id);
361+
362+
expect(inspectedElement).toMatchSnapshot('1: Initial inspection');
363+
expect(inspectedElement.value.props.object).toEqual({
364+
name: 'blah',
365+
hasOwnProperty: true,
366+
});
367+
368+
done();
369+
});
370+
328371
it('should support custom objects with enumerable properties and getters', async done => {
329372
class CustomData {
330373
_number = 42;

‎packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export default function KeyValue({
7878
type:
7979
value !== null &&
8080
typeof value === 'object' &&
81-
value.hasOwnProperty(meta.type)
81+
hasOwnProperty.call(value, meta.type)
8282
? value[meta.type]
8383
: typeof value,
8484
},
@@ -136,8 +136,8 @@ export default function KeyValue({
136136
</div>
137137
);
138138
} else if (
139-
value.hasOwnProperty(meta.type) &&
140-
!value.hasOwnProperty(meta.unserializable)
139+
hasOwnProperty.call(value, meta.type) &&
140+
!hasOwnProperty.call(value, meta.unserializable)
141141
) {
142142
children = (
143143
<div

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export function createRegExp(string: string): RegExp {
9393
}
9494

9595
export function getMetaValueLabel(data: Object): string | null {
96-
if (data.hasOwnProperty(meta.preview_long)) {
96+
if (hasOwnProperty.call(data, meta.preview_long)) {
9797
return data[meta.preview_long];
9898
} else {
9999
return formatDataForPreview(data, true);

0 commit comments

Comments
 (0)