Bug 1689290 - Fix handling of IP addresses in the "single-site" link handler. r=mkmelin
authorGeoff Lankow <geoff@darktrojan.net>
Thu, 28 Jan 2021 12:40:16 +0200
changeset 31538 bf3be1f71ed552b4c6beda5f240373c55dafca63
parent 31537 f6354f32e562df8f8cc916131e1d6614199081f1
child 31539 ea8fc7b7757e00b147cb2bdee3dc2ab62184f916
push id18400
push usermkmelin@iki.fi
push dateThu, 28 Jan 2021 10:50:42 +0000
treeherdercomm-central@90c7a8c7c110 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin
bugs1689290
Bug 1689290 - Fix handling of IP addresses in the "single-site" link handler. r=mkmelin Differential Revision: https://phabricator.services.mozilla.com/D103280
mail/base/actors/LinkHandlerChild.jsm
mail/base/test/browser/browser_linkHandler.js
mail/base/test/browser/files/links.html
--- a/mail/base/actors/LinkHandlerChild.jsm
+++ b/mail/base/actors/LinkHandlerChild.jsm
@@ -90,25 +90,32 @@ class LinkHandlerChild extends JSWindowA
 
     let eventHRef = hRefForClickEvent(event);
     if (!eventHRef) {
       return;
     }
 
     let eventURI = Services.io.newURI(eventHRef);
 
+    if (pageURI.host == eventURI.host) {
+      // Avoid using the eTLD service, and this also works for IP addresses.
+      return;
+    }
+
     try {
       if (
         Services.eTLD.getBaseDomain(eventURI) ==
         Services.eTLD.getBaseDomain(pageURI)
       ) {
         return;
       }
     } catch (ex) {
-      Cu.reportError(ex);
+      if (ex.result != Cr.NS_ERROR_HOST_IS_IP_ADDRESS) {
+        Cu.reportError(ex);
+      }
     }
 
     if (
       !protocolSvc.isExposedProtocol(eventURI.scheme) ||
       eventURI.schemeIs("http") ||
       eventURI.schemeIs("https")
     ) {
       event.preventDefault();
--- a/mail/base/test/browser/browser_linkHandler.js
+++ b/mail/base/test/browser/browser_linkHandler.js
@@ -2,29 +2,29 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, you can obtain one at http://mozilla.org/MPL/2.0/. */
 
 let { MockRegistrar } = ChromeUtils.import(
   "resource://testing-common/MockRegistrar.jsm"
 );
 
 const TEST_DOMAIN = "http://example.org";
+const TEST_IP = "http://127.0.0.1:8888";
 const TEST_PATH = "/browser/comm/mail/base/test/browser/files/links.html";
-const TEST_URL = `${TEST_DOMAIN}${TEST_PATH}`;
 
 let links = new Map([
-  ["#this-hash", `${TEST_DOMAIN}${TEST_PATH}#hash`],
-  ["#this-nohash", `${TEST_DOMAIN}${TEST_PATH}`],
+  ["#this-hash", `${TEST_PATH}#hash`],
+  ["#this-nohash", `${TEST_PATH}`],
   [
     "#local-here",
-    `${TEST_DOMAIN}/browser/comm/mail/base/test/browser/files/sampleContent.html`,
+    "/browser/comm/mail/base/test/browser/files/sampleContent.html",
   ],
   [
     "#local-elsewhere",
-    `${TEST_DOMAIN}/browser/comm/mail/components/extensions/test/browser/data/content.html`,
+    "/browser/comm/mail/components/extensions/test/browser/data/content.html",
   ],
   ["#other-https", `https://example.org${TEST_PATH}`],
   ["#other-port", `http://example.org:8000${TEST_PATH}`],
   ["#other-subdomain", `http://test1.example.org${TEST_PATH}`],
   ["#other-subsubdomain", `http://sub1.test1.example.org${TEST_PATH}`],
   ["#other-domain", `http://mochi.test:8888${TEST_PATH}`],
 ]);
 
@@ -51,27 +51,33 @@ registerCleanupFunction(() => {
 
   while (tabmail.tabInfo.length > 1) {
     tabmail.closeTab(tabmail.tabInfo[1]);
   }
 
   MockRegistrar.unregister(mockExternalProtocolServiceCID);
 });
 
-async function clickOnLink(browser, selector, url, shouldLoadInternally) {
+async function clickOnLink(
+  browser,
+  selector,
+  url,
+  pageURL,
+  shouldLoadInternally
+) {
   if (
     browser.webProgress?.isLoadingDocument ||
     browser.currentURI?.spec == "about:blank"
   ) {
     await BrowserTestUtils.browserLoaded(browser);
   }
   mockExternalProtocolService._loadedURLs.length = 0;
   Assert.equal(
     browser.currentURI?.spec,
-    TEST_URL,
+    pageURL,
     "original URL should be loaded"
   );
 
   info(`clicking on ${selector}`);
   await BrowserTestUtils.synthesizeMouseAtCenter(selector, {}, browser);
   // Responding to the click probably won't happen immediately. Let's hang
   // around and see what happens.
   // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
@@ -88,74 +94,123 @@ async function clickOnLink(browser, sele
       url,
       `${url} should load internally`
     );
     Assert.ok(
       !mockExternalProtocolService.urlLoaded(url),
       `${url} should not load externally`
     );
   } else {
-    if (url != TEST_URL) {
+    if (url != pageURL) {
       Assert.equal(
         browser.currentURI?.spec,
-        TEST_URL,
+        pageURL,
         `${url} should not load internally`
       );
     }
     Assert.ok(
       mockExternalProtocolService.urlLoaded(url),
       `${url} should load externally`
     );
   }
 
-  if (browser.currentURI?.spec != TEST_URL) {
+  if (browser.currentURI?.spec != pageURL) {
     let promise = new Promise(resolve => {
       let event = selector == "#this-hash" ? "hashchange" : "pageshow";
       let unregister = BrowserTestUtils.addContentEventListener(
         browser,
         event,
         () => {
           unregister();
           resolve();
         },
         { capture: true }
       );
     });
 
     browser.browsingContext.goBack();
     await promise;
-    Assert.equal(browser.currentURI?.spec, TEST_URL, "should have gone back");
+    Assert.equal(browser.currentURI?.spec, pageURL, "should have gone back");
   }
 }
 
-async function subtest(group, shouldLoadCB) {
+async function subtest(pagePrePath, group, shouldLoadCB) {
   let tabmail = document.getElementById("tabmail");
-  let tab = window.openContentTab(TEST_URL, undefined, group);
+  let tab = window.openContentTab(
+    `${pagePrePath}${TEST_PATH}`,
+    undefined,
+    group
+  );
 
   let expectedGroup = group;
   if (group === null) {
     expectedGroup = "browsers";
   } else if (group === undefined) {
     expectedGroup = "single-site";
   }
   Assert.equal(tab.browser.getAttribute("messagemanagergroup"), expectedGroup);
 
   for (let [selector, url] of links) {
-    await clickOnLink(tab.browser, selector, url, shouldLoadCB(selector));
+    if (url.startsWith("/")) {
+      url = `${pagePrePath}${url}`;
+    }
+    await clickOnLink(
+      tab.browser,
+      selector,
+      url,
+      `${pagePrePath}${TEST_PATH}`,
+      shouldLoadCB(selector)
+    );
   }
   tabmail.closeTab(tab);
 }
 
 add_task(function testNoGroup() {
-  return subtest(undefined, selector => selector != "#other-domain");
+  return subtest(
+    TEST_DOMAIN,
+    undefined,
+    selector => selector != "#other-domain"
+  );
 });
 
 add_task(function testBrowsersGroup() {
-  return subtest(null, selector => true);
+  return subtest(TEST_DOMAIN, null, selector => true);
+});
+
+add_task(function testSingleSiteGroup() {
+  return subtest(
+    TEST_DOMAIN,
+    "single-site",
+    selector => selector != "#other-domain"
+  );
+});
+
+add_task(function testSinglePageGroup() {
+  return subtest(TEST_DOMAIN, "single-page", selector =>
+    selector.startsWith("#this")
+  );
 });
 
-add_task(function testSSBGroup() {
-  return subtest("single-site", selector => selector != "#other-domain");
+add_task(function testNoGroupWithIP() {
+  return subtest(
+    TEST_IP,
+    undefined,
+    selector => selector.startsWith("#this") || selector.startsWith("#local")
+  );
+});
+
+add_task(function testBrowsersGroupWithIP() {
+  return subtest(TEST_IP, null, selector => true);
 });
 
-add_task(function testStrictGroup() {
-  return subtest("single-page", selector => selector.startsWith("#this"));
+add_task(function testSingleSiteGroupWithIP() {
+  return subtest(
+    TEST_IP,
+    "single-site",
+    selector => selector.startsWith("#this") || selector.startsWith("#local")
+  );
 });
+
+add_task(function testSinglePageGroupWithIP() {
+  return subtest(TEST_IP, "single-page", selector =>
+    selector.startsWith("#this")
+  );
+});
--- a/mail/base/test/browser/files/links.html
+++ b/mail/base/test/browser/files/links.html
@@ -4,19 +4,19 @@
   <meta charset="utf-8"/>
   <title>Links to other places</title>
 </head>
 <body>
   <h1>Links to things</h1>
   <p>This page is a test of what happens when you click on links. It should be loaded from http://example.org:80.</p>
 
   <h2>This page:</h2>
+  <ul>
     <li><a id="this-hash" href="#hash">Anchor on this page</a></li>
     <li><a id="this-nohash" href="links.html">This page</a></li>
-  <ul>
   </ul>
 
   <h2>Pages on this domain:</h2>
   <ul>
     <li><a id="local-here" href="sampleContent.html">A page in the same directory</a></li>
     <li><a id="local-elsewhere" href="/browser/comm/mail/components/extensions/test/browser/data/content.html">A page elsewhere</a></li>
   </ul>