Bug 1106913 - Detect cyclic objects when marshaling objects. r?whimboo draft
authorAndreas Tolfsen <ato@sny.no>
Fri, 24 Nov 2017 18:21:17 +0000
changeset 705707 81e7e05b54504575df52b3cbbbe032f3a7de446a
parent 705706 51ac14899f3d2352e95def6dc2237d6719f1a979
child 742432 4e0ba93b57f9be216d3d203564c2f130b8112c06
push id91554
push userbmo:ato@sny.no
push dateThu, 30 Nov 2017 15:13:32 +0000
reviewerswhimboo
bugs1106913
milestone59.0a1
Bug 1106913 - Detect cyclic objects when marshaling objects. r?whimboo Marionette does currently not test for cyclic object references as it marshals return values for transport across the wire. Example of cyclic object: let obj = {}; obj.cyclic = obj; Passing this through evalaute.toJSON currently causes an infinite recursion due to obj being referenced inside itself. We can use JSON.stringify to test if obj contains such cyclic values. It is assumed that the input to assert.acyclic is already JSON safe, so it can be parsed by JSON.stringify, because of the previous checks it has made. MozReview-Commit-ID: 4CnY2dcW5IF
testing/marionette/evaluate.js
testing/web-platform/meta/MANIFEST.json
testing/web-platform/meta/webdriver/tests/contexts/json_serialize_windowproxy.py.ini
testing/web-platform/meta/webdriver/tests/execute_script/user_prompts.py.ini
testing/web-platform/tests/webdriver/tests/execute_script/cyclic.py
--- a/testing/marionette/evaluate.js
+++ b/testing/marionette/evaluate.js
@@ -6,16 +6,17 @@
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://gre/modules/NetUtil.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+Cu.import("chrome://marionette/content/assert.js");
 const {
   element,
   WebElement,
 } = Cu.import("chrome://marionette/content/element.js", {});
 const {
   JavaScriptError,
   ScriptTimeoutError,
 } = Cu.import("chrome://marionette/content/error.js", {});
@@ -238,66 +239,98 @@ evaluate.fromJSON = function(obj, seenEl
       for (let prop in obj) {
         rv[prop] = evaluate.fromJSON(obj[prop], seenEls, window);
       }
       return rv;
   }
 };
 
 /**
- * Convert arbitrary objects to JSON-safe primitives that can be
+ * Marshal arbitrary objects to JSON-safe primitives that can be
  * transported over the Marionette protocol.
  *
- * Any DOM elements are converted to web elements by looking them up
- * and/or adding them to the element store provided.
+ * The marshaling rules are as follows:
+ *
+ * <ul>
+ *
+ * <li>
+ * Primitives are returned as is.
+ *
+ * <li>
+ * Collections, such as <code>Array</code>, <code>NodeList</code>,
+ * <code>HTMLCollection</code> et al. are expanded to arrays and
+ * then recursed.
+ *
+ * <li>
+ * Elements that are not known web elements are added to the
+ * <var>seenEls</var> element store.  Once known, the elements'
+ * associated web element representation is returned.
+ *
+ * <li>
+ * Objects with custom JSON representations, i.e. if they have a
+ * callable <code>toJSON</code> function, are returned verbatim.
+ * This means their internal integrity <em>are not</em> checked.
+ * Be careful.
+ *
+ * <li>
+ * Other arbitrary objects are first tested for cyclic references
+ * and then recursed into.
+ *
+ * </ul>
  *
  * @param {Object} obj
  *     Object to be marshaled.
  * @param {element.Store} seenEls
  *     Element store to use for lookup of web element references.
  *
  * @return {Object}
  *     Same object as provided by <var>obj</var> with the elements
  *     replaced by web elements.
+ *
+ * @throws {JavaScriptError}
+ *     If an object contains cyclic references.
  */
 evaluate.toJSON = function(obj, seenEls) {
   const t = Object.prototype.toString.call(obj);
 
   // null
   if (t == "[object Undefined]" || t == "[object Null]") {
     return null;
 
   // primitives
   } else if (t == "[object Boolean]" ||
       t == "[object Number]" ||
       t == "[object String]") {
     return obj;
 
   // Array, NodeList, HTMLCollection, et al.
   } else if (element.isCollection(obj)) {
+    assert.acyclic(obj);
     return [...obj].map(el => evaluate.toJSON(el, seenEls));
 
   // WebElement
   } else if (WebElement.isReference(obj)) {
     return obj;
 
-  // Element (HTMLElement, SVGElement, XULElement, &c.)
+  // Element (HTMLElement, SVGElement, XULElement, et al.)
   } else if (element.isElement(obj)) {
     let webEl = seenEls.add(obj);
     return webEl.toJSON();
 
   // custom JSON representation
   } else if (typeof obj.toJSON == "function") {
     let unsafeJSON = obj.toJSON();
     return evaluate.toJSON(unsafeJSON, seenEls);
   }
 
   // arbitrary objects + files
   let rv = {};
   for (let prop in obj) {
+    assert.acyclic(obj[prop]);
+
     try {
       rv[prop] = evaluate.toJSON(obj[prop], seenEls);
     } catch (e) {
       if (e.result == Cr.NS_ERROR_NOT_IMPLEMENTED) {
         log.debug(`Skipping ${prop}: ${e.message}`);
       } else {
         throw e;
       }
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -373751,16 +373751,22 @@
     ]
    ],
    "webdriver/tests/execute_async_script/user_prompts.py": [
     [
      "/webdriver/tests/execute_async_script/user_prompts.py",
      {}
     ]
    ],
+   "webdriver/tests/execute_script/cyclic.py": [
+    [
+     "/webdriver/tests/execute_script/cyclic.py",
+     {}
+    ]
+   ],
    "webdriver/tests/execute_script/user_prompts.py": [
     [
      "/webdriver/tests/execute_script/user_prompts.py",
      {}
     ]
    ],
    "webdriver/tests/fullscreen_window.py": [
     [
@@ -575625,17 +575631,17 @@
    "2fcd9de0de5291930f331b25ee98e1c92490ab6a",
    "testharness"
   ],
   "wasm/wasm_indexeddb_test.js": [
    "e202fc90095365ce4d19e3ff3f2423e93d2f979b",
    "support"
   ],
   "wasm/wasm_local_iframe_test.html": [
-   "dd715a4da792b9d8d634536d938b278230c66df5",
+   "e7b86a731b7cf7c122a1e37118cebce7342291fc",
    "testharness"
   ],
   "wasm/wasm_serialization_tests.html": [
    "aa7f9d293f6752b392307b7bd346ac2913874356",
    "testharness"
   ],
   "wasm/wasm_serialization_tests.js": [
    "e5a6d0f1a2218df9b080df7e99b684ddaceb4a11",
@@ -576437,17 +576443,17 @@
    "d589b53f0096893600e696b43ec19ca84e5ee2ab",
    "wdspec"
   ],
   "webdriver/tests/actions/key_shortcuts.py": [
    "dbe27dd0b1625169fc8cc2055f8fb49d5a4a78d2",
    "wdspec"
   ],
   "webdriver/tests/actions/modifier_click.py": [
-   "be31579cae0cb3dd26a913ce0d966be72fd79495",
+   "a41f28b359c950af698be51ef35e4d78dca53e2c",
    "wdspec"
   ],
   "webdriver/tests/actions/mouse.py": [
    "0af689cee458ed260b2b9cc6f3231c314a3a6638",
    "wdspec"
   ],
   "webdriver/tests/actions/mouse_dblclick.py": [
    "61bab159bf1ccc7d44e4034a3e67d60b13fc1607",
@@ -576532,16 +576538,20 @@
   "webdriver/tests/element_retrieval/get_active_element.py": [
    "918c6e48047f31a088ec44e9b0d070b0ae3d6077",
    "wdspec"
   ],
   "webdriver/tests/execute_async_script/user_prompts.py": [
    "e31edd4537f9b7479a348465154381f5b18f938c",
    "wdspec"
   ],
+  "webdriver/tests/execute_script/cyclic.py": [
+   "cbebfbd2413ea0b10f547ab66fcc7159898e684a",
+   "wdspec"
+  ],
   "webdriver/tests/execute_script/user_prompts.py": [
    "901487f8270dcce693867ca090393e093d26f22b",
    "wdspec"
   ],
   "webdriver/tests/fullscreen_window.py": [
    "817011a8cdff7cfd7e445fb8ecb84e5d91f03993",
    "wdspec"
   ],
--- a/testing/web-platform/meta/webdriver/tests/contexts/json_serialize_windowproxy.py.ini
+++ b/testing/web-platform/meta/webdriver/tests/contexts/json_serialize_windowproxy.py.ini
@@ -1,10 +1,10 @@
 [json_serialize_windowproxy.py]
-  expected: TIMEOUT
+
   [json_serialize_windowproxy.py::test_initial_window]
     expected: FAIL
 
   [json_serialize_windowproxy.py::test_window_open]
     expected: FAIL
 
   [json_serialize_windowproxy.py::test_frame]
     expected: FAIL
--- a/testing/web-platform/meta/webdriver/tests/execute_script/user_prompts.py.ini
+++ b/testing/web-platform/meta/webdriver/tests/execute_script/user_prompts.py.ini
@@ -1,11 +1,9 @@
 [user_prompts.py]
-  expected: ERROR
-
   [user_prompts.py::test_handle_prompt_accept]
     expected: FAIL
 
   [user_prompts.py::test_handle_prompt_dismiss]
     expected: FAIL
 
   [user_prompts.py::test_handle_prompt_dismiss_and_notify]
     expected: FAIL
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/webdriver/tests/execute_script/cyclic.py
@@ -0,0 +1,48 @@
+from tests.support.asserts import assert_error
+
+
+def execute_script(session, script, args=None):
+    if args is None:
+        args = []
+    body = {"script": script, "args": args}
+    return session.transport.send(
+        "POST",
+        "/session/{session_id}/execute/sync".format(
+            session_id=session.session_id),
+        body)
+
+
+def test_array(session):
+    response = execute_script(session, """
+        let arr = [];
+        arr.push(arr);
+        return arr;
+        """)
+    assert_error(response, "javascript error")
+
+
+def test_object(session):
+    response = execute_script(session, """
+        let obj = {};
+        obj.reference = obj;
+        return obj;
+        """)
+    assert_error(response, "javascript error")
+
+
+def test_array_in_object(session):
+    response = execute_script(session, """
+        let arr = [];
+        arr.push(arr);
+        return {arr};
+        """)
+    assert_error(response, "javascript error")
+
+
+def test_object_in_array(session):
+    response = execute_script(session, """
+        let obj = {};
+        obj.reference = obj;
+        return [obj];
+        """)
+    assert_error(response, "javascript error")