Bug 1417828 - Ignore ports in cookies API r?zombie draft
authorRob Wu <rob@robwu.nl>
Sat, 02 Dec 2017 02:09:51 +0100
changeset 712089 7dc914ed8d202402edaf0a44ccabf9c78cdf7ba2
parent 706205 781485c695e1f07b8782427d556f6570e4a8072f
child 743977 1af65760841515aeb96f52b6046481fbb1ffc925
push id93253
push userbmo:rob@robwu.nl
push dateFri, 15 Dec 2017 18:09:08 +0000
reviewerszombie
bugs1417828, 1398630
milestone59.0a1
Bug 1417828 - Ignore ports in cookies API r?zombie Fixes regression from bug 1398630, where nsIURI was replaced with DOM URL. In nsIURI, "host" does not include a port number, but in a DOM URL, "host" does include a port number and "hostname" has to be used to obtain the original result. MozReview-Commit-ID: KxCF6Srn7X4
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
@@ -175,17 +175,17 @@ const query = function* (detailsIn, prop
   let url;
   let originAttributes = {
     userContextId,
     privateBrowsingId: isPrivate ? 1 : 0,
   };
   if ("url" in details) {
     try {
       url = new URL(details.url);
-      enumerator = Services.cookies.getCookiesFromHost(url.host, originAttributes);
+      enumerator = Services.cookies.getCookiesFromHost(url.hostname, originAttributes);
     } catch (ex) {
       // This often happens for about: URLs
       return;
     }
   } else if ("domain" in details) {
     enumerator = Services.cookies.getCookiesFromHost(details.domain, originAttributes);
   } else {
     enumerator = Services.cookies.getCookiesWithOriginAttributes(JSON.stringify(originAttributes));
@@ -211,17 +211,17 @@ const query = function* (detailsIn, prop
 
       // URL path is a substring of the cookie path, so it matches if, and
       // only if, the next character is a path delimiter.
       return path[cookiePath.length] === "/";
     }
 
     // "Restricts the retrieved cookies to those that would match the given URL."
     if (url) {
-      if (!domainMatches(url.host)) {
+      if (!domainMatches(url.hostname)) {
         return false;
       }
 
       if (cookie.isSecure && url.protocol != "https:") {
         return false;
       }
 
       if (!pathMatches(url.pathname)) {
--- a/toolkit/components/extensions/test/mochitest/test_ext_cookies.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_cookies.html
@@ -53,16 +53,22 @@ add_task(async function test_cookies() {
       let {id: windowId} = await browser.windows.create({
         incognito: true,
         url: PRIVATE_TEST_URL,
       });
       let tabId = await tabReadyPromise;
       return {windowId, tabId};
     }
 
+    function changePort(href, port) {
+      let url = new URL(href);
+      url.port = port;
+      return url.href;
+    }
+
     const TEST_URL = "http://example.org/";
     const TEST_SECURE_URL = "https://example.org/";
     const THE_FUTURE = Date.now() + 5 * 60;
     const TEST_PATH = "set_path";
     const TEST_URL_WITH_PATH = TEST_URL + TEST_PATH;
     const TEST_COOKIE_PATH = `/${TEST_PATH}`;
     const STORE_ID = "firefox-default";
     const PRIVATE_STORE_ID = "firefox-private";
@@ -115,16 +121,39 @@ add_task(async function test_cookies() {
     browser.test.assertEq(cookies.length, 0, "no cookies found for invalid storeId");
 
     let details = await browser.cookies.remove({url: TEST_URL, name: "name1"});
     assertExpected({url: TEST_URL, name: "name1", storeId: STORE_ID}, details);
 
     cookie = await browser.cookies.get({url: TEST_URL, name: "name1"});
     browser.test.assertEq(null, cookie, "removed cookie not found");
 
+    // Ports in cookie URLs should be ignored. Every API call uses a different port number for better coverage.
+    cookie = await browser.cookies.set({url: changePort(TEST_URL, 1234), name: "name1", value: "value1", expirationDate: THE_FUTURE});
+    assertExpected(expected, cookie);
+
+    cookie = await browser.cookies.get({url: changePort(TEST_URL, 65535), name: "name1"});
+    assertExpected(expected, cookie);
+
+    cookies = await browser.cookies.getAll({url: TEST_URL});
+    browser.test.assertEq(cookies.length, 1, "Found cookie using getAll without port");
+    assertExpected(expected, cookies[0]);
+
+    cookies = await browser.cookies.getAll({url: changePort(TEST_URL, 1)});
+    browser.test.assertEq(cookies.length, 1, "Found cookie using getAll with port");
+    assertExpected(expected, cookies[0]);
+
+    // .remove should return the URL of the API call, so the port is included in the return value.
+    const TEST_URL_TO_REMOVE = changePort(TEST_URL, 1023);
+    details = await browser.cookies.remove({url: TEST_URL_TO_REMOVE, name: "name1"});
+    assertExpected({url: TEST_URL_TO_REMOVE, name: "name1", storeId: STORE_ID}, details);
+
+    cookie = await browser.cookies.get({url: TEST_URL, name: "name1"});
+    browser.test.assertEq(null, cookie, "removed cookie not found");
+
     let stores = await browser.cookies.getAllCookieStores();
     browser.test.assertEq(1, stores.length, "expected number of stores returned");
     browser.test.assertEq(STORE_ID, stores[0].id, "expected store id returned");
     browser.test.assertEq(1, stores[0].tabIds.length, "one tabId returned for store");
     browser.test.assertEq("number", typeof stores[0].tabIds[0], "tabId is a number");
 
     // TODO bug 1372178: Opening private windows/tabs is not supported on Android
     if (browser.windows) {