Bug 1088050 - Add a pref to disable search for single-word hosts. r=smaug
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Sun, 16 Nov 2014 23:01:59 +0000
changeset 217693 b16c0306ec5711c600f4d104ae62dc977b2d1e10
parent 217692 3fec8eb74f104d6fcd82180bc808e9a305036888
child 217694 5c804aa57e4b6449d0607b6d17a449cbb9be2663
push id27887
push userryanvm@gmail.com
push dateThu, 27 Nov 2014 02:08:38 +0000
treeherdermozilla-central@c63e741bca2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1088050
milestone36.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 1088050 - Add a pref to disable search for single-word hosts. r=smaug
docshell/base/nsDefaultURIFixup.cpp
docshell/test/unit/test_nsDefaultURIFixup_info.js
modules/libpref/init/all.js
--- a/docshell/base/nsDefaultURIFixup.cpp
+++ b/docshell/base/nsDefaultURIFixup.cpp
@@ -29,16 +29,17 @@
 
 using namespace mozilla;
 
 /* Implementation file */
 NS_IMPL_ISUPPORTS(nsDefaultURIFixup, nsIURIFixup)
 
 static bool sInitializedPrefCaches = false;
 static bool sFixTypos = true;
+static bool sDNSFirstForSingleWords = false;
 static bool sFixupKeywords = true;
 
 nsDefaultURIFixup::nsDefaultURIFixup()
 {
   /* member initializers and constructor code */
 }
 
 
@@ -239,16 +240,21 @@ nsDefaultURIFixup::GetFixupURIInfo(const
     if (!sInitializedPrefCaches) {
       // Check if we want to fix up common scheme typos.
       rv = Preferences::AddBoolVarCache(&sFixTypos,
                                         "browser.fixup.typo.scheme",
                                         sFixTypos);
       MOZ_ASSERT(NS_SUCCEEDED(rv),
                 "Failed to observe \"browser.fixup.typo.scheme\"");
 
+      rv = Preferences::AddBoolVarCache(&sDNSFirstForSingleWords,
+                                        "browser.fixup.dns_first_for_single_words",
+                                        sDNSFirstForSingleWords);
+      MOZ_ASSERT(NS_SUCCEEDED(rv), "Failed to observe \"browser.fixup.dns_first_for_single_words\"");
+
       rv = Preferences::AddBoolVarCache(&sFixupKeywords, "keyword.enabled",
                                         sFixupKeywords);
       MOZ_ASSERT(NS_SUCCEEDED(rv), "Failed to observe \"keyword.enabled\"");
       sInitializedPrefCaches = true;
     }
 
     // Fix up common scheme typos.
     if (sFixTypos && (aFixupFlags & FIXUP_FLAG_FIX_SCHEME_TYPOS)) {
@@ -1062,26 +1068,28 @@ nsDefaultURIFixup::KeywordURIFixup(const
     // If there are only colons and only hexadecimal characters ([a-z][0-9])
     // enclosed in [], then don't do a keyword lookup
     if (looksLikeIpv6) {
         return NS_OK;
     }
 
     nsresult rv = NS_OK;
     // We do keyword lookups if a space or quote preceded the dot, colon
-    // or question mark (or if the latter were not found)
-    // or when the host is the same as asciiHost and there are no
-    // characters from [a-z][A-Z]
+    // or question mark (or if the latter is not found, or if the input starts with a question mark)
     if (((firstSpaceLoc < firstDotLoc || firstQuoteLoc < firstDotLoc) &&
          (firstSpaceLoc < firstColonLoc || firstQuoteLoc < firstColonLoc) &&
-         (firstSpaceLoc < firstQMarkLoc || firstQuoteLoc < firstQMarkLoc)) || firstQMarkLoc == 0 ||
-        (isValidAsciiHost && isValidHost && !hasAsciiAlpha &&
-         host.EqualsIgnoreCase(asciiHost.get()))) {
-
+         (firstSpaceLoc < firstQMarkLoc || firstQuoteLoc < firstQMarkLoc)) || firstQMarkLoc == 0) {
         rv = TryKeywordFixupForURIInfo(aFixupInfo->mOriginalInput, aFixupInfo, aPostData);
+    // ... or when the host is the same as asciiHost and there are no
+    // characters from [a-z][A-Z]
+    } else if (isValidAsciiHost && isValidHost && !hasAsciiAlpha &&
+               host.EqualsIgnoreCase(asciiHost.get())) {
+        if (!sDNSFirstForSingleWords) {
+            rv = TryKeywordFixupForURIInfo(aFixupInfo->mOriginalInput, aFixupInfo, aPostData);
+        }
     }
     // ... or if there is no question mark or colon, and there is either no
     // dot, or exactly 1 and it is the first or last character of the input:
     else if ((firstDotLoc == uint32_t(kNotFound) ||
               (foundDots == 1 && (firstDotLoc == 0 || firstDotLoc == aURIString.Length() - 1))) &&
               firstColonLoc == uint32_t(kNotFound) && firstQMarkLoc == uint32_t(kNotFound)) {
 
         if (isValidAsciiHost && IsDomainWhitelisted(asciiHost, firstDotLoc)) {
@@ -1093,16 +1101,19 @@ nsDefaultURIFixup::KeywordURIFixup(const
         rv = TryKeywordFixupForURIInfo(aFixupInfo->mOriginalInput, aFixupInfo, aPostData);
     }
     return rv;
 }
 
 bool nsDefaultURIFixup::IsDomainWhitelisted(const nsAutoCString aAsciiHost,
                                             const uint32_t aDotLoc)
 {
+    if (sDNSFirstForSingleWords) {
+        return true;
+    }
     // Check if this domain is whitelisted as an actual
     // domain (which will prevent a keyword query)
     // NB: any processing of the host here should stay in sync with
     // code in the front-end(s) that set the pref.
 
     nsAutoCString pref("browser.fixup.domainwhitelist.");
 
     if (aDotLoc == aAsciiHost.Length() - 1) {
--- a/docshell/test/unit/test_nsDefaultURIFixup_info.js
+++ b/docshell/test/unit/test_nsDefaultURIFixup_info.js
@@ -15,26 +15,28 @@ Services.search.addEngineWithDetails(kSe
                                      kSearchEngineURL);
 
 let oldDefaultEngine = Services.search.defaultEngine;
 Services.search.defaultEngine = Services.search.getEngineByName(kSearchEngineID);
 
 let selectedName = Services.search.defaultEngine.name;
 do_check_eq(selectedName, kSearchEngineID);
 
+const kForceHostLookup = "browser.fixup.dns_first_for_single_words";
 do_register_cleanup(function() {
   if (oldDefaultEngine) {
     Services.search.defaultEngine = oldDefaultEngine;
   }
   let engine = Services.search.getEngineByName(kSearchEngineID);
   if (engine) {
     Services.search.removeEngine(engine);
   }
   Services.prefs.clearUserPref("keyword.enabled");
   Services.prefs.clearUserPref("browser.fixup.typo.scheme");
+  Services.prefs.clearUserPref(kForceHostLookup);
 });
 
 let flagInputs = [
   urifixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP,
   urifixup.FIXUP_FLAGS_MAKE_ALTERNATE_URI,
   urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS,
   urifixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST,
 ];
@@ -51,16 +53,17 @@ flagInputs.concat([
   {
     input: "", // Input string, required
     fixedURI: "", // Expected fixedURI
     alternateURI: "", // Expected alternateURI
     keywordLookup: false, // Whether a keyword lookup is expected
     protocolChange: false, // Whether a protocol change is expected
     affectedByWhitelist: false, // Whether the input host is affected by the whitelist
     inWhitelist: false, // Whether the input host is in the whitelist
+    affectedByDNSForSingleHosts: false, // Whether the input host could be a host, but is normally assumed to be a keyword query
   }
 */
 let testcases = [ {
     input: "http://www.mozilla.org",
     fixedURI: "http://www.mozilla.org/",
   }, {
     input: "http://127.0.0.1/",
     fixedURI: "http://127.0.0.1/",
@@ -111,29 +114,31 @@ let testcases = [ {
     input: "192.168.10.110",
     fixedURI: "http://192.168.10.110/",
     protocolChange: true,
   }, {
     input: "1.2.3",
     fixedURI: "http://1.2.3/",
     keywordLookup: true,
     protocolChange: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "1.2.3/",
     fixedURI: "http://1.2.3/",
     protocolChange: true,
   }, {
     input: "1.2.3/foo",
     fixedURI: "http://1.2.3/foo",
     protocolChange: true,
   }, {
     input: "1.2.3:8000",
     fixedURI: "http://1.2.3:8000/",
     keywordLookup: true,
     protocolChange: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "1.2.3:8000/",
     fixedURI: "http://1.2.3:8000/",
     protocolChange: true,
   }, {
     input: "1.2.3:8000/foo",
     fixedURI: "http://1.2.3:8000/foo",
     protocolChange: true,
@@ -193,54 +198,59 @@ let testcases = [ {
     keywordLookup: true,
     protocolChange: true
   }, {
     input: "[::1][100",
     fixedURI: "http://[::1][100/",
     alternateURI: "http://[::1][100/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true
+    affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "[::1]]",
     keywordLookup: true,
     protocolChange: true
   }, {
     input: "1234",
     fixedURI: "http://1234/",
     alternateURI: "http://www.1234.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "host/foo.txt",
     fixedURI: "http://host/foo.txt",
     alternateURI: "http://www.host.com/foo.txt",
     protocolChange: true,
     affectedByWhitelist: true,
   }, {
     input: "mozilla",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "test.",
     fixedURI: "http://test./",
     alternateURI: "http://www.test./",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: ".test",
     fixedURI: "http://.test/",
     alternateURI: "http://www..test/",
     keywordLookup: true,
     protocolChange: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "mozilla is amazing",
     keywordLookup: true,
     protocolChange: true,
   }, {
     input: "search ?mozilla",
     keywordLookup: true,
     protocolChange: true,
@@ -258,23 +268,25 @@ let testcases = [ {
     protocolChange: true,
   }, {
     input: "mozilla ",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "   mozilla  ",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "mozilla \\",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
   }, {
     input: "mozilla \\ foo.txt",
     keywordLookup: true,
@@ -285,30 +297,33 @@ let testcases = [ {
     protocolChange: true,
   }, {
     input: "mozilla\n",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "mozilla \r\n",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "moz\r\nfirefox\nos\r",
     fixedURI: "http://mozfirefoxos/",
     alternateURI: "http://www.mozfirefoxos.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "moz\r\n firefox\n",
     keywordLookup: true,
     protocolChange: true,
   }, {
     input: "",
     keywordLookup: true,
     protocolChange: true,
@@ -326,40 +341,45 @@ let testcases = [ {
     input: "café.local",
     fixedURI: "http://café.local/",
     alternateURI: "http://www.café.local/",
     protocolChange: true
   }, {
     input: "47.6182,-122.830",
     fixedURI: "http://47.6182,-122.830/",
     keywordLookup: true,
-    protocolChange: true
+    protocolChange: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "-47.6182,-23.51",
     fixedURI: "http://-47.6182,-23.51/",
     keywordLookup: true,
-    protocolChange: true
+    protocolChange: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "-22.14,23.51-",
     fixedURI: "http://-22.14,23.51-/",
     keywordLookup: true,
-    protocolChange: true
+    protocolChange: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "32.7",
     fixedURI: "http://32.7/",
     alternateURI: "http://www.32.7/",
     keywordLookup: true,
-    protocolChange: true
+    protocolChange: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "5+2",
     fixedURI: "http://5+2/",
     alternateURI: "http://www.5+2.com/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true
+    affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   }, {
     input: "moz ?.::%27",
     keywordLookup: true,
     protocolChange: true
   }, {
     input: "mozilla.com/?q=search",
     fixedURI: "http://mozilla.com/?q=search",
     alternateURI: "http://www.mozilla.com/?q=search",
@@ -445,16 +465,17 @@ if (Services.appinfo.OS.toLowerCase().st
   });
   testcases.push({
     input: "mozilla\\",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   });
 } else {
   testcases.push({
     input: "/some/file.txt",
     fixedURI: "file:///some/file.txt",
     protocolChange: true,
   });
   testcases.push({
@@ -464,37 +485,65 @@ if (Services.appinfo.OS.toLowerCase().st
   });
   testcases.push({
     input: "mozilla\\",
     fixedURI: "http://mozilla\\/",
     alternateURI: "http://www.mozilla/",
     keywordLookup: true,
     protocolChange: true,
     affectedByWhitelist: true,
+    affectedByDNSForSingleHosts: true,
   });
 }
 
 function sanitize(input) {
   return input.replace(/\r|\n/g, "").trim();
 }
 
+
+let gSingleWordHostLookup = false;
 function run_test() {
+  // Only keywordlookup things should be affected by requiring a DNS lookup for single-word hosts:
+  do_print("Check only keyword lookup testcases should be affected by requiring DNS for single hosts");
+  let affectedTests = testcases.filter(t => !t.keywordLookup && t.affectedByDNSForSingleHosts);
+  if (affectedTests.length) {
+    for (let testcase of affectedTests) {
+      do_print("Affected: " + testcase.input);
+    }
+  }
+  do_check_eq(affectedTests.length, 0);
+  do_single_test_run();
+  gSingleWordHostLookup = true;
+  do_single_test_run();
+}
+
+function do_single_test_run() {
+  Services.prefs.setBoolPref(kForceHostLookup, gSingleWordHostLookup);
+
+  let relevantTests = gSingleWordHostLookup ? testcases.filter(t => t.keywordLookup) :
+                                              testcases;
+
   for (let { input: testInput,
              fixedURI: expectedFixedURI,
              alternateURI: alternativeURI,
              keywordLookup: expectKeywordLookup,
              protocolChange: expectProtocolChange,
              affectedByWhitelist: affectedByWhitelist,
-             inWhitelist: inWhitelist } of testcases) {
+             inWhitelist: inWhitelist,
+             affectedByDNSForSingleHosts: affectedByDNSForSingleHosts,
+           } of relevantTests) {
 
     // Explicitly force these into a boolean
     expectKeywordLookup = !!expectKeywordLookup;
     expectProtocolChange = !!expectProtocolChange;
     affectedByWhitelist = !!affectedByWhitelist;
     inWhitelist = !!inWhitelist;
+    affectedByDNSForSingleHosts = !!affectedByDNSForSingleHosts;
+
+    expectKeywordLookup = expectKeywordLookup && (!affectedByDNSForSingleHosts || !gSingleWordHostLookup);
 
     for (let flags of flagInputs) {
       let info;
       let fixupURIOnly = null;
       try {
         fixupURIOnly = urifixup.createFixupURI(testInput, flags);
       } catch (ex) {
         do_print("Caught exception: " + ex);
@@ -506,17 +555,18 @@ function run_test() {
       } catch (ex) {
         // Both APIs should return an error in the same cases.
         do_print("Caught exception: " + ex);
         do_check_eq(expectedFixedURI, null);
         do_check_eq(fixupURIOnly, null);
         continue;
       }
 
-      do_print("Checking \"" + testInput + "\" with flags " + flags);
+      do_print("Checking \"" + testInput + "\" with flags " + flags +
+               " (host lookup for single words: " + (gSingleWordHostLookup ? "yes" : "no") + ")");
 
       // Both APIs should then also be using the same spec.
       do_check_eq(!!fixupURIOnly, !!info.preferredURI);
       if (fixupURIOnly)
         do_check_eq(fixupURIOnly.spec, info.preferredURI.spec);
 
       let isFileURL = expectedFixedURI && expectedFixedURI.startsWith("file");
 
@@ -550,18 +600,20 @@ function run_test() {
             do_check_eq(spec, searchURL);
           } else {
             do_check_eq(info.preferredURI, null);
           }
         } else {
           do_check_eq(info.preferredURI.spec, info.fixedURI.spec);
         }
       } else if (requiresWhitelistedDomain) {
-        // Not a keyword search, but we want to enforce the host whitelist
-        if (!affectedByWhitelist || (affectedByWhitelist && inWhitelist))
+        // Not a keyword search, but we want to enforce the host whitelist.
+        // If we always do DNS lookups, we should always havea  preferred URI here - unless the
+        // input starts with '?' in which case preferredURI will just be null...
+        if (!affectedByWhitelist || (affectedByWhitelist && inWhitelist) || (gSingleWordHostLookup && !testInput.startsWith("?")))
           do_check_eq(info.preferredURI.spec, info.fixedURI.spec);
         else
           do_check_eq(info.preferredURI, null);
       } else {
         // In these cases, we should never be doing a keyword lookup and
         // the fixed URI should be preferred:
         do_check_eq(info.preferredURI.spec, info.fixedURI.spec);
       }
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -839,16 +839,17 @@ pref("slider.snapMultiplier", 0);
 
 // option to choose plug-in finder
 pref("application.use_ns_plugin_finder", false);
 
 // URI fixup prefs
 pref("browser.fixup.alternate.enabled", true);
 pref("browser.fixup.alternate.prefix", "www.");
 pref("browser.fixup.alternate.suffix", ".com");
+pref("browser.fixup.dns_first_for_single_words", false);
 pref("browser.fixup.hide_user_pass", true);
 
 // Location Bar AutoComplete
 pref("browser.urlbar.autocomplete.enabled", true);
 
 // Print header customization
 // Use the following codes:
 // &T - Title