Bug 1406182 - Obtain the length of typed arrays in a safe way. draft
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Sat, 07 Oct 2017 16:55:26 +0200
changeset 676704 06903d3ff5b8b09cd5268ab321822d82306a3571
parent 676397 6b0491f83229a80462793c74825f79b1ec1d9cda
child 735030 cd95e0629a5a5cb7988aa04ac0fd4ce6304c2b5d
push id83597
push userbmo:oriol-bugzilla@hotmail.com
push dateMon, 09 Oct 2017 12:25:31 +0000
bugs1406182
milestone58.0a1
Bug 1406182 - Obtain the length of typed arrays in a safe way. MozReview-Commit-ID: ItlYZowJw7x
devtools/server/actors/object.js
devtools/server/tests/unit/test_objectgrips-20.js
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@@ -113,18 +113,17 @@ ObjectActor.prototype = {
     if (g.class == "Promise") {
       g.promiseState = this._createPromiseState();
     }
 
     // FF40+: Allow to know how many properties an object has to lazily display them
     // when there is a bunch.
     if (isTypedArray(g)) {
       // Bug 1348761: getOwnPropertyNames is unnecessary slow on TypedArrays
-      let length = DevToolsUtils.getProperty(this.obj, "length");
-      g.ownPropertyLength = length;
+      g.ownPropertyLength = getArrayLength(this.obj);
     } else if (g.class != "Proxy") {
       g.ownPropertyLength = this.obj.getOwnPropertyNames().length;
     }
 
     let raw = this.obj.unsafeDereference();
 
     // If Cu is not defined, we are running on a worker thread, where xrays
     // don't exist.
@@ -865,31 +864,18 @@ PropertyIteratorActor.prototype = {
 
 PropertyIteratorActor.prototype.requestTypes = {
   "names": PropertyIteratorActor.prototype.names,
   "slice": PropertyIteratorActor.prototype.slice,
   "all": PropertyIteratorActor.prototype.all,
 };
 
 function enumArrayProperties(objectActor, options) {
-  let length = DevToolsUtils.getProperty(objectActor.obj, "length");
-  if (!isUint32(length)) {
-    // Pseudo arrays are flagged as ArrayLike if they have
-    // subsequent indexed properties without having any length attribute.
-    length = 0;
-    let names = objectActor.obj.getOwnPropertyNames();
-    for (let key of names) {
-      if (!isArrayIndex(key) || key != length++) {
-        break;
-      }
-    }
-  }
-
   return {
-    size: length,
+    size: getArrayLength(objectActor.obj),
     propertyName(index) {
       return index;
     },
     propertyDescription(index) {
       return objectActor._propertyDescriptor(index);
     }
   };
 }
@@ -1314,20 +1300,17 @@ DebuggerServer.ObjectActorPreviewers = {
 
     grip.preview = {
       timestamp: hooks.createValueGrip(time),
     };
     return true;
   }],
 
   Array: [function ({obj, hooks}, grip) {
-    let length = DevToolsUtils.getProperty(obj, "length");
-    if (typeof length != "number") {
-      return false;
-    }
+    let length = getArrayLength(obj);
 
     grip.preview = {
       kind: "ArrayLike",
       length: length,
     };
 
     if (hooks.getGripDepth() > 1) {
       return true;
@@ -1631,24 +1614,19 @@ function GenericObject(objectActor, grip
 
 // Preview functions that do not rely on the object class.
 DebuggerServer.ObjectActorPreviewers.Object = [
   function TypedArray({obj, hooks}, grip) {
     if (!isTypedArray(obj)) {
       return false;
     }
 
-    let length = DevToolsUtils.getProperty(obj, "length");
-    if (typeof length != "number") {
-      return false;
-    }
-
     grip.preview = {
       kind: "ArrayLike",
-      length: length,
+      length: getArrayLength(obj),
     };
 
     if (hooks.getGripDepth() > 1) {
       return true;
     }
 
     let raw = obj.unsafeDereference();
     let global = Cu.getGlobalForObject(DebuggerServer);
@@ -2527,16 +2505,42 @@ function isTypedArray(object) {
  *        The debuggee object to test.
  * @return Boolean
  */
 function isArray(object) {
   return isTypedArray(object) || object.class === "Array";
 }
 
 /**
+ * Returns the length of an array (or typed array).
+ *
+ * @param obj Debugger.Object
+ *        The debuggee object of the array.
+ * @return Number
+ * @throws if the object is not an array.
+ */
+function getArrayLength(object) {
+  if (!isArray(object)) {
+    throw new Error("Expected an array, got a " + object.class);
+  }
+
+  // Real arrays have a reliable `length` own property.
+  if (object.class === "Array") {
+    return DevToolsUtils.getProperty(object, "length");
+  }
+
+  // For typed arrays, `DevToolsUtils.getProperty` is not reliable because the `length`
+  // getter could be shadowed by an own property, and `getOwnPropertyNames` is
+  // unnecessarily slow. Obtain the `length` getter safely and call it manually.
+  let typedProto = Object.getPrototypeOf(Uint8Array.prototype);
+  let getter = Object.getOwnPropertyDescriptor(typedProto, "length").get;
+  return getter.call(object.unsafeDereference());
+}
+
+/**
  * Returns true if the parameter can be stored as a 32-bit unsigned integer.
  * If so, it will be suitable for use as the length of an array object.
  *
  * @param num Number
  *        The number to test.
  * @return Boolean
  */
 function isUint32(num) {
--- a/devtools/server/tests/unit/test_objectgrips-20.js
+++ b/devtools/server/tests/unit/test_objectgrips-20.js
@@ -148,16 +148,37 @@ async function run_test_with_server(serv
     expectedNonIndexedProperties: [
       ["foo", "bar"],
       ["bar", "foo"],
       ["length", 2],
       ["buffer", DO_NOT_CHECK_VALUE],
       ["byteLength", 2],
       ["byteOffset", 0],
     ],
+  }, {
+    evaledObject: `(() => {
+      x = new Int8Array([1, 2]);
+      Object.defineProperty(x, 'length', {value: 0});
+      return x;
+    })()`,
+    expectedIndexedProperties: [["0", 1], ["1", 2]],
+    expectedNonIndexedProperties: [
+      ["length", 0],
+      ["buffer", DO_NOT_CHECK_VALUE],
+      ["byteLength", 2],
+      ["byteOffset", 0],
+    ],
+  }, {
+    evaledObject: `(() => {
+      x = new Int32Array([1, 2]);
+      Object.setPrototypeOf(x, null);
+      return x;
+    })()`,
+    expectedIndexedProperties: [["0", 1], ["1", 2]],
+    expectedNonIndexedProperties: [],
   }].forEach(async (testData) => {
     await test_object_grip(debuggee, dbgClient, threadClient, testData);
   });
 
   await dbgClient.close();
 }
 
 async function test_object_grip(debuggee, dbgClient, threadClient, testData = {}) {