From 02d1bcc047e54297fc8f8dae41f1966f55d62496 Mon Sep 17 00:00:00 2001
From: Keith Brown <keith.brown@sony.com>
Date: Thu, 7 Jun 2018 02:26:17 -0700
Subject: [PATCH] =?UTF-8?q?Modified=20deepFreezeAndThrowOnMutationInDev=20?=
 =?UTF-8?q?to=20use=20Object.prototype.ha=E2=80=A6=20(#19598)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
This PR fixes a bug in `deepFreezeAndThrowOnMutationInDev` which did not take into account that objects passed to it may have been created with `Object.create(null)` and thus may not have a prototype. Such objects don't have the methods `hasOwnProperty`, `__defineGetter__`, or `__defineSetter__` on the instance.

I ran into an unrecoverable error in React Native when passing this type of object across the bridge because `deepFreezeAndThrowOnMutationInDev` attempts to call `object.hasOwnProperty(key)`, `object.__defineGetter__` and `object__defineSetter__` on objects passed to it. But my object instance does not have these prototype methods.

Changes:
* Defined `Object.prototype.hasOwnProperty` as a `const` (pattern used elsewhere in React Native)
* Modified calls to `object.hasOwnProperty(key)` to use `hasOwnProperty.call(object, key)` (Per ESLint rule [here](https://eslint.org/docs/rules/no-prototype-builtins))
* Modified calls to deprecated methods `object.__defineGetter__` and `object.__defineSetter__` to instead use `Object.defineProperty` to define get and set methods on the object. (Per guidance on [MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/__defineGetter__))
* Added a new test to `deepFreezeAndThrowOnMutationInDev-test` to verify the fix.

I tried to create a reproducible example to post to Snack by passing prototype-less objects to a `Text` component, in various ways, but they appear to be converted to plain objects before crossing the bridge and therefore they do not throw an error.

However, I was able to create a new test to reproduce the issue. I added the following test to `deepFreezeAndThrowOnMutationInDev-test`:

```JavaScript
it('should not throw on object without prototype', () => {
    __DEV__ = true;
    var o = Object.create(null);
    o.key = 'Value';
    expect(() => deepFreezeAndThrowOnMutationInDev(o)).not.toThrow();
  });
```

The changes in this PR include this new test.

ESLint test produced no change in Error count (3) or Warnings (671)

N/A
Other areas with _possibly_ the same issue:
https://github.com/facebook/react-native/blob/c6b96c0df789717d53ec520ad28ba0ae00db6ec2/Libraries/vendor/core/mergeInto.js#L50
https://github.com/facebook/react-native/blob/8dc3ba0444c94d9bbb66295b5af885bff9b9cd34/Libraries/ReactNative/requireNativeComponent.js#L134

 [GENERAL] [BUGFIX] [Libraries/Utilities/deepFreezeAndThrowOnMutationInDev] -Fix for compatibility with objects without a prototype.
Closes https://github.com/facebook/react-native/pull/19598

Differential Revision: D8310845

Pulled By: TheSavior

fbshipit-source-id: 020c414a1062a637e97f9ee99bf8e5ba2d1fcf4f
---
 .../deepFreezeAndThrowOnMutationInDev-test.js       |  7 +++++++
 .../Utilities/deepFreezeAndThrowOnMutationInDev.js  | 13 +++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/Libraries/Utilities/__tests__/deepFreezeAndThrowOnMutationInDev-test.js b/Libraries/Utilities/__tests__/deepFreezeAndThrowOnMutationInDev-test.js
index 3ec7c7bab..4b5e11ea5 100644
--- a/Libraries/Utilities/__tests__/deepFreezeAndThrowOnMutationInDev-test.js
+++ b/Libraries/Utilities/__tests__/deepFreezeAndThrowOnMutationInDev-test.js
@@ -26,6 +26,13 @@ describe('deepFreezeAndThrowOnMutationInDev', function() {
     expect(() => deepFreezeAndThrowOnMutationInDev()).not.toThrow();
   });
 
+  it('should not throw on object without prototype', () => {
+    __DEV__ = true;
+    var o = Object.create(null);
+    o.key = 'Value';
+    expect(() => deepFreezeAndThrowOnMutationInDev(o)).not.toThrow();
+  });
+
   it('should throw on mutation in dev with strict', () => {
     'use strict';
     __DEV__ = true;
diff --git a/Libraries/Utilities/deepFreezeAndThrowOnMutationInDev.js b/Libraries/Utilities/deepFreezeAndThrowOnMutationInDev.js
index 71a29584a..2e2c4952e 100644
--- a/Libraries/Utilities/deepFreezeAndThrowOnMutationInDev.js
+++ b/Libraries/Utilities/deepFreezeAndThrowOnMutationInDev.js
@@ -39,12 +39,17 @@ function deepFreezeAndThrowOnMutationInDev<T: Object>(object: T): T {
     }
 
     const keys = Object.keys(object);
+    const hasOwnProperty = Object.prototype.hasOwnProperty;
 
     for (var i = 0; i < keys.length; i++) {
       var key = keys[i];
-      if (object.hasOwnProperty(key)) {
-        object.__defineGetter__(key, identity.bind(null, object[key]));
-        object.__defineSetter__(key, throwOnImmutableMutation.bind(null, key));
+      if (hasOwnProperty.call(object, key)) {
+        Object.defineProperty(object, key, {
+          get: identity.bind(null, object[key]),
+        });
+        Object.defineProperty(object, key, {
+          set: throwOnImmutableMutation.bind(null, key),
+        });
       }
     }
 
@@ -53,7 +58,7 @@ function deepFreezeAndThrowOnMutationInDev<T: Object>(object: T): T {
 
     for (var i = 0; i < keys.length; i++) {
       var key = keys[i];
-      if (object.hasOwnProperty(key)) {
+      if (hasOwnProperty.call(object, key)) {
         deepFreezeAndThrowOnMutationInDev(object[key]);
       }
     }