Bug 1634650 - Add whitelisting of domain suffixes for URIFixup. r=harry

Differential Revision: https://phabricator.services.mozilla.com/D74015

browser/app/profile/firefox.js

@@ -191,7 +191,19 @@ pref("browser.uitour.url", "https://www.mozilla.org/%LOCALE%/firefox/%VERSION%/t
 pref("browser.uitour.surveyDuration", 7200);
 
 pref("keyword.enabled", true);
+// Fixup whitelists, the urlbar won't try to search for these words, but will
+// instead consider them valid TLDs. Don't check these directly, use
+// Services.uriFixup.isDomainWhitelisted() instead.
 pref("browser.fixup.domainwhitelist.localhost", true);
+// https://tools.ietf.org/html/rfc2606
+pref("browser.fixup.domainsuffixwhitelist.test", true);
+pref("browser.fixup.domainsuffixwhitelist.example", true);
+pref("browser.fixup.domainsuffixwhitelist.invalid", true);
+pref("browser.fixup.domainsuffixwhitelist.localhost", true);
+// https://tools.ietf.org/html/draft-wkumari-dnsop-internal-00
+pref("browser.fixup.domainsuffixwhitelist.internal", true);
+// https://tools.ietf.org/html/rfc6762
+pref("browser.fixup.domainsuffixwhitelist.local", true);
 
 #ifdef UNIX_BUT_NOT_MAC
   pref("general.autoScroll", false);

browser/base/content/browser.js

@@ -1292,7 +1292,7 @@ var gKeywordURIFixup = {
     // Normalize out a single trailing dot - NB: not using endsWith/lastIndexOf
     // because we need to be sure this last dot is the *only* dot, too.
     // More generally, this is used for the pref and should stay in sync with
-    // the code in nsDefaultURIFixup::KeywordURIFixup .
+    // the code in URIFixup::KeywordURIFixup .
     if (asciiHost.indexOf(".") == asciiHost.length - 1) {
       asciiHost = asciiHost.slice(0, -1);
     }

browser/components/urlbar/tests/browser/browser_copying.js

@@ -22,19 +22,12 @@ function test() {
     Services.prefs.clearUserPref(trimPref);
     Services.prefs.clearUserPref(phishyUserPassPref);
     Services.prefs.clearUserPref(decodeURLpref);
-    Services.prefs.clearUserPref(
-      "browser.fixup.domainwhitelist.sub2.xn--lt-uia.mochi.test"
-    );
 
     gURLBar.setURI();
   });
 
   Services.prefs.setBoolPref(trimPref, true);
   Services.prefs.setIntPref(phishyUserPassPref, 32); // avoid prompting about phishing
-  Services.prefs.setBoolPref(
-    "browser.fixup.domainwhitelist.sub2.xn--lt-uia.mochi.test",
-    true
-  );
 
   waitForExplicitFinish();
 

docshell/base/URIFixup.jsm

@@ -173,6 +173,64 @@ XPCOMUtils.defineLazyGetter(this, "domainsWhitelist", () => {
   return domains;
 });
 
+// Cache of whitelisted suffixes.
+// This works differently from the domains whitelist, because when we examine a
+// domain we can't tell how many dot-separated parts constitute the suffix.
+// We create a Map keyed by the last dotted part, containing a Set of
+// all the suffixes ending with that part:
+//   "two" => ["two"]
+//   "three" => ["some.three", "three"]
+// When searching we can restrict the linear scan based on the last part.
+// The ideal structure for this would be a Directed Acyclic Word Graph, but
+// since we expect this list to be small it's not worth the complication.
+XPCOMUtils.defineLazyGetter(this, "suffixesWhitelist", () => {
+  const branch = "browser.fixup.domainsuffixwhitelist.";
+  let suffixes = new Map();
+  let prefs = Services.prefs
+    .getChildList(branch)
+    .filter(p => Services.prefs.getBoolPref(p, false));
+  for (let pref of prefs) {
+    let suffix = pref.substring(branch.length);
+    let lastPart = suffix.substr(suffix.lastIndexOf(".") + 1);
+    if (lastPart) {
+      let entries = suffixes.get(lastPart);
+      if (!entries) {
+        entries = new Set();
+        suffixes.set(lastPart, entries);
+      }
+      entries.add(suffix);
+    }
+  }
+  // Hold onto the observer to avoid it being GC-ed.
+  suffixes._observer = {
+    observe(subject, topic, data) {
+      let suffix = data.substring(branch.length);
+      let lastPart = suffix.substr(suffix.lastIndexOf(".") + 1);
+      let entries = suffixes.get(lastPart);
+      if (Services.prefs.getBoolPref(data, false)) {
+        // Add the suffix.
+        if (!entries) {
+          entries = new Set();
+          suffixes.set(lastPart, entries);
+        }
+        entries.add(suffix);
+      } else if (entries) {
+        // Remove the suffix.
+        entries.delete(suffix);
+        if (!entries.size) {
+          suffixes.delete(lastPart);
+        }
+      }
+    },
+    QueryInterface: ChromeUtils.generateQI([
+      Ci.nsIObserver,
+      Ci.nsISupportsWeakReference,
+    ]),
+  };
+  Services.prefs.addObserver(branch, suffixes._observer, true);
+  return suffixes;
+});
+
 function URIFixup() {}
 
 URIFixup.prototype = {
@@ -548,10 +606,28 @@ function isDomainWhitelisted(asciiHost) {
   // domain (which will prevent a keyword query)
   // Note that any processing of the host here should stay in sync with
   // code in the front-end(s) that set the pref.
-  if (asciiHost.endsWith(".")) {
+  let lastDotIndex = asciiHost.lastIndexOf(".");
+  if (lastDotIndex == asciiHost.length - 1) {
     asciiHost = asciiHost.substring(0, asciiHost.length - 1);
+    lastDotIndex = asciiHost.lastIndexOf(".");
   }
-  return domainsWhitelist.has(asciiHost.toLowerCase());
+  if (domainsWhitelist.has(asciiHost.toLowerCase())) {
+    return true;
+  }
+  // If there's no dot or only a leading dot we are done, otherwise we'll check
+  // against the suffixes whitelist.
+  if (lastDotIndex <= 0) {
+    return false;
+  }
+  // Don't use getPublicSuffix here, since the suffix is not in a known list,
+  // thus it couldn't tell if the suffix is made up of one or multiple
+  // dot-separated parts.
+  let lastPart = asciiHost.substr(lastDotIndex + 1);
+  let suffixes = suffixesWhitelist.get(lastPart);
+  if (suffixes) {
+    return Array.from(suffixes).some(s => asciiHost.endsWith(s));
+  }
+  return false;
 }
 
 /**

docshell/test/unit/test_URIFixup_info.js

@@ -388,6 +388,22 @@ var testcases = [
     protocolChange: true,
     inWhitelist: true,
   },
+  // local.domain is a whitelisted suffix...
+  {
+    input: "some.local.domain",
+    fixedURI: "http://some.local.domain/",
+    protocolChange: true,
+    inWhitelist: true,
+  },
+  // ...but .domain is not.
+  {
+    input: "some.domain",
+    fixedURI: "http://some.domain/",
+    alternateURI: "http://www.some.domain/",
+    keywordLookup: true,
+    protocolChange: true,
+    affectedByDNSForSingleWordHosts: true,
+  },
   {
     input: "café.com",
     fixedURI: "http://xn--caf-dma.com/",
@@ -623,7 +639,8 @@ add_task(async function setup() {
     "browser.fixup.typo.scheme",
     "keyword.enabled",
     "browser.fixup.domainwhitelist.whitelisted",
-    "browser.fixup.domainwhitelist.mochi.test",
+    "browser.fixup.domainsuffixwhitelist.test",
+    "browser.fixup.domainsuffixwhitelist.local.domain",
     "browser.search.separatePrivateDefault",
     "browser.search.separatePrivateDefault.ui.enabled",
   ];

docshell/test/unit/xpcshell.ini

@@ -3,12 +3,12 @@ head = head_docshell.js
 
 [test_bug442584.js]
 [test_browsing_context_structured_clone.js]
-[test_nsDefaultURIFixup.js]
+[test_URIFixup.js]
 # Disabled for 1563343 -- URI fixup should be done at the app level in GV.
 skip-if = os == 'android'
-[test_nsDefaultURIFixup_search.js]
+[test_URIFixup_search.js]
 skip-if = os == 'android'
-[test_nsDefaultURIFixup_info.js]
+[test_URIFixup_info.js]
 skip-if = os == 'android'
 [test_pb_notification.js]
 # Bug 751575: unrelated JS changes cause timeouts on random platforms

testing/profiles/unittest-required/user.js

@@ -66,7 +66,7 @@ user_pref("browser.urlbar.speculativeConnect.enabled", false);
 user_pref("browser.urlbar.suggest.searches", false);
 user_pref("browser.urlbar.usepreloadedtopurls.enabled", false);
 // URIFixup whitelist
-user_pref("browser.fixup.domainwhitelist.mochi.test", true);
+user_pref("browser.fixup.domainsuffixwhitelist.test", true);
 user_pref("browser.warnOnQuit", false);
 // Enable webapps testing mode, which bypasses native installation.
 user_pref("browser.webapps.testing", true);