[ChromeDriver] Fix array serialization when Array.toJSON is present

When web page adds toJSON function to Array prototype, ChromeDriver incorrectly calls this function while doing JSON serialization of array objects. This is incorrect, as the W3C WebDriver spec (https://w3c.github.io/webdriver/#dfn-internal-json-clone-algorithm) explicitly specifies that implementation should check for collection before looking for the toJSON function. Bug: chromedriver:3084 Change-Id: Ib2ed0c79833a6ca557d33595687efd70ade7d8c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1749617 Commit-Queue: John Chen <johnchen@chromium.org> Reviewed-by: Caleb Rouleau <crouleau@chromium.org> Reviewed-by: Rohan Pavone <rohpavone@chromium.org> Cr-Commit-Position: refs/heads/master@{#687307}
chromium · Aug 15, 2019 · 467762a · 467762a
1 parent 45822db
commit 467762a
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 7 deletions.
diff --git a/chrome/test/chromedriver/js/call_function.js b/chrome/test/chromedriver/js/call_function.js
@@ -318,6 +318,22 @@ function cloneWithAlgorithm(item, seen, algo, opt_cache) {
   return tmp;
 }
 
+/**
+ * Wrapper to cloneWithAlgorithm, with circular reference detection logic.
+ * @param {*} item Object or collection to deep clone.
+ * @param {!Array<*>} seen Object references that have already been seen.
+ * @param {function(*, Array<*>, ?Cache) : *} algo Cloning algorithm to use to
+ *     deep clone properties of item.
+ * @return {*} Clone of item with status of cloning.
+ */
+function cloneWithCircularCheck(item, seen, algo) {
+  if (seen.includes(item))
+    throw newError('circular reference', StatusCode.JAVA_SCRIPT_ERROR);
+  seen.push(item);
+  const result = cloneWithAlgorithm(item, seen, algo);
+  seen.pop();
+  return result;
+}
 
 /**
  * Returns deep clone of given value, replacing element references with a
@@ -343,6 +359,8 @@ function jsonSerialize(item, seen) {
     ret[ELEMENT_KEY] = cache.storeItem(item);
     return ret;
   }
+  if (isCollection(item))
+    return cloneWithCircularCheck(item, seen, jsonSerialize);
   // http://crbug.com/chromedriver/2995: Placed here because some element
   // (above) are type 'function', so this check must be performed after.
   if (typeof item === 'function')
@@ -353,13 +371,8 @@ function jsonSerialize(item, seen) {
        Object.getPrototypeOf(item).hasOwnProperty('toJSON')))
     return item.toJSON();
 
-  // Deep clone collections and Objects.
-  if (seen.includes(item))
-    throw newError('circular reference', StatusCode.JAVA_SCRIPT_ERROR);
-  seen.push(item);
-  const result = cloneWithAlgorithm(item, seen, jsonSerialize);
-  seen.pop();
-  return result;
+  // Deep clone Objects.
+  return cloneWithCircularCheck(item, seen, jsonSerialize);
 }
 
 /**

diff --git a/chrome/test/chromedriver/js/call_function_test.html b/chrome/test/chromedriver/js/call_function_test.html
@@ -314,6 +314,7 @@
 }
 
 // Verify callFunction works when Object.prototype has user-defined functions.
+// (https://crbug.com/chromedriver/3074)
 function testCallWithFunctionInObject(runner) {
   clearCache();
 
@@ -329,6 +330,25 @@
   runner.waitForAsync();
 }
 
+// Verify array serialization works when Array.prototype.toJSON is defined.
+// (https://crbug.com/chromedriver/3084)
+function testCallWithArrayToJSON(runner) {
+  clearCache();
+
+  Array.prototype.toJSON = () => '["testing"]';
+  function func() {
+    return [1, 2, 3];
+  }
+  callFunction(func, []).then((result) => {
+    assert(result.value instanceof Array);
+    assertEquals(result.value.length, 3);
+    assertEquals(result.value[2], 3);
+    delete Array.prototype.toJSON;
+    runner.continueTesting();
+  });
+  runner.waitForAsync();
+}
+
 </script>
 <body>
 <div><span></span></div>