Bug 1284232 - Guard against ill-behaved objects on error check; r?automatedtester draft
authorAndreas Tolfsen <ato@mozilla.com>
Fri, 08 Jul 2016 13:58:04 +0100
changeset 385462 78a68e53c82d9e9d3477125a014c9ef888d43ca3
parent 385458 8a78ce00563ec29ac4ab465a17d7400830096848
child 385463 c74b57859f12749904c44a326e3431744ef911fd
push id22510
push userbmo:ato@mozilla.com
push dateFri, 08 Jul 2016 13:34:06 +0000
reviewersautomatedtester
bugs1284232
milestone50.0a1
Bug 1284232 - Guard against ill-behaved objects on error check; r?automatedtester It turns out that certain objects such as a DOMRectList have peculiarities that cause string comparison to throw. This is normally not a problem as error.isError is usually passed JSON serialisable data. But when it is not, this try condition helps diagnose problems. MozReview-Commit-ID: BLNSziwfxXs
testing/marionette/error.js
testing/marionette/test_error.js
--- a/testing/marionette/error.js
+++ b/testing/marionette/error.js
@@ -24,31 +24,31 @@ const ERRORS = [
   "TimeoutError",
   "UnableToSetCookieError",
   "UnknownCommandError",
   "UnknownError",
   "UnsupportedOperationError",
   "WebDriverError",
 ];
 
+const BUILTIN_ERRORS = new Set([
+  "Error",
+  "EvalError",
+  "InternalError",
+  "RangeError",
+  "ReferenceError",
+  "SyntaxError",
+  "TypeError",
+  "URIError",
+]);
+
 this.EXPORTED_SYMBOLS = ["error"].concat(ERRORS);
 
 this.error = {};
 
-error.BuiltinErrors = {
-  Error: 0,
-  EvalError: 1,
-  InternalError: 2,
-  RangeError: 3,
-  ReferenceError: 4,
-  SyntaxError: 5,
-  TypeError: 6,
-  URIError: 7,
-};
-
 /**
  * Checks if obj is an instance of the Error prototype in a safe manner.
  * Prefer using this over using instanceof since the Error prototype
  * isn't unique across browsers, and XPCOM nsIException's are special
  * snowflakes.
  *
  * @param {*} val
  *     Any value that should be undergo the test for errorness.
@@ -56,17 +56,23 @@ error.BuiltinErrors = {
  *     True if error, false otherwise.
  */
 error.isError = function(val) {
   if (val === null || typeof val != "object") {
     return false;
   } else if (val instanceof Ci.nsIException) {
     return true;
   } else {
-    return Object.getPrototypeOf(val) in error.BuiltinErrors;
+    // DOMRectList errors on string comparison
+   try {
+      let proto = Object.getPrototypeOf(val);
+      return BUILTIN_ERRORS.has(proto.toString());
+    } catch (e) {
+      return false;
+    }
   }
 };
 
 /**
  * Checks if obj is an object in the WebDriverError prototypal chain.
  */
 error.isWebDriverError = function(obj) {
   return error.isError(obj) &&
--- a/testing/marionette/test_error.js
+++ b/testing/marionette/test_error.js
@@ -5,29 +5,16 @@
 const {utils: Cu} = Components;
 
 Cu.import("chrome://marionette/content/error.js");
 
 function notok(condition) {
   ok(!(condition));
 }
 
-add_test(function test_BuiltinErrors() {
-  ok("Error" in error.BuiltinErrors);
-  ok("EvalError" in error.BuiltinErrors);
-  ok("InternalError" in error.BuiltinErrors);
-  ok("RangeError" in error.BuiltinErrors);
-  ok("ReferenceError" in error.BuiltinErrors);
-  ok("SyntaxError" in error.BuiltinErrors);
-  ok("TypeError" in error.BuiltinErrors);
-  ok("URIError" in error.BuiltinErrors);
-
-  run_next_test();
-});
-
 add_test(function test_isError() {
   notok(error.isError(null));
   notok(error.isError([]));
   notok(error.isError(new Date()));
 
   ok(error.isError(new Components.Exception()));
   ok(error.isError(new Error()));
   ok(error.isError(new EvalError()));