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.
facebook · Jan 3, 2020 · 7e2ab87 · 7e2ab87
1 parent 5d3d71b
commit 7e2ab87
Show file tree

Hide file tree

Showing 10 changed files with 266 additions and 45 deletions.
diff --git a/...es/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap b/...es/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,

diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js b/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;

diff --git a/...ages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap b/...ages/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,

diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/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;

diff --git a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js b/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

diff --git a/packages/react-devtools-shared/src/devtools/views/utils.js b/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);