DevTools: Replaced unsafe hasOwnProperty() calls (#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.

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

@@ -510,11 +510,6 @@ exports[`InspectedElementContext should support complex data types: 1: Inspected
     "object_of_objects": {
       "inner": {}
     },
-    "object_with_null_proto": {
-      "string": "abc",
-      "number": 123,
-      "boolean": true
-    },
     "react_element": {},
     "regexp": {},
     "set": {
@@ -552,6 +547,39 @@ exports[`InspectedElementContext should support custom objects with enumerable p
 }
 `;
 
+exports[`InspectedElementContext should support objects with no prototype: 1: Inspected element 2 1`] = `
+{
+  "id": 2,
+  "owners": null,
+  "context": null,
+  "hooks": null,
+  "props": {
+    "object": {
+      "string": "abc",
+      "number": 123,
+      "boolean": true
+    }
+  },
+  "state": null
+}
+`;
+
+exports[`InspectedElementContext should support objects with overridden hasOwnProperty: 1: Inspected element 2 1`] = `
+{
+  "id": 2,
+  "owners": null,
+  "context": null,
+  "hooks": null,
+  "props": {
+    "object": {
+      "name": "blah",
+      "hasOwnProperty": true
+    }
+  },
+  "state": null
+}
+`;
+
 exports[`InspectedElementContext should support simple data types: 1: Initial inspection 1`] = `
 {
   "id": 2,

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

@@ -537,10 +537,6 @@ describe('InspectedElementContext', () => {
         xyz: 1,
       },
     });
-    const objectWithNullProto = Object.create(null);
-    objectWithNullProto.string = 'abc';
-    objectWithNullProto.number = 123;
-    objectWithNullProto.boolean = true;
 
     const container = document.createElement('div');
     await utils.actAsync(() =>
@@ -558,7 +554,6 @@ describe('InspectedElementContext', () => {
           map={mapShallow}
           map_of_maps={mapOfMaps}
           object_of_objects={objectOfObjects}
-          object_with_null_proto={objectWithNullProto}
           react_element={<span />}
           regexp={/abc/giu}
           set={setShallow}
@@ -609,7 +604,6 @@ describe('InspectedElementContext', () => {
       map,
       map_of_maps,
       object_of_objects,
-      object_with_null_proto,
       react_element,
       regexp,
       set,
@@ -701,12 +695,6 @@ describe('InspectedElementContext', () => {
     );
     expect(object_of_objects.inner[meta.preview_short]).toBe('{…}');
 
-    expect(object_with_null_proto).toEqual({
-      boolean: true,
-      number: 123,
-      string: 'abc',
-    });
-
     expect(react_element[meta.inspectable]).toBe(false);
     expect(react_element[meta.name]).toBe('span');
     expect(react_element[meta.type]).toBe('react_element');
@@ -753,6 +741,101 @@ describe('InspectedElementContext', () => {
     done();
   });
 
+  it('should support objects with no prototype', async done => {
+    const Example = () => null;
+
+    const object = Object.create(null);
+    object.string = 'abc';
+    object.number = 123;
+    object.boolean = true;
+
+    const container = document.createElement('div');
+    await utils.actAsync(() =>
+      ReactDOM.render(<Example object={object} />, container),
+    );
+
+    const id = ((store.getElementIDAtIndex(0): any): number);
+
+    let inspectedElement = null;
+
+    function Suspender({target}) {
+      const {getInspectedElement} = React.useContext(InspectedElementContext);
+      inspectedElement = getInspectedElement(id);
+      return null;
+    }
+
+    await utils.actAsync(
+      () =>
+        TestRenderer.create(
+          <Contexts
+            defaultSelectedElementID={id}
+            defaultSelectedElementIndex={0}>
+            <React.Suspense fallback={null}>
+              <Suspender target={id} />
+            </React.Suspense>
+          </Contexts>,
+        ),
+      false,
+    );
+
+    expect(inspectedElement).not.toBeNull();
+    expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`);
+    expect(inspectedElement.props.object).toEqual({
+      boolean: true,
+      number: 123,
+      string: 'abc',
+    });
+
+    done();
+  });
+
+  it('should support objects with overridden hasOwnProperty', async done => {
+    const Example = () => null;
+
+    const object = {
+      name: 'blah',
+      hasOwnProperty: true,
+    };
+
+    const container = document.createElement('div');
+    await utils.actAsync(() =>
+      ReactDOM.render(<Example object={object} />, container),
+    );
+
+    const id = ((store.getElementIDAtIndex(0): any): number);
+
+    let inspectedElement = null;
+
+    function Suspender({target}) {
+      const {getInspectedElement} = React.useContext(InspectedElementContext);
+      inspectedElement = getInspectedElement(id);
+      return null;
+    }
+
+    await utils.actAsync(
+      () =>
+        TestRenderer.create(
+          <Contexts
+            defaultSelectedElementID={id}
+            defaultSelectedElementIndex={0}>
+            <React.Suspense fallback={null}>
+              <Suspender target={id} />
+            </React.Suspense>
+          </Contexts>,
+        ),
+      false,
+    );
+
+    expect(inspectedElement).not.toBeNull();
+    expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`);
+    expect(inspectedElement.props.object).toEqual({
+      name: 'blah',
+      hasOwnProperty: true,
+    });
+
+    done();
+  });
+
   it('should support custom objects with enumerable properties and getters', async done => {
     class CustomData {
       _number = 42;

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

@@ -151,11 +151,6 @@ Object {
     "object_of_objects": {
       "inner": {}
     },
-    "object_with_null_proto": {
-      "string": "abc",
-      "number": 123,
-      "boolean": true
-    },
     "react_element": {},
     "regexp": {},
     "set": {
@@ -198,6 +193,47 @@ Object {
 }
 `;
 
+exports[`InspectedElementContext should support objects with no prototype: 1: Initial inspection 1`] = `
+Object {
+  "id": 2,
+  "type": "full-data",
+  "value": {
+  "id": 2,
+  "owners": null,
+  "context": {},
+  "hooks": null,
+  "props": {
+    "object": {
+      "string": "abc",
+      "number": 123,
+      "boolean": true
+    }
+  },
+  "state": null
+},
+}
+`;
+
+exports[`InspectedElementContext should support objects with overridden hasOwnProperty: 1: Initial inspection 1`] = `
+Object {
+  "id": 2,
+  "type": "full-data",
+  "value": {
+  "id": 2,
+  "owners": null,
+  "context": {},
+  "hooks": null,
+  "props": {
+    "object": {
+      "name": "blah",
+      "hasOwnProperty": true
+    }
+  },
+  "state": null
+},
+}
+`;
+
 exports[`InspectedElementContext should support simple data types: 1: Initial inspection 1`] = `
 Object {
   "id": 2,

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

@@ -168,10 +168,6 @@ describe('InspectedElementContext', () => {
         xyz: 1,
       },
     });
-    const objectWithNullProto = Object.create(null);
-    objectWithNullProto.string = 'abc';
-    objectWithNullProto.number = 123;
-    objectWithNullProto.boolean = true;
 
     act(() =>
       ReactDOM.render(
@@ -188,7 +184,6 @@ describe('InspectedElementContext', () => {
           map={mapShallow}
           map_of_maps={mapOfMaps}
           object_of_objects={objectOfObjects}
-          object_with_null_proto={objectWithNullProto}
           react_element={<span />}
           regexp={/abc/giu}
           set={setShallow}
@@ -217,7 +212,6 @@ describe('InspectedElementContext', () => {
       map,
       map_of_maps,
       object_of_objects,
-      object_with_null_proto,
       react_element,
       regexp,
       set,
@@ -283,12 +277,6 @@ describe('InspectedElementContext', () => {
     );
     expect(object_of_objects.inner[meta.preview_short]).toBe('{…}');
 
-    expect(object_with_null_proto).toEqual({
-      boolean: true,
-      number: 123,
-      string: 'abc',
-    });
-
     expect(react_element[meta.inspectable]).toBe(false);
     expect(react_element[meta.name]).toBe('span');
     expect(react_element[meta.type]).toBe('react_element');
@@ -325,6 +313,61 @@ describe('InspectedElementContext', () => {
     done();
   });
 
+  it('should support objects with no prototype', async done => {
+    const Example = () => null;
+
+    const object = Object.create(null);
+    object.string = 'abc';
+    object.number = 123;
+    object.boolean = true;
+
+    act(() =>
+      ReactDOM.render(
+        <Example object={object} />,
+        document.createElement('div'),
+      ),
+    );
+
+    const id = ((store.getElementIDAtIndex(0): any): number);
+    const inspectedElement = await read(id);
+
+    expect(inspectedElement).toMatchSnapshot('1: Initial inspection');
+    expect(inspectedElement.value.props.object).toEqual({
+      boolean: true,
+      number: 123,
+      string: 'abc',
+    });
+
+    done();
+  });
+
+  it('should support objects with overridden hasOwnProperty', async done => {
+    const Example = () => null;
+
+    const object = {
+      name: 'blah',
+      hasOwnProperty: true,
+    };
+
+    act(() =>
+      ReactDOM.render(
+        <Example object={object} />,
+        document.createElement('div'),
+      ),
+    );
+
+    const id = ((store.getElementIDAtIndex(0): any): number);
+    const inspectedElement = await read(id);
+
+    expect(inspectedElement).toMatchSnapshot('1: Initial inspection');
+    expect(inspectedElement.value.props.object).toEqual({
+      name: 'blah',
+      hasOwnProperty: true,
+    });
+
+    done();
+  });
+
   it('should support custom objects with enumerable properties and getters', async done => {
     class CustomData {
       _number = 42;

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

@@ -78,7 +78,7 @@ export default function KeyValue({
       type:
         value !== null &&
         typeof value === 'object' &&
-        value.hasOwnProperty(meta.type)
+        hasOwnProperty.call(value, meta.type)
           ? value[meta.type]
           : typeof value,
     },
@@ -136,8 +136,8 @@ export default function KeyValue({
       </div>
     );
   } else if (
-    value.hasOwnProperty(meta.type) &&
-    !value.hasOwnProperty(meta.unserializable)
+    hasOwnProperty.call(value, meta.type) &&
+    !hasOwnProperty.call(value, meta.unserializable)
   ) {
     children = (
       <div

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

@@ -93,7 +93,7 @@ export function createRegExp(string: string): RegExp {
 }
 
 export function getMetaValueLabel(data: Object): string | null {
-  if (data.hasOwnProperty(meta.preview_long)) {
+  if (hasOwnProperty.call(data, meta.preview_long)) {
     return data[meta.preview_long];
   } else {
     return formatDataForPreview(data, true);

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

@@ -385,7 +385,7 @@ export function getDataType(data: Object): DataType {
       if (Array.isArray(data)) {
         return 'array';
       } else if (ArrayBuffer.isView(data)) {
-        return data.constructor.hasOwnProperty('BYTES_PER_ELEMENT')
+        return hasOwnProperty.call(data.constructor, 'BYTES_PER_ELEMENT')
           ? 'typed_array'
           : 'data_view';
       } else if (data.constructor && data.constructor.name === 'ArrayBuffer') {
@@ -490,7 +490,7 @@ export function formatDataForPreview(
   data: any,
   showFormattedValue: boolean,
 ): string {
-  if (data != null && data.hasOwnProperty(meta.type)) {
+  if (data != null && hasOwnProperty.call(data, meta.type)) {
     return showFormattedValue
       ? data[meta.preview_long]
       : data[meta.preview_short];
@@ -534,7 +534,7 @@ export function formatDataForPreview(
         }
         return `[${truncateForDisplay(formatted)}]`;
       } else {
-        const length = data.hasOwnProperty(meta.size)
+        const length = hasOwnProperty.call(data, meta.size)
           ? data[meta.size]
           : data.length;
         return `Array(${length})`;

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

@@ -0,0 +1,33 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+import React from 'react';
+
+const objectWithModifiedHasOwnProperty = {
+  foo: 'abc',
+  bar: 123,
+  hasOwnProperty: true,
+};
+
+const objectWithNullProto = Object.create(null);
+objectWithNullProto.foo = 'abc';
+objectWithNullProto.bar = 123;
+
+export default function EdgeCaseObjects() {
+  return (
+    <ChildComponent
+      objectWithModifiedHasOwnProperty={objectWithModifiedHasOwnProperty}
+      objectWithNullProto={objectWithNullProto}
+    />
+  );
+}
+
+function ChildComponent(props: any) {
+  return null;
+}

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

@@ -13,6 +13,7 @@ import CircularReferences from './CircularReferences';
 import Contexts from './Contexts';
 import CustomHooks from './CustomHooks';
 import CustomObject from './CustomObject';
+import EdgeCaseObjects from './EdgeCaseObjects.js';
 import NestedProps from './NestedProps';
 import SimpleValues from './SimpleValues';
 
@@ -28,6 +29,7 @@ export default function InspectableElements() {
       <Contexts />
       <CustomHooks />
       <CustomObject />
+      <EdgeCaseObjects />
       <CircularReferences />
     </Fragment>
   );

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

@@ -25,9 +25,6 @@ const immutable = Immutable.fromJS({
     xyz: 1,
   },
 });
-const objectWithNullProto = Object.create(null);
-objectWithNullProto.foo = 'abc';
-objectWithNullProto.bar = 123;
 
 export default function UnserializableProps() {
   return (
@@ -40,7 +37,6 @@ export default function UnserializableProps() {
       setOfSets={setOfSets}
       typedArray={typedArray}
       immutable={immutable}
-      objectWithNullProto={objectWithNullProto}
     />
   );
 }