Bug 1258139: Part 2 - [webext] Fix dead wrapper issues in storage API. r?gabor draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 25 Mar 2016 01:14:39 +0100
changeset 344620 7103fe5bfd80c7395a6e4d30dc94755720066a78
parent 344619 01b032741b88c3858b4b42b72be3cbf34fe9e2bc
child 517007 61dde8e708c4171762d8bc48e29ac50045953e55
push id13883
push usermaglione.k@gmail.com
push dateFri, 25 Mar 2016 00:17:17 +0000
reviewersgabor
bugs1258139
milestone48.0a1
Bug 1258139: Part 2 - [webext] Fix dead wrapper issues in storage API. r?gabor MozReview-Commit-ID: 7F0oBpjl7Qt
toolkit/components/extensions/ExtensionStorage.jsm
toolkit/components/extensions/ext-storage.js
toolkit/components/extensions/test/mochitest/mochitest.ini
toolkit/components/extensions/test/mochitest/test_ext_storage.html
toolkit/components/extensions/test/mochitest/test_ext_storage_tab.html
--- a/toolkit/components/extensions/ExtensionStorage.jsm
+++ b/toolkit/components/extensions/ExtensionStorage.jsm
@@ -16,22 +16,80 @@ Cu.import("resource://gre/modules/Servic
 Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://gre/modules/AsyncShutdown.jsm");
 
 /* globals OS ExtensionStorage */
 
 var Path = OS.Path;
 var profileDir = OS.Constants.Path.profileDir;
 
+function jsonReplacer(key, value) {
+  switch (typeof(value)) {
+    // Serialize primitive types as-is.
+    case "string":
+    case "number":
+    case "boolean":
+      return value;
+
+    case "object":
+      if (value === null) {
+        return value;
+      }
+
+      switch (Cu.getClassName(value, true)) {
+        // Serialize arrays and ordinary objects as-is.
+        case "Array":
+        case "Object":
+          return value;
+
+        // Serialize Date objects and regular expressions as their
+        // string representations.
+        case "Date":
+        case "RegExp":
+          return String(value);
+      }
+      break;
+  }
+
+  if (!key) {
+    // If this is the root object, and we can't serialize it, serialize
+    // the value to an empty object.
+    return {};
+  }
+
+  // Everything else, omit entirely.
+  return undefined;
+}
+
 this.ExtensionStorage = {
   cache: new Map(),
   listeners: new Map(),
 
   extensionDir: Path.join(profileDir, "browser-extension-data"),
 
+  /**
+   * Sanitizes the given value, and returns a JSON-compatible
+   * representation of it, based on the privileges of the given global.
+   */
+  sanitize(value, global) {
+    // We can't trust that the global has privileges to access this
+    // value enough to clone it using a privileged JSON object. And JSON
+    // objects don't support X-ray wrappers, so we can't use the JSON
+    // object from the unprivileged global directly, either.
+    //
+    // So, instead, we create a new one, which we know is clean,
+    // belonging to the same principal as the unprivileged scope, and
+    // use that instead.
+    let JSON_ = Cu.waiveXrays(Cu.Sandbox(global).JSON);
+
+    let json = JSON_.stringify(value, jsonReplacer);
+
+    return JSON.parse(json);
+  },
+
   getExtensionDir(extensionId) {
     return Path.join(this.extensionDir, extensionId);
   },
 
   getStorageFile(extensionId) {
     return Path.join(this.extensionDir, extensionId, "storage.js");
   },
 
@@ -70,22 +128,23 @@ this.ExtensionStorage = {
       "ExtensionStorage: Finish writing extension data",
       promise);
 
     return promise.then(() => {
       AsyncShutdown.profileBeforeChange.removeBlocker(promise);
     });
   },
 
-  set(extensionId, items) {
+  set(extensionId, items, global) {
     return this.read(extensionId).then(extData => {
       let changes = {};
       for (let prop in items) {
-        changes[prop] = {oldValue: extData[prop], newValue: items[prop]};
-        extData[prop] = items[prop];
+        let item = this.sanitize(items[prop], global);
+        changes[prop] = {oldValue: extData[prop], newValue: item};
+        extData[prop] = item;
       }
 
       this.notifyListeners(extensionId, changes);
 
       return this.write(extensionId);
     });
   },
 
--- a/toolkit/components/extensions/ext-storage.js
+++ b/toolkit/components/extensions/ext-storage.js
@@ -13,17 +13,17 @@ var {
 extensions.registerSchemaAPI("storage", "storage", (extension, context) => {
   return {
     storage: {
       local: {
         get: function(keys) {
           return ExtensionStorage.get(extension.id, keys);
         },
         set: function(items) {
-          return ExtensionStorage.set(extension.id, items);
+          return ExtensionStorage.set(extension.id, items, context.cloneScope);
         },
         remove: function(items) {
           return ExtensionStorage.remove(extension.id, items);
         },
         clear: function() {
           return ExtensionStorage.clear(extension.id);
         },
       },
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -53,16 +53,18 @@ skip-if = buildapp == 'b2g' # port.sende
 [test_ext_sandbox_var.html]
 [test_ext_sendmessage_reply.html]
 skip-if = buildapp == 'b2g' # sender.tab is undefined on b2g.
 [test_ext_sendmessage_reply2.html]
 skip-if = buildapp == 'b2g' # sender.tab is undefined on b2g.
 [test_ext_sendmessage_doublereply.html]
 skip-if = buildapp == 'b2g' # sender.tab is undefined on b2g.
 [test_ext_storage.html]
+[test_ext_storage_tab.html]
+skip-if = os == 'android' # Android does not currently support tabs.
 [test_ext_background_runtime_connect_params.html]
 [test_ext_cookies.html]
 [test_ext_cookies_permissions.html]
 skip-if = e10s || buildapp == 'b2g' # Uses cookie service via SpecialPowers.Services, which does not support e10s.
 [test_ext_bookmarks.html]
 skip-if = buildapp == 'b2g' # unimplemented api.
 [test_ext_alarms.html]
 [test_ext_background_window_properties.html]
--- a/toolkit/components/extensions/test/mochitest/test_ext_storage.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_storage.html
@@ -117,33 +117,47 @@ function backgroundScript() {
     return new Promise(resolve => browser.test.onMessage.addListener(resolve));
   }).then(() => {
     return check("test-prop1", "value1");
   }).then(() => {
     return check("test-prop2", "value2");
 
   // Make sure we can store complex JSON data.
   }).then(() => {
-    return storage.set({"test-prop1": {str: "hello", bool: true, undef: undefined, obj: {}, arr: [1, 2]}});
+    return storage.set({"test-prop1": {str: "hello", bool: true, undef: undefined, obj: {}, arr: [1, 2],
+                                       date: new Date(0),
+                                       regexp: /regexp/,
+                                       func: function func() {}, window}});
+  }).then(() => {
+    return storage.set({"test-prop2": function func() {}});
   }).then(() => {
     browser.test.assertEq("value1", globalChanges["test-prop1"].oldValue, "oldValue correct");
     browser.test.assertEq("object", typeof(globalChanges["test-prop1"].newValue), "newValue is obj");
     globalChanges = {};
-    return storage.get({"test-prop1": undefined});
+    return storage.get({"test-prop1": undefined, "test-prop2": undefined});
   }).then(data => {
     let obj = data["test-prop1"];
 
     browser.test.assertEq("hello", obj.str, "string part correct");
     browser.test.assertEq(true, obj.bool, "bool part correct");
     browser.test.assertEq(undefined, obj.undef, "undefined part correct");
+    browser.test.assertEq(undefined, obj.func, "function part correct");
+    browser.test.assertEq(undefined, obj.window, "window part correct");
+    browser.test.assertEq("1970-01-01T00:00:00.000Z", obj.date, "date part correct");
+    browser.test.assertEq("/regexp/", obj.regexp, "date part correct");
     browser.test.assertEq("object", typeof(obj.obj), "object part correct");
     browser.test.assertTrue(Array.isArray(obj.arr), "array part present");
     browser.test.assertEq(1, obj.arr[0], "arr[0] part correct");
     browser.test.assertEq(2, obj.arr[1], "arr[1] part correct");
     browser.test.assertEq(2, obj.arr.length, "arr.length part correct");
+
+    obj = data["test-prop2"];
+
+    browser.test.assertEq("[object Object]", {}.toString.call(obj), "function serialized as a plain object");
+    browser.test.assertEq(0, Object.keys(obj).length, "function serialized as an empty object");
   }).then(() => {
     browser.test.notifyPass("storage");
   }).catch(e => {
     browser.test.fail(`Error: ${e} :: ${e.stack}`);
     browser.test.notifyFail("storage");
   });
 }
 
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_storage_tab.html
@@ -0,0 +1,117 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>WebExtension test</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+add_task(function* test_multiple_pages() {
+  function background() {
+    let tabReady = new Promise(resolve => {
+      browser.runtime.onMessage.addListener(function listener(msg) {
+        browser.test.log("onMessage " + msg);
+        if (msg == "tab-ready") {
+          browser.runtime.onMessage.removeListener(listener);
+          resolve();
+        }
+      });
+    });
+
+    let tabId;
+    let tabRemoved = new Promise(resolve => {
+      browser.tabs.onRemoved.addListener(function listener(removedId) {
+        if (removedId == tabId) {
+          browser.tabs.onRemoved.removeListener(listener);
+
+          // Delay long enough to be sure the inner window has been nuked.
+          setTimeout(resolve, 0);
+        }
+      });
+    });
+
+    let storage = browser.storage.local;
+
+    browser.test.log("create");
+    browser.tabs.create({url: "tab.html"}).then(tab => {
+      tabId = tab.id;
+
+      return tabReady;
+    }).then(() => {
+      return storage.get("key");
+    }).then(result => {
+      browser.test.assertEq(undefined, result.key, "Key should be undefined");
+
+      return browser.runtime.sendMessage("tab-set-key");
+    }).then(() => {
+      return storage.get("key");
+    }).then(result => {
+      browser.test.assertEq(JSON.stringify({foo: {bar: "baz"}}),
+                            JSON.stringify(result.key),
+                            "Key should be set to the value from the tab");
+    }).then(() => {
+      browser.test.log("Remove tab");
+      return Promise.all([browser.tabs.remove(tabId),
+                          tabRemoved]);
+    }).then(() => {
+      return storage.get("key");
+    }).then(result => {
+      browser.test.assertEq(JSON.stringify({foo: {bar: "baz"}}),
+                            JSON.stringify(result.key),
+                            "Key should still be set to the value from the tab");
+    }).then(() => {
+      browser.test.notifyPass("storage-multiple");
+    }).catch(e => {
+      browser.test.fail(`Error: ${e} :: ${e.stack}`);
+      browser.test.notifyFail("storage-multiple");
+    });
+  }
+
+  function tab() {
+    browser.test.log("tab");
+    browser.runtime.onMessage.addListener(msg => {
+      if (msg == "tab-set-key") {
+        return browser.storage.local.set({key: {foo: {bar: "baz"}}});
+      }
+    });
+
+    browser.runtime.sendMessage("tab-ready");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    background: `(${background})()`,
+
+    files: {
+      "tab.html": `<!DOCTYPE html>
+        <html>
+          <head>
+            <meta charset="utf-8">
+            <script src="tab.js"></${"script"}>
+          </head>
+        </html>`,
+
+      "tab.js": `(${tab})()`,
+    },
+
+    manifest: {
+      permissions: ["storage"],
+    },
+  });
+
+  yield extension.startup();
+
+  yield extension.awaitFinish("storage-multiple");
+  yield extension.unload();
+});
+
+</script>
+
+</body>
+</html>