Bug 1272953 - Error handling for WebExtension cookies API draft
authorJesper Kristensen <mail@jesperkristensen.dk>
Sun, 08 May 2016 23:24:35 +0200
changeset 417408 b0e66ff8c416f2a3e647a24bad3a5df489de3a82
parent 417407 439756bf71490136a1c29badf14cd72967b825fc
child 532086 c131661e1db88b57e8bbb7246ffb862aa1474b34
push id30398
push usermail@jesperkristensen.dk
push dateSun, 25 Sep 2016 10:35:31 +0000
bugs1272953
milestone52.0a1
Bug 1272953 - Error handling for WebExtension cookies API MozReview-Commit-ID: K8XhX6bn232
toolkit/components/extensions/ext-cookies.js
toolkit/components/extensions/test/mochitest/test_ext_cookies.html
--- a/toolkit/components/extensions/ext-cookies.js
+++ b/toolkit/components/extensions/ext-cookies.js
@@ -11,17 +11,17 @@ var {
 
 var DEFAULT_STORE = "firefox-default";
 var PRIVATE_STORE = "firefox-private";
 
 global.getCookieStoreIdForTab = function(tab) {
   return tab.incognito ? PRIVATE_STORE : DEFAULT_STORE;
 };
 
-function convert({cookie, isPrivate}) {
+function convert(cookie, isPrivate) {
   let result = {
     name: cookie.name,
     value: cookie.value,
     domain: cookie.host,
     hostOnly: !cookie.isDomain,
     path: cookie.path,
     secure: cookie.isSecure,
     httpOnly: cookie.isHttpOnly,
@@ -125,52 +125,63 @@ function checkSetCookiePermissions(exten
 
   // We don't do any significant checking of path permissions. RFC2109
   // suggests we only allow sites to add cookies for sub-paths, similar to
   // same origin policy enforcement, but no-one implements this.
 
   return true;
 }
 
-function* query(detailsIn, props, context) {
+function checkGetCookiePermissions(extension, url) {
+  let uri;
+  try {
+    uri = NetUtil.newURI(url).QueryInterface(Ci.nsIURL);
+  } catch (ex) {
+    // This often happens for about: URLs
+    return false;
+  }
+
+  if (uri.scheme != "http" && uri.scheme != "https") {
+    return false;
+  }
+
+  if (!extension.whiteListedHosts.matchesIgnoringPath(uri)) {
+    return false;
+  }
+
+  return true;
+}
+
+function* query(detailsIn, props, extension, isPrivate) {
   // Different callers want to filter on different properties. |props|
   // tells us which ones they're interested in.
   let details = {};
   props.forEach(property => {
     if (detailsIn[property] !== null) {
       details[property] = detailsIn[property];
     }
   });
 
   if ("domain" in details) {
     details.domain = details.domain.toLowerCase().replace(/^\./, "");
   }
 
-  let isPrivate = context.incognito;
-  if(details.storeId == DEFAULT_STORE) {
-    isPrivate = false;
-  } else if (details.storeId == PRIVATE_STORE) {
-    isPrivate = true;
-  } else if ("storeId" in details) {
-    return;
-  }
-
   // We can use getCookiesFromHost for faster searching.
   let enumerator;
   let uri;
   if ("url" in details) {
     try {
       uri = NetUtil.newURI(details.url).QueryInterface(Ci.nsIURL);
-      Services.cookies.usePrivateMode(isPrivate, () => {
-        enumerator = Services.cookies.getCookiesFromHost(uri.host, {});
-      });
     } catch (ex) {
       // This often happens for about: URLs
       return;
     }
+    Services.cookies.usePrivateMode(isPrivate, () => {
+      enumerator = Services.cookies.getCookiesFromHost(uri.host, {});
+    });
   } else if ("domain" in details) {
     Services.cookies.usePrivateMode(isPrivate, () => {
       enumerator = Services.cookies.getCookiesFromHost(details.domain, {});
     });
   } else {
     Services.cookies.usePrivateMode(isPrivate, () => {
       enumerator = Services.cookies.enumerator;
     });
@@ -233,48 +244,70 @@ function* query(detailsIn, props, contex
       return false;
     }
 
     if ("session" in details && details.session != cookie.isSession) {
       return false;
     }
 
     // Check that the extension has permissions for this host.
-    if (!context.extension.whiteListedHosts.matchesCookie(cookie)) {
+    if (!extension.whiteListedHosts.matchesCookie(cookie)) {
       return false;
     }
 
     return true;
   }
 
   while (enumerator.hasMoreElements()) {
     let cookie = enumerator.getNext().QueryInterface(Ci.nsICookie2);
     if (matches(cookie)) {
-      yield {cookie, isPrivate};
+      yield cookie;
     }
   }
 }
 
 extensions.registerSchemaAPI("cookies", "addon_parent", context => {
   let {extension} = context;
   let self = {
     cookies: {
       get: function(details) {
+        let isPrivate = context.incognito;
+        if(details.storeId == DEFAULT_STORE) {
+          isPrivate = false;
+        } else if (details.storeId == PRIVATE_STORE) {
+          isPrivate = true;
+        } else if (details.storeId !== null) {
+          return Promise.reject({message: "Unknown storeId"});
+        }
+
+        if (!checkGetCookiePermissions(extension, details.url)) {
+          return Promise.reject({message: `No host permissions for cookies at url: "${details.url}".`});
+        }
+
         // FIXME: We don't sort by length of path and creation time.
-        for (let cookie of query(details, ["url", "name", "storeId"], context)) {
-          return Promise.resolve(convert(cookie));
+        for (let cookie of query(details, ["url", "name", "storeId"], extension, isPrivate)) {
+          return Promise.resolve(convert(cookie, isPrivate));
         }
 
         // Found no match.
         return Promise.resolve(null);
       },
 
       getAll: function(details) {
+        let isPrivate = context.incognito;
+        if(details.storeId == DEFAULT_STORE) {
+          isPrivate = false;
+        } else if (details.storeId == PRIVATE_STORE) {
+          isPrivate = true;
+        } else if (details.storeId !== null) {
+          return Promise.reject({message: "Unknown storeId"});
+        }
+
         let allowed = ["url", "name", "domain", "path", "secure", "session", "storeId"];
-        let result = Array.from(query(details, allowed, context), convert);
+        let result = Array.from(query(details, allowed, extension, isPrivate), cookie => convert(cookie, isPrivate));
 
         return Promise.resolve(result);
       },
 
       set: function(details) {
         let uri = NetUtil.newURI(details.url).QueryInterface(Ci.nsIURL);
 
         let path;
@@ -314,17 +347,30 @@ extensions.registerSchemaAPI("cookies", 
           Services.cookies.add(cookieAttrs.host, path, name, value,
                                secure, httpOnly, isSession, expiry, {});
         });
 
         return self.cookies.get(details);
       },
 
       remove: function(details) {
-        for (let {cookie, isPrivate} of query(details, ["url", "name", "storeId"], context)) {
+        let isPrivate = context.incognito;
+        if(details.storeId == DEFAULT_STORE) {
+          isPrivate = false;
+        } else if (details.storeId == PRIVATE_STORE) {
+          isPrivate = true;
+        } else if (details.storeId !== null) {
+          return Promise.reject({message: "Unknown storeId"});
+        }
+
+        if (!checkGetCookiePermissions(extension, details.url)) {
+          return Promise.reject({message: `No host permissions for cookies at url: "${details.url}".`});
+        }
+
+        for (let cookie of query(details, ["url", "name", "storeId"], extension, isPrivate)) {
           Services.cookies.usePrivateMode(isPrivate, () => {
             Services.cookies.remove(cookie.host, cookie.name, cookie.path, false, cookie.originAttributes);
           });
           // Todo: could there be multiple per subdomain?
           return Promise.resolve({
             url: details.url,
             name: details.name,
             storeId: isPrivate ? PRIVATE_STORE : DEFAULT_STORE,
@@ -358,17 +404,17 @@ extensions.registerSchemaAPI("cookies", 
       },
 
       onChanged: new EventManager(context, "cookies.onChanged", fire => {
         let observer = (subject, topic, data) => {
           let notify = (removed, cookie, cause) => {
             cookie.QueryInterface(Ci.nsICookie2);
 
             if (extension.whiteListedHosts.matchesCookie(cookie)) {
-              fire({removed, cookie: convert({cookie, isPrivate: topic == "private-cookie-changed"}), cause});
+              fire({removed, cookie: convert(cookie, topic == "private-cookie-changed"), cause});
             }
           };
 
           // We do our best effort here to map the incompatible states.
           switch (data) {
             case "deleted":
               notify(true, subject, "explicit");
               break;
--- a/toolkit/components/extensions/test/mochitest/test_ext_cookies.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_cookies.html
@@ -67,19 +67,28 @@ add_task(function* test_cookies() {
       assertExpected(expected, cookies[0]);
       return browser.cookies.getAll({secure: true});
     }).then(cookies => {
       browser.test.assertEq(cookies.length, 0, "no secure cookies found");
       return browser.cookies.getAll({storeId: STORE_ID});
     }).then(cookies => {
       browser.test.assertEq(cookies.length, 1, "one cookie found for valid storeId");
       assertExpected(expected, cookies[0]);
-      return browser.cookies.getAll({storeId: "invalid_id"});
-    }).then(cookies => {
-      browser.test.assertEq(cookies.length, 0, "no cookies found for invalid storeId");
+      return browser.cookies.getAll({storeId: "invalid_id"}).then(cookie => {
+        browser.test.fail("using invalid storeId should reject");
+      }, error => {
+        browser.test.assertEq(error.message, "Unknown storeId");
+      });
+    }).then(() => {
+      return browser.cookies.get({url: "http://no-permission", name: "name1"}).then(cookie => {
+        browser.test.fail("Using url without permission should reject");
+      }, error => {
+        browser.test.assertEq(error.message, 'No host permissions for cookies at url: "http://no-permission".');
+      });
+    }).then(() => {
       return browser.cookies.remove({url: TEST_URL, name: "name1"});
     }).then(details => {
       assertExpected({url: TEST_URL, name: "name1", storeId: STORE_ID}, details);
       return browser.cookies.get({url: TEST_URL, name: "name1"});
     }).then(cookie => {
       browser.test.assertEq(null, cookie, "removed cookie not found");
       return browser.cookies.getAllCookieStores();
     }).then(stores => {
@@ -172,43 +181,52 @@ add_task(function* test_cookies() {
       assertExpected({url: TEST_URL, name: "name1", storeId: STORE_ID}, details);
       return browser.cookies.set({url: TEST_URL});
     }).then(cookie => {
       browser.test.assertEq("", cookie.name, "default name set");
       browser.test.assertEq("", cookie.value, "default value set");
       browser.test.assertEq(true, cookie.session, "no expiry date created session cookie");
       return browser.windows.create({incognito: true});
     }).then(privateWindow => {
-      return browser.cookies.set({url: TEST_URL, name: "store", value: "private", expirationDate: THE_FUTURE, storeId: PRIVATE_STORE_ID}).then(cookie => {
+      return browser.cookies.set({url: TEST_URL, name: "store", value: "private", expirationDate: THE_FUTURE, storeId: PRIVATE_STORE_ID}).then(() => {
         return browser.cookies.set({url: TEST_URL, name: "store", value: "default", expirationDate: THE_FUTURE, storeId: STORE_ID});
-      }).then(cookie => {
+      }).then(() => {
         return browser.cookies.get({url: TEST_URL, name: "store", storeId: PRIVATE_STORE_ID});
       }).then(cookie => {
         browser.test.assertEq("private", cookie.value, "get the private cookie");
         browser.test.assertEq(PRIVATE_STORE_ID, cookie.storeId, "get the private cookie storeId");
         return browser.cookies.get({url: TEST_URL, name: "store", storeId: STORE_ID});
       }).then(cookie => {
         browser.test.assertEq("default", cookie.value, "get the default cookie");
         browser.test.assertEq(STORE_ID, cookie.storeId, "get the default cookie storeId");
+        return browser.cookies.set({url: TEST_URL, name: "store", value: "invalid", expirationDate: THE_FUTURE, storeId: "invalid"}).then(() => {
+          browser.test.fail("using invalid storeId should reject");
+        }, error => {
+          browser.test.assertEq(error.message, "Unknown storeId");
+        });
+      }).then(() => {
         return browser.cookies.remove({url: TEST_URL, name: "store", storeId: STORE_ID});
       }).then(details => {
         assertExpected({url: TEST_URL, name: "store", storeId: STORE_ID}, details);
         return browser.cookies.get({url: TEST_URL, name: "store", storeId: STORE_ID});
       }).then(cookie => {
         browser.test.assertEq(null, cookie, "deleted the default cookie");
         return browser.cookies.remove({url: TEST_URL, name: "store", storeId: PRIVATE_STORE_ID});
       }).then(details => {
         assertExpected({url: TEST_URL, name: "store", storeId: PRIVATE_STORE_ID}, details);
         return browser.cookies.get({url: TEST_URL, name: "store", storeId: PRIVATE_STORE_ID});
       }).then(cookie => {
         browser.test.assertEq(null, cookie, "deleted the private cookie");
         return browser.windows.remove(privateWindow.id);
       });
     }).then(() => {
       browser.test.notifyPass("cookies");
+    }, error => {
+      browser.test.fail(`Error: ${error} :: ${error.stack}`);
+      browser.test.notifyFail("cookies");
     });
   }
 
   let extension = ExtensionTestUtils.loadExtension({
     background,
     manifest: {
       permissions: ["cookies", "*://example.org/"],
     },