Bug 1417828 - Ignore ports in cookies API r=zombie
authorRob Wu <rob@robwu.nl>
Sat, 02 Dec 2017 02:09:51 +0100
changeset 396558 eb72a70b524f7434a307df720019efa7442a6554
parent 396557 fd98493f58fa4c5c9d55558c0d623fbc2ac9f3e6
child 396559 b83870070900d6d4c487b830e9b1342de474142d
push id57044
push userrob@robwu.nl
push dateFri, 15 Dec 2017 18:12:33 +0000
treeherderautoland@eb72a70b524f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszombie
bugs1417828, 1398630
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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) {