Bug 1254221 - Error handling for WebExtension cookies API
MozReview-Commit-ID: EXANQbcJyeA
--- a/toolkit/components/extensions/ext-cookies.js
+++ b/toolkit/components/extensions/ext-cookies.js
@@ -125,156 +125,167 @@ 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, 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];
+extensions.registerSchemaAPI("cookies", "cookies", (extension, context) => {
+ function* query(detailsIn, props) {
+ // 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(/^\./, "");
}
- });
-
- if ("domain" in details) {
- details.domain = details.domain.toLowerCase().replace(/^\./, "");
- }
- if(details.storeId == DEFAULT_STORE) {
- isPrivate = false;
- } else if (details.storeId == PRIVATE_STORE) {
- isPrivate = true;
- } else if ("storeId" in details) {
- return;
- }
+ let isPrivate = context.incognito;
+ if(details.storeId == DEFAULT_STORE) {
+ isPrivate = false;
+ } else if (details.storeId == PRIVATE_STORE) {
+ isPrivate = true;
+ } else if ("storeId" in details) {
+ throw new context.cloneScope.Error("Unknown storeId");
+ }
- // We can use getCookiesFromHost for faster searching.
- let enumerator;
- let uri;
- if ("url" in details) {
- try {
- uri = NetUtil.newURI(details.url).QueryInterface(Ci.nsIURL);
+ // We can use getCookiesFromHost for faster searching.
+ let enumerator;
+ let uri;
+ if ("url" in details) {
+ try {
+ uri = NetUtil.newURI(details.url).QueryInterface(Ci.nsIURL);
+ } catch (ex) {
+ // This often happens for about: URLs
+ }
+ if (!uri || uri.scheme != "http" && uri.scheme != "https" ||
+ !extension.whiteListedHosts.matchesIgnoringPath(uri)) {
+ if (props.includes("domain")) {
+ return; // Scilently ignore for getAll
+ }
+ throw new context.cloneScope.Error(`No host permissions for cookies at url: "${details.url}".`);
+ }
Services.cookies.usePrivateMode(isPrivate, () => {
enumerator = Services.cookies.getCookiesFromHost(uri.host);
});
- } catch (ex) {
- // This often happens for about: URLs
- return;
- }
- } else if ("domain" in details) {
- Services.cookies.usePrivateMode(isPrivate, () => {
- enumerator = Services.cookies.getCookiesFromHost(details.domain);
- });
- } else {
- Services.cookies.usePrivateMode(isPrivate, () => {
- enumerator = Services.cookies.enumerator;
- });
- }
-
- // Based on nsCookieService::GetCookieStringInternal
- function matches(cookie) {
- function domainMatches(host) {
- return cookie.rawHost == host || (cookie.isDomain && host.endsWith(cookie.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;
+ });
}
- function pathMatches(path) {
- let cookiePath = cookie.path.replace(/\/$/, "");
+ // Based on nsCookieService::GetCookieStringInternal
+ function matches(cookie) {
+ function domainMatches(host) {
+ return cookie.rawHost == host || (cookie.isDomain && host.endsWith(cookie.host));
+ }
+
+ function pathMatches(path) {
+ let cookiePath = cookie.path.replace(/\/$/, "");
+
+ if (!path.startsWith(cookiePath)) {
+ return false;
+ }
+
+ // path == cookiePath, but without the redundant string compare.
+ if (path.length == cookiePath.length) {
+ return true;
+ }
- if (!path.startsWith(cookiePath)) {
+ // URL path is a substring of the cookie path, so it matches if, and
+ // only if, the next character is a path delimiter.
+ let pathDelimiters = ["/", "?", "#", ";"];
+ return pathDelimiters.includes(path[cookiePath.length]);
+ }
+
+ // "Restricts the retrieved cookies to those that would match the given URL."
+ if (uri) {
+ if (!domainMatches(uri.host)) {
+ return false;
+ }
+
+ if (cookie.isSecure && uri.scheme != "https") {
+ return false;
+ }
+
+ if (!pathMatches(uri.path)) {
+ return false;
+ }
+ }
+
+ if ("name" in details && details.name != cookie.name) {
return false;
}
- // path == cookiePath, but without the redundant string compare.
- if (path.length == cookiePath.length) {
- return true;
+ // "Restricts the retrieved cookies to those whose domains match or are subdomains of this one."
+ if ("domain" in details && !isSubdomain(cookie.rawHost, details.domain)) {
+ return false;
}
- // URL path is a substring of the cookie path, so it matches if, and
- // only if, the next character is a path delimiter.
- let pathDelimiters = ["/", "?", "#", ";"];
- return pathDelimiters.includes(path[cookiePath.length]);
- }
-
- // "Restricts the retrieved cookies to those that would match the given URL."
- if (uri) {
- if (!domainMatches(uri.host)) {
+ // "Restricts the retrieved cookies to those whose path exactly matches this string.""
+ if ("path" in details && details.path != cookie.path) {
return false;
}
- if (cookie.isSecure && uri.scheme != "https") {
+ if ("secure" in details && details.secure != cookie.isSecure) {
+ return false;
+ }
+
+ if ("session" in details && details.session != cookie.isSession) {
+ return false;
+ }
+
+ // Check that the extension has permissions for this host.
+ if (!extension.whiteListedHosts.matchesCookie(cookie)) {
return false;
}
- if (!pathMatches(uri.path)) {
- return false;
- }
- }
-
- if ("name" in details && details.name != cookie.name) {
- return false;
- }
-
- // "Restricts the retrieved cookies to those whose domains match or are subdomains of this one."
- if ("domain" in details && !isSubdomain(cookie.rawHost, details.domain)) {
- return false;
- }
-
- // "Restricts the retrieved cookies to those whose path exactly matches this string.""
- if ("path" in details && details.path != cookie.path) {
- return false;
+ return true;
}
- if ("secure" in details && details.secure != cookie.isSecure) {
- return false;
- }
-
- if ("session" in details && details.session != cookie.isSession) {
- return false;
+ while (enumerator.hasMoreElements()) {
+ let cookie = enumerator.getNext().QueryInterface(Ci.nsICookie2);
+ if (matches(cookie)) {
+ yield {cookie, isPrivate};
+ }
}
-
- // Check that the extension has permissions for this host.
- 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};
- }
- }
-}
-
-extensions.registerSchemaAPI("cookies", "cookies", (extension, context) => {
let self = {
cookies: {
get: function(details) {
- // FIXME: We don't sort by length of path and creation time.
- for (let cookie of query(details, ["url", "name", "storeId"], extension, context.incognito)) {
- return Promise.resolve(convert(cookie));
- }
+ return Promise.resolve().then(() => {
+ // FIXME: We don't sort by length of path and creation time.
+ for (let cookie of query(details, ["url", "name", "storeId"])) {
+ return convert(cookie);
+ }
- // Found no match.
- return Promise.resolve(null);
+ // Found no match.
+ return null;
+ });
},
getAll: function(details) {
- let allowed = ["url", "name", "domain", "path", "secure", "session", "storeId"];
- let result = Array.from(query(details, allowed, extension, context.incognito), convert);
+ return Promise.resolve().then(() => {
+ let allowed = ["url", "name", "domain", "path", "secure", "session", "storeId"];
+ let result = Array.from(query(details, allowed), convert);
- return Promise.resolve(result);
+ return result;
+ });
},
set: function(details) {
let uri = NetUtil.newURI(details.url).QueryInterface(Ci.nsIURL);
let path;
if (details.path !== null) {
path = details.path;
@@ -293,48 +304,50 @@ extensions.registerSchemaAPI("cookies",
let isSession = details.expirationDate === null;
let expiry = isSession ? Number.MAX_SAFE_INTEGER : details.expirationDate;
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"});
+ return Promise.reject(new context.cloneScope.Error("Unknown storeId"));
}
let cookieAttrs = {host: details.domain, path: path, isSecure: secure};
if (!checkSetCookiePermissions(extension, uri, cookieAttrs)) {
- return Promise.reject({message: `Permission denied to set cookie ${JSON.stringify(details)}`});
+ return Promise.reject(new context.cloneScope.Error(`Permission denied to set cookie ${JSON.stringify(details)}`));
}
// The permission check may have modified the domain, so use
// the new value instead.
Services.cookies.usePrivateMode(isPrivate, () => {
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"], extension, context.incognito)) {
- 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,
- });
- }
+ return Promise.resolve().then(() => {
+ for (let {cookie, isPrivate} of query(details, ["url", "name", "storeId"])) {
+ Services.cookies.usePrivateMode(isPrivate, () => {
+ Services.cookies.remove(cookie.host, cookie.name, cookie.path, false, cookie.originAttributes);
+ });
+ // Todo: could there be multiple per subdomain?
+ return {
+ url: details.url,
+ name: details.name,
+ storeId: isPrivate ? PRIVATE_STORE : DEFAULT_STORE,
+ };
+ }
- return Promise.resolve(null);
+ return null;
+ });
},
getAllCookieStores: function() {
let defaultTabs = [];
let privateTabs = [];
for (let window of WindowListManager.browserWindows()) {
let tabs = TabManager.for(extension).getTabs(window);
for (let tab of tabs) {
--- 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"});
+ 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(cookies => {
- browser.test.assertEq(cookies.length, 0, "no cookies found for invalid storeId");
+ 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(cookies => {
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 => {
@@ -183,32 +192,40 @@ add_task(function* test_cookies() {
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(cookie => {
+ browser.test.fail("using invalid storeId should reject");
+ }, error => {
+ browser.test.assertEq(error.message, "Unknown storeId");
+ });
+ }).then(cookie => {
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.notifyFail("cookies: " + error);
});
}
let extension = ExtensionTestUtils.loadExtension({
background: `(${backgroundScript})()`,
manifest: {
permissions: ["cookies", "*://example.org/"],
},