When sorting an object with a Proxy parent, look up values on the

Summary: I checked the code to make sure obj_ was not being used incorrectly anywhere else in StandardSortModel::swap(). Reviewed By: avp Differential Revision: D23877002 fbshipit-source-id: 47b5bf9c688e01fb2c9c83fb54458941a24aef78
facebook · Sep 23, 2020 · 4a0538a · 4a0538a
1 parent de50748
commit 4a0538a
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 10 deletions.
diff --git a/lib/VM/JSLib/Array.cpp b/lib/VM/JSLib/Array.cpp
@@ -1024,19 +1024,18 @@ class StandardSortModel : public SortModel {
           return ExecutionStatus::EXCEPTION;
         }
         aHandle_ = keyRes->get();
-        CallResult<bool> hasPropRes =
-            JSProxy::getOwnProperty(obj_, runtime_, aHandle_, aDesc, nullptr);
+        CallResult<bool> hasPropRes = JSProxy::getOwnProperty(
+            aDescObjHandle_, runtime_, aHandle_, aDesc, nullptr);
         if (hasPropRes == ExecutionStatus::EXCEPTION) {
           return ExecutionStatus::EXCEPTION;
         }
         if (*hasPropRes) {
-          auto res = JSProxy::getComputed(obj_, runtime_, aHandle_, obj_);
+          auto res =
+              JSProxy::getComputed(aDescObjHandle_, runtime_, aHandle_, obj_);
           if (res == ExecutionStatus::EXCEPTION) {
             return ExecutionStatus::EXCEPTION;
           }
           aValue_ = std::move(*res);
-          // signal later code
-          aDescObjHandle_ = *obj_;
         } else {
           aDescObjHandle_ = nullptr;
         }
@@ -1058,19 +1057,18 @@ class StandardSortModel : public SortModel {
           return ExecutionStatus::EXCEPTION;
         }
         bHandle_ = keyRes->get();
-        CallResult<bool> hasPropRes =
-            JSProxy::getOwnProperty(obj_, runtime_, bHandle_, bDesc, nullptr);
+        CallResult<bool> hasPropRes = JSProxy::getOwnProperty(
+            bDescObjHandle_, runtime_, bHandle_, bDesc, nullptr);
         if (hasPropRes == ExecutionStatus::EXCEPTION) {
           return ExecutionStatus::EXCEPTION;
         }
         if (*hasPropRes) {
-          auto res = JSProxy::getComputed(obj_, runtime_, bHandle_, obj_);
+          auto res =
+              JSProxy::getComputed(bDescObjHandle_, runtime_, bHandle_, obj_);
           if (res == ExecutionStatus::EXCEPTION) {
             return ExecutionStatus::EXCEPTION;
           }
           bValue_ = std::move(*res);
-          // signal later code
-          bDescObjHandle_ = *obj_;
         } else {
           bDescObjHandle_ = nullptr;
         }

diff --git a/test/hermes/array-functions.js b/test/hermes/array-functions.js
@@ -578,6 +578,13 @@ a.__defineGetter__(1, function() { a.length = 0; return 0; });
 a.sort();
 print('sorting', a, 'did not crash');
 // CHECK-NEXT: sorting  did not crash
+var a = new Array(2);
+// hole at 0
+a[1] = 1;
+a.__proto__ = new Proxy([],{});
+a.sort();
+print(a);
+// CHECK-NEXT: 1,
 
 print('splice');
 // CHECK-LABEL: splice