Bug 1642943 - Introduce a pref to control post-search DNS resolution of single word hosts. r=Gijs,mkaply, a=jcristau
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 11 Jun 2020 12:42:29 +0000
changeset 597128 95386b54ae0e9487bb1345fdce06a8843f03ebb6
parent 597127 c81d1a64cab4dfeac8cf8ecbcf9e36c587139b34
child 597129 674d751cba2dc520ff2c38e97b909f0ebee550c9
push id13274
push userjcristau@mozilla.com
push dateFri, 12 Jun 2020 17:45:29 +0000
treeherdermozilla-beta@941e6cb8893a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, mkaply, jcristau
bugs1642943
milestone78.0
Bug 1642943 - Introduce a pref to control post-search DNS resolution of single word hosts. r=Gijs,mkaply, a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D78403
browser/app/profile/firefox.js
browser/base/content/browser.js
browser/components/enterprisepolicies/schemas/policies-schema.json
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/UrlbarPrefs.jsm
browser/components/urlbar/tests/browser/browser_searchSingleWordNotification.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -186,16 +186,17 @@ pref("browser.uitour.enabled", true);
 pref("browser.uitour.loglevel", "Error");
 pref("browser.uitour.requireSecure", true);
 pref("browser.uitour.themeOrigin", "https://addons.mozilla.org/%LOCALE%/firefox/themes/");
 pref("browser.uitour.url", "https://www.mozilla.org/%LOCALE%/firefox/%VERSION%/tour/");
 // How long to show a Hearbeat survey (two hours, in seconds)
 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);
@@ -262,16 +263,18 @@ pref("browser.overlink-delay", 80);
 // Whether using `ctrl` when hitting return/enter in the URL bar
 // (or clicking 'go') should prefix 'www.' and suffix
 // browser.fixup.alternate.suffix to the URL bar value prior to
 // navigating.
 pref("browser.urlbar.ctrlCanonizesURLs", true);
 
 // Control autoFill behavior
 pref("browser.urlbar.autoFill", true);
+
+// Whether to warm up network connections for autofill or search results.
 pref("browser.urlbar.speculativeConnect.enabled", true);
 
 // Whether bookmarklets should be filtered out of Address Bar matches.
 // This is enabled for security reasons, when true it is still possible to
 // search for bookmarklets typing "javascript: " followed by the actual query.
 pref("browser.urlbar.filter.javascript", true);
 
 // the maximum number of results to show in autocomplete when doing richResults
@@ -332,16 +335,22 @@ pref("browser.urlbar.update1.interventio
 pref("browser.urlbar.update1.searchTips", true);
 
 // Whether we expand the font size when when the urlbar is
 // focused in design update 2.
 pref("browser.urlbar.update2.expandTextOnFocus", false);
 
 pref("browser.urlbar.eventTelemetry.enabled", false);
 
+// Controls when to DNS resolve single word search strings, after they were
+// searched for. If the string is resolved as a valid host, show a
+// "Did you mean to go to 'host'" prompt.
+// 0 - never resolve; 1 - use heuristics (default); 2 - always resolve
+pref("browser.urlbar.dnsResolveSingleWordsAfterSearch", 1);
+
 pref("browser.altClickSave", false);
 
 // Enable logging downloads operations to the Console.
 pref("browser.download.loglevel", "Error");
 
 // Number of milliseconds to wait for the http headers (and thus
 // the Content-Disposition filename) before giving up and falling back to
 // picking a filename without that info in hand so that the user sees some
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1264,17 +1264,23 @@ XPCOMUtils.defineLazyPreferenceGetter(
   "privacy.popups.maxReported"
 );
 
 var gKeywordURIFixup = {
   check(browser, { fixedURI, keywordProviderName, preferredURI }) {
     // We get called irrespective of whether we did a keyword search, or
     // whether the original input would be vaguely interpretable as a URL,
     // so figure that out first.
-    if (!keywordProviderName || !fixedURI || !fixedURI.host) {
+    if (
+      !keywordProviderName ||
+      !fixedURI ||
+      !fixedURI.host ||
+      UrlbarPrefs.get("browser.fixup.dns_first_for_single_words") ||
+      UrlbarPrefs.get("dnsResolveSingleWordsAfterSearch") == 0
+    ) {
       return;
     }
 
     let contentPrincipal = browser.contentPrincipal;
 
     // At this point we're still only just about to load this URI.
     // When the async DNS lookup comes back, we may be in any of these states:
     // 1) still on the previous URI, waiting for the preferredURI (keyword
--- a/browser/components/enterprisepolicies/schemas/policies-schema.json
+++ b/browser/components/enterprisepolicies/schemas/policies-schema.json
@@ -879,16 +879,20 @@
           "type": "boolean"
         },
         "browser.cache.disk.enable": {
           "type": "boolean"
         },
         "browser.fixup.dns_first_for_single_words": {
           "type": "boolean"
         },
+        "browser.urlbar.dnsResolveSingleWordsAfterSearch": {
+          "type": "number",
+          "enum": [0, 1, 2]
+        },
         "browser.newtabpage.activity-stream.default.sites": {
           "type": "string"
         },
         "browser.safebrowsing.phishing.enabled": {
           "type": "boolean"
         },
         "browser.safebrowsing.malware.enabled": {
           "type": "boolean"
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -663,26 +663,31 @@ class UrlbarInput {
           // start a new search so that the offer appears in the view by itself
           // to make it even clearer to the user what's going on.
           this.startQuery();
           return;
         }
 
         if (
           result.heuristic &&
+          // If we asked the DNS earlier, avoid the post-facto check.
+          !UrlbarPrefs.get("browser.fixup.dns_first_for_single_words") &&
+          // TODO (bug 1642623): for now there is no smart heuristic to skip the
+          // DNS lookup, so any value above 0 will run it.
+          UrlbarPrefs.get("dnsResolveSingleWordsAfterSearch") > 0 &&
           this.window.gKeywordURIFixup &&
           UrlbarUtils.looksLikeSingleWordHost(originalUntrimmedValue)
         ) {
-          // When fixing a single word to a search, the docShell also checks the
-          // DNS and asks the user whether they would rather visit that as a
-          // host. On a positive answer, it adds to the domain whitelist that
-          // we use to make decisions. Because we are directly asking for a
-          // search here, bypassing the docShell, we need invoke the same check
-          // ourselves. See also URIFixupChild.jsm and keyword-uri-fixup.
-          // Don't interrupt the load action in case of errors.
+          // When fixing a single word to a search, the docShell would also
+          // query the DNS and if resolved ask the user whether they would
+          // rather visit that as a host. On a positive answer, it adds the host
+          // the the list that we use to make decisions.
+          // Because we are directly asking for a search here, bypassing the
+          // docShell, we need to do the same ourselves.
+          // See also URIFixupChild.jsm and keyword-uri-fixup.
           let fixupInfo = this._getURIFixupInfo(originalUntrimmedValue.trim());
           if (fixupInfo) {
             this.window.gKeywordURIFixup.check(
               this.window.gBrowser.selectedBrowser,
               fixupInfo
             );
           }
         }
--- a/browser/components/urlbar/UrlbarPrefs.jsm
+++ b/browser/components/urlbar/UrlbarPrefs.jsm
@@ -56,16 +56,22 @@ const PREF_URLBAR_DEFAULTS = new Map([
   // "heuristic" result).  We fetch it as fast as possible.
   ["delay", 50],
 
   // Some performance tests disable this because extending the urlbar needs
   // layout information that we can't get before the first paint. (Or we could
   // but this would mean flushing layout.)
   ["disableExtendForTests", false],
 
+  // Controls when to DNS resolve single word search strings, after they were
+  // searched for. If the string is resolved as a valid host, show a
+  // "Did you mean to go to 'host'" prompt.
+  // 0 - never resolve; 1 - use heuristics (default); 2 - always resolve
+  ["dnsResolveSingleWordsAfterSearch", 1],
+
   // Whether telemetry events should be recorded.
   ["eventTelemetry.enabled", false],
 
   // When true, `javascript:` URLs are not included in search results.
   ["filter.javascript", true],
 
   // Applies URL highlighting and other styling to the text in the urlbar input.
   ["formatting.enabled", false],
--- a/browser/components/urlbar/tests/browser/browser_searchSingleWordNotification.js
+++ b/browser/components/urlbar/tests/browser/browser_searchSingleWordNotification.js
@@ -1,17 +1,24 @@
 /* eslint-disable mozilla/no-arbitrary-setTimeout */
 "use strict";
 
-var notificationObserver;
-registerCleanupFunction(function() {
-  Services.prefs.clearUserPref("browser.fixup.domainwhitelist.localhost");
-  if (notificationObserver) {
-    notificationObserver.disconnect();
-  }
+let gDNSResolved = false;
+let gRealDNSService = gDNSService;
+add_task(async function setup() {
+  gDNSService = {
+    asyncResolve() {
+      gDNSResolved = true;
+      return gRealDNSService.asyncResolve(...arguments);
+    },
+  };
+  registerCleanupFunction(function() {
+    gDNSService = gRealDNSService;
+    Services.prefs.clearUserPref("browser.fixup.domainwhitelist.localhost");
+  });
 });
 
 function promiseNotification(aBrowser, value, expected, input) {
   return new Promise(resolve => {
     let notificationBox = aBrowser.getNotificationBox(aBrowser.selectedBrowser);
     if (expected) {
       info("Waiting for " + value + " notification");
       resolve(
@@ -32,18 +39,20 @@ function promiseNotification(aBrowser, v
     }
   });
 }
 
 async function runURLBarSearchTest({
   valueToOpen,
   expectSearch,
   expectNotification,
+  expectDNSResolve,
   aWindow = window,
 }) {
+  gDNSResolved = false;
   // Test both directly setting a value and pressing enter, or setting the
   // value through input events, like the user would do.
   const setValueFns = [
     value => {
       aWindow.gURLBar.value = value;
     },
     value => {
       return UrlbarTestUtils.promiseAutocompleteResultPopup({
@@ -97,131 +106,154 @@ async function runURLBarSearchTest({
           aWindow.gBrowser.selectedBrowser
         );
         notification.querySelector("button").click();
         await docLoadPromise;
       } else {
         notificationBox.currentNotification.close();
       }
     }
+    Assert.equal(
+      gDNSResolved,
+      expectDNSResolve,
+      `Should${expectDNSResolve ? "" : " not"} DNS resolve "${valueToOpen}"`
+    );
   }
 }
 
 add_task(async function test_navigate_full_domain() {
   let tab = (gBrowser.selectedTab = BrowserTestUtils.addTab(
     gBrowser,
     "about:blank"
   ));
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await runURLBarSearchTest({
     valueToOpen: "www.singlewordtest.org",
     expectSearch: false,
     expectNotification: false,
+    expectDNSResolve: false,
   });
   gBrowser.removeTab(tab);
 });
 
 add_task(async function test_navigate_decimal_ip() {
   let tab = (gBrowser.selectedTab = BrowserTestUtils.addTab(
     gBrowser,
     "about:blank"
   ));
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await runURLBarSearchTest({
     valueToOpen: "1234",
     expectSearch: true,
     expectNotification: false,
+    expectDNSResolve: false, // Possible IP in numeric format.
   });
   gBrowser.removeTab(tab);
 });
 
 add_task(async function test_navigate_decimal_ip_with_path() {
   let tab = (gBrowser.selectedTab = BrowserTestUtils.addTab(
     gBrowser,
     "about:blank"
   ));
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await runURLBarSearchTest({
     valueToOpen: "1234/12",
     expectSearch: true,
     expectNotification: false,
+    expectDNSResolve: false,
   });
   gBrowser.removeTab(tab);
 });
 
 add_task(async function test_navigate_large_number() {
   let tab = (gBrowser.selectedTab = BrowserTestUtils.addTab(
     gBrowser,
     "about:blank"
   ));
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await runURLBarSearchTest({
     valueToOpen: "123456789012345",
     expectSearch: true,
     expectNotification: false,
+    expectDNSResolve: false, // Possible IP in numeric format.
   });
   gBrowser.removeTab(tab);
 });
 
 add_task(async function test_navigate_small_hex_number() {
   let tab = (gBrowser.selectedTab = BrowserTestUtils.addTab(
     gBrowser,
     "about:blank"
   ));
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await runURLBarSearchTest({
     valueToOpen: "0x1f00ffff",
     expectSearch: true,
     expectNotification: false,
+    expectDNSResolve: false, // Possible IP in numeric format.
   });
   gBrowser.removeTab(tab);
 });
 
 add_task(async function test_navigate_large_hex_number() {
   let tab = (gBrowser.selectedTab = BrowserTestUtils.addTab(
     gBrowser,
     "about:blank"
   ));
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await runURLBarSearchTest({
     valueToOpen: "0x7f0000017f000001",
     expectSearch: true,
     expectNotification: false,
+    expectDNSResolve: false, // Possible IP in numeric format.
   });
   gBrowser.removeTab(tab);
 });
 
-function get_test_function_for_localhost_with_hostname(hostName, isPrivate) {
+function get_test_function_for_localhost_with_hostname(
+  hostName,
+  isPrivate = false
+) {
   return async function test_navigate_single_host() {
+    info(`Test ${hostName}${isPrivate ? " in Private Browsing mode" : ""}`);
     const pref = "browser.fixup.domainwhitelist.localhost";
     let win;
     if (isPrivate) {
       let promiseWin = BrowserTestUtils.waitForNewWindow();
       win = OpenBrowserWindow({ private: true });
       await promiseWin;
-      await new Promise(resolve => {
-        waitForFocus(resolve, win);
-      });
+      await SimpleTest.promiseFocus(win);
+      // We can do this since the window will be gone shortly.
+      delete win.gDNSService;
+      win.gDNSService = {
+        asyncResolve() {
+          gDNSResolved = true;
+          return gRealDNSService.asyncResolve(...arguments);
+        },
+      };
     } else {
       win = window;
     }
 
-    // Remove the domain from the whitelist.
+    // Remove the domain from the whitelist, the notification sould appear,
+    // unless we are in private browsing mode.
     Services.prefs.setBoolPref(pref, false);
 
     await BrowserTestUtils.withNewTab(
       {
         gBrowser: win.gBrowser,
         url: "about:blank",
       },
       browser =>
         runURLBarSearchTest({
           valueToOpen: hostName,
           expectSearch: true,
           expectNotification: true,
+          expectDNSResolve: true,
           aWindow: win,
         })
     );
 
     // check pref value
     let prefValue = Services.prefs.getBoolPref(pref);
     is(prefValue, !isPrivate, "Pref should have the correct state.");
 
@@ -232,40 +264,62 @@ function get_test_function_for_localhost
         gBrowser: win.gBrowser,
         url: "about:blank",
       },
       browser =>
         runURLBarSearchTest({
           valueToOpen: hostName,
           expectSearch: isPrivate,
           expectNotification: isPrivate,
+          expectDNSResolve: isPrivate,
           aWindow: win,
         })
     );
 
     if (isPrivate) {
       info("Waiting for private window to close");
       await BrowserTestUtils.closeWindow(win);
-      await new Promise(resolve => {
-        info("Waiting for focus");
-        waitForFocus(resolve, window);
-      });
+      await SimpleTest.promiseFocus(window);
     }
   };
 }
 
 add_task(get_test_function_for_localhost_with_hostname("localhost"));
 add_task(get_test_function_for_localhost_with_hostname("localhost."));
 add_task(get_test_function_for_localhost_with_hostname("localhost", true));
 
+add_task(async function test_dnsResolveSingleWordsAfterSearch() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["browser.urlbar.dnsResolveSingleWordsAfterSearch", 0],
+      ["browser.fixup.domainwhitelist.localhost", false],
+    ],
+  });
+  await BrowserTestUtils.withNewTab(
+    {
+      gBrowser,
+      url: "about:blank",
+    },
+    browser =>
+      runURLBarSearchTest({
+        valueToOpen: "localhost",
+        expectSearch: true,
+        expectNotification: false,
+        expectDNSResolve: false,
+      })
+  );
+  await SpecialPowers.popPrefEnv();
+});
+
 add_task(async function test_navigate_invalid_url() {
   let tab = (gBrowser.selectedTab = BrowserTestUtils.addTab(
     gBrowser,
     "about:blank"
   ));
   await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await runURLBarSearchTest({
     valueToOpen: "mozilla is awesome",
     expectSearch: true,
     expectNotification: false,
+    expectDNSResolve: false,
   });
   gBrowser.removeTab(tab);
 });