Bug 1465063 - Return FP cookies from cookies.getAll even if domain/url is set draft
authorRob Wu <rob@robwu.nl>
Fri, 08 Jun 2018 17:20:02 +0200
changeset 805885 544c45fb747495d10558b087f006bfe463d0f210
parent 805884 d5ccbff0e13d5d6c6789b7f5fe70c50026cd51e2
push id112798
push userbmo:rob@robwu.nl
push dateFri, 08 Jun 2018 17:17:58 +0000
bugs1465063
milestone62.0a1
Bug 1465063 - Return FP cookies from cookies.getAll even if domain/url is set `browser.cookies.getAll(details)` is expected to return all cookies if the "details.firstPartyDomain" key is set to undefined/null. This usually works, because `getCookiesWithOriginAttributes` is used, which iterates over all cookies and only checks origin attributes that exist in the specified pattern. If a "url" or "domain" are set, then `getCookiesFromHost` was used, which uses a look-up table keyed by host and OriginAttributes, with undefined origin attributes initialized at their default value. Consequently, only non-FP cookies were returned. This commit fixes the issue by using `getCookiesWithOriginAttributes` when the `firstPartyDomain` does not have an explicit (string) value. MozReview-Commit-ID: AxHbbDeqSwr
toolkit/components/extensions/parent/ext-cookies.js
toolkit/components/extensions/test/mochitest/test_ext_cookies_first_party.html
--- a/toolkit/components/extensions/parent/ext-cookies.js
+++ b/toolkit/components/extensions/parent/ext-cookies.js
@@ -169,36 +169,43 @@ const query = function* (detailsIn, prop
   if (isPrivate) {
     storeId = PRIVATE_STORE;
   } else if ("storeId" in details) {
     storeId = details.storeId;
   }
 
   // We can use getCookiesFromHost for faster searching.
   let enumerator;
+  let host;
   let url;
   let originAttributes = {
     userContextId,
     privateBrowsingId: isPrivate ? 1 : 0,
   };
   if ("firstPartyDomain" in details) {
     originAttributes.firstPartyDomain = details.firstPartyDomain;
   }
   if ("url" in details) {
     try {
       url = new URL(details.url);
-      enumerator = Services.cookies.getCookiesFromHost(url.hostname, originAttributes);
+      host = url.hostname;
     } catch (ex) {
       // This often happens for about: URLs
       return;
     }
   } else if ("domain" in details) {
-    enumerator = Services.cookies.getCookiesFromHost(details.domain, originAttributes);
+    host = details.domain;
+  }
+
+  if (host && ("firstPartyDomain" in originAttributes)) {
+    // getCookiesFromHost is more efficient than getCookiesWithOriginAttributes
+    // if the host and all origin attributes are known.
+    enumerator = Services.cookies.getCookiesFromHost(host, originAttributes);
   } else {
-    enumerator = Services.cookies.getCookiesWithOriginAttributes(JSON.stringify(originAttributes));
+    enumerator = Services.cookies.getCookiesWithOriginAttributes(JSON.stringify(originAttributes), host);
   }
 
   // Based on nsCookieService::GetCookieStringInternal
   function matches(cookie) {
     function domainMatches(host) {
       return cookie.rawHost == host || (cookie.isDomain && host.endsWith(cookie.host));
     }
 
--- a/toolkit/components/extensions/test/mochitest/test_ext_cookies_first_party.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_cookies_first_party.html
@@ -78,38 +78,41 @@ async function background() {
     cookie = await browser.cookies.get({url, name: "foo2", firstPartyDomain: ""});
     assertExpectedCookies([null], [cookie], "get: FPI off, w/ empty firstPartyDomain, no cookie");
     cookie = await browser.cookies.get({url, name: "foo2", firstPartyDomain: null});
     assertExpectedCookies([null], [cookie], "get: FPI off, w/ null firstPartyDomain, no cookie");
     cookie = await browser.cookies.get({url, name: "foo2", firstPartyDomain: undefined});
     assertExpectedCookies([null], [cookie], "get: FPI off, w/ undefined firstPartyDomain, no cookie");
 
     // getAll
-    cookies = await browser.cookies.getAll({});
-    assertExpectedCookies([
-      {name: "foo1", value: "bar1", firstPartyDomain: ""},
-    ], cookies, "getAll: FPI off, w/o firstPartyDomain, non-FP cookies");
-    cookies = await browser.cookies.getAll({firstPartyDomain: ""});
-    assertExpectedCookies([
-      {name: "foo1", value: "bar1", firstPartyDomain: ""},
-    ], cookies, "getAll: FPI off, w/ empty firstPartyDomain, non-FP cookies");
-    cookies = await browser.cookies.getAll({firstPartyDomain: null});
-    assertExpectedCookies([
-      {name: "foo1", value: "bar1", firstPartyDomain: ""},
-      {name: "foo2", value: "bar2", firstPartyDomain},
-    ], cookies, "getAll: FPI off, w/ null firstPartyDomain, all cookies");
-    cookies = await browser.cookies.getAll({firstPartyDomain: undefined});
-    assertExpectedCookies([
-      {name: "foo1", value: "bar1", firstPartyDomain: ""},
-      {name: "foo2", value: "bar2", firstPartyDomain},
-    ], cookies, "getAll: FPI off, w/ undefined firstPartyDomain, all cookies");
-    cookies = await browser.cookies.getAll({firstPartyDomain});
-    assertExpectedCookies([
-      {name: "foo2", value: "bar2", firstPartyDomain},
-    ], cookies, "getAll: FPI off, w/ firstPartyDomain, FP cookies");
+    for (let extra of [{}, {url}, {domain: firstPartyDomain}]) {
+      const prefix = `getAll(${JSON.stringify(extra)})`;
+      cookies = await browser.cookies.getAll({...extra});
+      assertExpectedCookies([
+        {name: "foo1", value: "bar1", firstPartyDomain: ""},
+      ], cookies, `${prefix}: FPI off, w/o firstPartyDomain, non-FP cookies`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain: ""});
+      assertExpectedCookies([
+        {name: "foo1", value: "bar1", firstPartyDomain: ""},
+      ], cookies, `${prefix}: FPI off, w/ empty firstPartyDomain, non-FP cookies`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain: null});
+      assertExpectedCookies([
+        {name: "foo1", value: "bar1", firstPartyDomain: ""},
+        {name: "foo2", value: "bar2", firstPartyDomain},
+      ], cookies, `${prefix}: FPI off, w/ null firstPartyDomain, all cookies`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain: undefined});
+      assertExpectedCookies([
+        {name: "foo1", value: "bar1", firstPartyDomain: ""},
+        {name: "foo2", value: "bar2", firstPartyDomain},
+      ], cookies, `${prefix}: FPI off, w/ undefined firstPartyDomain, all cookies`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain});
+      assertExpectedCookies([
+        {name: "foo2", value: "bar2", firstPartyDomain},
+      ], cookies, `${prefix}: FPI off, w/ firstPartyDomain, FP cookies`);
+    }
 
     // remove
     cookie = await browser.cookies.remove({url, name: "foo1"});
     assertExpectedCookies([
       {url, name: "foo1", firstPartyDomain: ""},
     ], [cookie], "remove: FPI off, w/ empty firstPartyDomain, non-FP cookie");
     cookie = await browser.cookies.remove({url, name: "foo2", firstPartyDomain});
     assertExpectedCookies([
@@ -159,41 +162,44 @@ async function background() {
       {name: "foo4", value: "bar4", firstPartyDomain},
     ], [cookie], "get: FPI on, w/ firstPartyDomain, FP cookie");
     cookie = await browser.cookies.get({url, name: "foo2", firstPartyDomain});
     assertExpectedCookies([
       {name: "foo2", value: "bar2", firstPartyDomain},
     ], [cookie], "get: FPI on, w/ firstPartyDomain, FP cookie (set when FPI off)");
 
     // getAll
-    await browser.test.assertRejects(
-      browser.cookies.getAll({}),
-      expectedError,
-      "getAll: FPI on, w/o firstPartyDomain, rejection");
-    cookies = await browser.cookies.getAll({firstPartyDomain: ""});
-    assertExpectedCookies([
-      {name: "foo1", value: "bar1", firstPartyDomain: ""},
-    ], cookies, "getAll: FPI on, w/ empty firstPartyDomain, non-FP cookies");
-    cookies = await browser.cookies.getAll({firstPartyDomain: null});
-    assertExpectedCookies([
-      {name: "foo1", value: "bar1", firstPartyDomain: ""},
-      {name: "foo2", value: "bar2", firstPartyDomain},
-      {name: "foo4", value: "bar4", firstPartyDomain},
-    ], cookies, "getAll: FPI on, w/ null firstPartyDomain, all cookies");
-    cookies = await browser.cookies.getAll({firstPartyDomain: undefined});
-    assertExpectedCookies([
-      {name: "foo1", value: "bar1", firstPartyDomain: ""},
-      {name: "foo2", value: "bar2", firstPartyDomain},
-      {name: "foo4", value: "bar4", firstPartyDomain},
-    ], cookies, "getAll: FPI on, w/ undefined firstPartyDomain, all cookies");
-    cookies = await browser.cookies.getAll({firstPartyDomain});
-    assertExpectedCookies([
-      {name: "foo2", value: "bar2", firstPartyDomain},
-      {name: "foo4", value: "bar4", firstPartyDomain},
-    ], cookies, "getAll: FPI on, w/ firstPartyDomain, FP cookies");
+    for (let extra of [{}, {url}, {domain: firstPartyDomain}]) {
+      const prefix = `getAll(${JSON.stringify(extra)})`;
+      await browser.test.assertRejects(
+        browser.cookies.getAll({...extra}),
+        expectedError,
+        `${prefix}: FPI on, w/o firstPartyDomain, rejection`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain: ""});
+      assertExpectedCookies([
+        {name: "foo1", value: "bar1", firstPartyDomain: ""},
+      ], cookies, `${prefix}: FPI on, w/ empty firstPartyDomain, non-FP cookies`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain: null});
+      assertExpectedCookies([
+        {name: "foo1", value: "bar1", firstPartyDomain: ""},
+        {name: "foo2", value: "bar2", firstPartyDomain},
+        {name: "foo4", value: "bar4", firstPartyDomain},
+      ], cookies, `${prefix}: FPI on, w/ null firstPartyDomain, all cookies`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain: undefined});
+      assertExpectedCookies([
+        {name: "foo1", value: "bar1", firstPartyDomain: ""},
+        {name: "foo2", value: "bar2", firstPartyDomain},
+        {name: "foo4", value: "bar4", firstPartyDomain},
+      ], cookies, `${prefix}: FPI on, w/ undefined firstPartyDomain, all cookies`);
+      cookies = await browser.cookies.getAll({...extra, firstPartyDomain});
+      assertExpectedCookies([
+        {name: "foo2", value: "bar2", firstPartyDomain},
+        {name: "foo4", value: "bar4", firstPartyDomain},
+      ], cookies, `${prefix}: FPI on, w/ firstPartyDomain, FP cookies`);
+    }
 
     // remove
     await browser.test.assertRejects(
       browser.cookies.remove({url, name: "foo3"}),
       expectedError,
       "remove: FPI on, w/o firstPartyDomain, rejection");
     cookie = await browser.cookies.remove({url, name: "foo4", firstPartyDomain});
     assertExpectedCookies([