Bug 1107883 - support ipv6 URLs in the awesomebar with unified autocomplete enabled. r=mak, r=smaug

docshell/base/nsDefaultURIFixup.cpp

@@ -382,28 +382,8 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI,
     info->mFixupCreatedAlternateURI = MakeAlternateURI(info->mFixedURI);
   }
 
-  // If there is no relevent dot in the host, do we require the domain to
-  // be whitelisted?
   if (info->mFixedURI) {
-    if (aFixupFlags & FIXUP_FLAG_REQUIRE_WHITELISTED_HOST) {
-      nsAutoCString asciiHost;
-      if (NS_SUCCEEDED(info->mFixedURI->GetAsciiHost(asciiHost)) &&
-          !asciiHost.IsEmpty()) {
-        uint32_t dotLoc = uint32_t(asciiHost.FindChar('.'));
-
-        if ((dotLoc == uint32_t(kNotFound) ||
-             dotLoc == asciiHost.Length() - 1)) {
-          if (IsDomainWhitelisted(asciiHost, dotLoc)) {
-            info->mPreferredURI = info->mFixedURI;
-          }
-        } else {
-          info->mPreferredURI = info->mFixedURI;
-        }
-      }
-    } else {
-      info->mPreferredURI = info->mFixedURI;
-    }
-
+    info->mPreferredURI = info->mFixedURI;
     return NS_OK;
   }
 
@@ -1111,6 +1091,15 @@ nsDefaultURIFixup::IsDomainWhitelisted(const nsAutoCString aAsciiHost,
   return Preferences::GetBool(pref.get(), false);
 }
 
+NS_IMETHODIMP
+nsDefaultURIFixup::IsDomainWhitelisted(const nsACString& aDomain,
+                                       const uint32_t aDotLoc,
+                                       bool* aResult)
+{
+  *aResult = IsDomainWhitelisted(nsAutoCString(aDomain), aDotLoc);
+  return NS_OK;
+}
+
 /* Implementation of nsIURIFixupInfo */
 NS_IMPL_ISUPPORTS(nsDefaultURIFixupInfo, nsIURIFixupInfo)
 

docshell/base/nsIURIFixup.idl

@@ -70,7 +70,7 @@ interface nsIURIFixupInfo : nsISupports
 /**
  * Interface implemented by objects capable of fixing up strings into URIs
  */
-[scriptable, uuid(d2a78abe-e678-4103-9bcc-dd1377460c44)]
+[scriptable, uuid(1da7e9d4-620b-4949-849a-1cd6077b1b2d)]
 interface nsIURIFixup : nsISupports
 {
     /** No fixup flags. */
@@ -89,19 +89,13 @@ interface nsIURIFixup : nsISupports
      */
     const unsigned long FIXUP_FLAGS_MAKE_ALTERNATE_URI = 2;
 
-    /**
-     * For an input that may be just a domain with only 1 level (eg, "mozilla"),
-     * require that the host be whitelisted.
-     *
-     * Overridden by FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP.
-     */
-    const unsigned long FIXUP_FLAG_REQUIRE_WHITELISTED_HOST = 4;
-
     /*
      * Fix common scheme typos.
      */
     const unsigned long FIXUP_FLAG_FIX_SCHEME_TYPOS = 8;
 
+    /* NB: If adding an extra flag, 4 is free (again) */
+
     /**
      * Converts an internal URI (e.g. a wyciwyg URI) into one which we can
      * expose to the user, for example on the URL bar.
@@ -155,5 +149,18 @@ interface nsIURIFixup : nsISupports
      */
     nsIURIFixupInfo keywordToURI(in AUTF8String aKeyword,
                                  [optional] out nsIInputStream aPostData);
+
+    /**
+     * Returns true if the specified domain is whitelisted and false otherwise.
+     * A whitelisted domain is relevant when we have a single word and can't be
+     * sure whether to treat the word as a host name or should instead be
+     * treated as a search term.
+     *
+     * @param aDomain A domain name to query.
+     * @param aDotPos The position of the first '.' character in aDomain, or
+     *                -1 if no '.' character exists.
+     */
+    bool isDomainWhitelisted(in AUTF8String aDomain,
+                             in uint32_t aDotPos);
 };
 

docshell/test/unit/test_nsDefaultURIFixup_info.js

@@ -38,7 +38,6 @@ 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,
 ];
 
 flagInputs.concat([
@@ -56,7 +55,6 @@ flagInputs.concat([
     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
   }
@@ -176,25 +174,21 @@ let testcases = [ {
     fixedURI: "http://[::1]/",
     alternateURI: "http://[::1]/",
     protocolChange: true,
-    affectedByWhitelist: true
   }, {
     input: "[::1]/",
     fixedURI: "http://[::1]/",
     alternateURI: "http://[::1]/",
     protocolChange: true,
-    affectedByWhitelist: true,
   }, {
     input: "[::1]:8000",
     fixedURI: "http://[::1]:8000/",
     alternateURI: "http://[::1]:8000/",
     protocolChange: true,
-    affectedByWhitelist: true,
   }, {
     input: "[::1]:8000/",
     fixedURI: "http://[::1]:8000/",
     alternateURI: "http://[::1]:8000/",
     protocolChange: true,
-    affectedByWhitelist: true,
   }, {
     input: "[[::1]]/",
     keywordLookup: true,
@@ -204,7 +198,6 @@ let testcases = [ {
     fixedURI: "http://[fe80:cd00:0:cde:1257:0:211e:729c]/",
     alternateURI: "http://[fe80:cd00:0:cde:1257:0:211e:729c]/",
     protocolChange: true,
-    affectedByWhitelist: true
   }, {
     input: "[64:ff9b::8.8.8.8]",
     fixedURI: "http://[64:ff9b::8.8.8.8]/",
@@ -223,7 +216,6 @@ let testcases = [ {
     alternateURI: "http://[::1][100/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "[::1]]",
@@ -235,21 +227,18 @@ let testcases = [ {
     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.",
@@ -257,7 +246,6 @@ let testcases = [ {
     alternateURI: "http://www.test./",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: ".test",
@@ -292,7 +280,6 @@ let testcases = [ {
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "   mozilla  ",
@@ -300,13 +287,11 @@ let testcases = [ {
     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,
@@ -321,7 +306,6 @@ let testcases = [ {
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "mozilla \r\n",
@@ -329,7 +313,6 @@ let testcases = [ {
     alternateURI: "http://www.mozilla.com/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "moz\r\nfirefox\nos\r",
@@ -337,7 +320,6 @@ let testcases = [ {
     alternateURI: "http://www.mozfirefoxos.com/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "moz\r\n firefox\n",
@@ -355,7 +337,6 @@ let testcases = [ {
     input: "http://whitelisted/",
     fixedURI: "http://whitelisted/",
     alternateURI: "http://www.whitelisted.com/",
-    affectedByWhitelist: true,
     inWhitelist: true,
   }, {
     input: "café.local",
@@ -393,7 +374,6 @@ let testcases = [ {
     alternateURI: "http://www.5+2.com/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "5/2",
@@ -401,7 +381,6 @@ let testcases = [ {
     alternateURI: "http://www.5.com/2",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "moz ?.::%27",
@@ -444,20 +423,17 @@ let testcases = [ {
     alternateURI: "http://www.'.com/?",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true
   }, {
     input: "a?.com",
     fixedURI: "http://a/?.com",
     alternateURI: "http://www.a.com/?.com",
     protocolChange: true,
-    affectedByWhitelist: true
   }, {
     input: "?'.com",
     fixedURI: "http:///?%27.com",
     alternateURI: "http://www..com/?%27.com",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true
   }, {
     input: "' ?.com",
     keywordLookup: true,
@@ -468,40 +444,34 @@ let testcases = [ {
     alternateURI: "http://www..com/?mozilla",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true
   }, {
     input: "??mozilla",
     fixedURI: "http:///??mozilla",
     alternateURI: "http://www..com/??mozilla",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true
   }, {
     input: "mozilla/",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     protocolChange: true,
-    affectedByWhitelist: true,
   }, {
     input: "mozilla",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     protocolChange: true,
     keywordLookup: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   }, {
     input: "mozilla5/2",
     fixedURI: "http://mozilla5/2",
     alternateURI: "http://www.mozilla5.com/2",
     protocolChange: true,
-    affectedByWhitelist: true,
   }, {
     input: "mozilla/foo",
     fixedURI: "http://mozilla/foo",
     alternateURI: "http://www.mozilla.com/foo",
     protocolChange: true,
-    affectedByWhitelist: true,
   }];
 
 if (Services.appinfo.OS.toLowerCase().startsWith("win")) {
@@ -515,14 +485,12 @@ if (Services.appinfo.OS.toLowerCase().startsWith("win")) {
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     protocolChange: true,
-    affectedByWhitelist: true,
   });
   testcases.push({
     input: "mozilla\\",
     fixedURI: "http://mozilla/",
     alternateURI: "http://www.mozilla.com/",
     protocolChange: true,
-    affectedByWhitelist: true,
   });
 } else {
   testcases.push({
@@ -541,7 +509,6 @@ if (Services.appinfo.OS.toLowerCase().startsWith("win")) {
     alternateURI: "http://www.mozilla/",
     keywordLookup: true,
     protocolChange: true,
-    affectedByWhitelist: true,
     affectedByDNSForSingleHosts: true,
   });
 }
@@ -578,7 +545,6 @@ function do_single_test_run() {
              alternateURI: alternativeURI,
              keywordLookup: expectKeywordLookup,
              protocolChange: expectProtocolChange,
-             affectedByWhitelist: affectedByWhitelist,
              inWhitelist: inWhitelist,
              affectedByDNSForSingleHosts: affectedByDNSForSingleHosts,
            } of relevantTests) {
@@ -586,7 +552,6 @@ function do_single_test_run() {
     // Explicitly force these into a boolean
     expectKeywordLookup = !!expectKeywordLookup;
     expectProtocolChange = !!expectProtocolChange;
-    affectedByWhitelist = !!affectedByWhitelist;
     inWhitelist = !!inWhitelist;
     affectedByDNSForSingleHosts = !!affectedByDNSForSingleHosts;
 
@@ -637,10 +602,9 @@ function do_single_test_run() {
       do_check_eq(info.fixupCreatedAlternateURI, makeAlternativeURI && alternativeURI != null);
 
       // Check the preferred URI
-      let requiresWhitelistedDomain = flags & urifixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST;
       if (couldDoKeywordLookup) {
         if (expectKeywordLookup) {
-          if (!affectedByWhitelist || (affectedByWhitelist && !inWhitelist)) {
+          if (!inWhitelist) {
             let urlparamInput = encodeURIComponent(sanitize(testInput)).replace(/%20/g, "+");
             // If the input starts with `?`, then info.preferredURI.spec will omit it
             // In order to test this behaviour, remove `?` only if it is the first character
@@ -656,14 +620,6 @@ function do_single_test_run() {
         } 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 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:

toolkit/components/places/UnifiedComplete.js

@@ -65,6 +65,11 @@ const TELEMETRY_6_FIRST_RESULTS = "PLACES_AUTOCOMPLETE_6_FIRST_RESULTS_TIME_MS";
 // The default frecency value used when inserting matches with unknown frecency.
 const FRECENCY_DEFAULT = 1000;
 
+// A regex that matches "single word" hostnames for whitelisting purposes.
+// The hostname will already have been checked for general validity, so we
+// don't need to be exhaustive here, so allow dashes anywhere.
+const REGEXP_SINGLEWORD_HOST = new RegExp("^[a-z0-9-]+$", "i");
+
 // Sqlite result row index constants.
 const QUERYINDEX_QUERYTYPE     = 0;
 const QUERYINDEX_URL           = 1;
@@ -1005,7 +1010,7 @@ Search.prototype = {
   // scheme isn't specificed.
   _matchUnknownUrl: function* () {
     let flags = Ci.nsIURIFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS |
-                Ci.nsIURIFixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST;
+                Ci.nsIURIFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
     let fixupInfo = null;
     try {
       fixupInfo = Services.uriFixup.getFixupURIInfo(this._originalSearchString,
@@ -1014,15 +1019,31 @@ Search.prototype = {
       return false;
     }
 
-    let uri = fixupInfo.preferredURI;
+    // If the URI cannot be fixed or the preferred URI would do a keyword search,
+    // that basically means this isn't useful to us. Note that
+    // fixupInfo.keywordAsSent will never be true if the keyword.enabled pref
+    // is false or there are no engines, so in that case we will always return
+    // a "visit".
+    if (!fixupInfo.fixedURI || fixupInfo.keywordAsSent)
+      return false;
+
+    let uri = fixupInfo.fixedURI;
     // Check the host, as "http:///" is a valid nsIURI, but not useful to us.
     // But, some schemes are expected to have no host. So we check just against
     // schemes we know should have a host. This allows new schemes to be
     // implemented without us accidentally blocking access to them.
     let hostExpected = new Set(["http", "https", "ftp", "chrome", "resource"]);
-    if (!uri || (hostExpected.has(uri.scheme) && !uri.host))
+    if (hostExpected.has(uri.scheme) && !uri.host)
       return false;
 
+    // If the result is something that looks like a single-worded hostname
+    // we need to check the domain whitelist to treat it as such.
+    if (uri.asciiHost &&
+        REGEXP_SINGLEWORD_HOST.test(uri.asciiHost) &&
+        !Services.uriFixup.isDomainWhitelisted(uri.asciiHost, -1)) {
+      return false;
+    }
+
     let value = makeActionURL("visiturl", {
       url: uri.spec,
       input: this._originalSearchString,

toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js

@@ -302,11 +302,18 @@ function makeActionURI(action, params) {
   return NetUtil.newURI(url);
 }
 
-// Hide all the search engines so they don't influence tests results.
-add_task(function ensure_no_search_engines() {
-  let count = {};
-  let engines = Services.search.getEngines(count);
-  for (let i = 0; i < count.value; i++) {
-    engines[i].hidden = true;
+// Ensure we have a default search engine and the keyword.enabled preference
+// set.
+add_task(function ensure_search_engine() {
+  // keyword.enabled is necessary for the tests to see keyword searches.
+  Services.prefs.setBoolPref("keyword.enabled", true);
+
+  // Remove any existing engines before adding ours.
+  for (let engine of Services.search.getEngines()) {
+    Services.search.removeEngine(engine);
   }
+  Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
+                                       "http://s.example.com/search");
+  let engine = Services.search.getEngineByName("MozSearch");
+  Services.search.currentEngine = engine;
 });

toolkit/components/places/tests/unifiedcomplete/test_empty_search.js

@@ -49,9 +49,13 @@ add_task(function* test_javascript_match() {
                { uri: uri4, title: "title", style: ["bookmark"] },
                { uri: uri5, title: "title", style: ["bookmark"] },
                { uri: uri6, title: "title", style: ["bookmark"] },
-               { uri: makeActionURI("switchtab", {url: "http://t.foo/6"}), title: "title", style: [ "action,switchtab" ] }, ]
+               { uri: makeActionURI("switchtab", {url: "http://t.foo/6"}), title: "title", style: [ "action,switchtab" ] },
+               { uri: makeActionURI("searchengine", {engineName: "MozSearch", input: "foo", searchQuery: "foo"}), title: "MozSearch", style: [ "action", "searchengine" ] },
+             ]
   });
 
+  // Note the next few tests do *not* get a search result as enable-actions
+  // isn't specified.
   do_print("Match only typed history");
   yield check_autocomplete({
     search: "foo ^ ~",
@@ -82,7 +86,10 @@ add_task(function* test_javascript_match() {
   yield check_autocomplete({
     search: "",
     searchParam: "enable-actions",
-    matches: [ { uri: makeActionURI("switchtab", {url: "http://t.foo/6"}), title: "title", style: [ "action,switchtab" ] }, ]
+    matches: [
+               { uri: makeActionURI("switchtab", {url: "http://t.foo/6"}), title: "title", style: [ "action,switchtab" ] },
+               { uri: makeActionURI("searchengine", {engineName: "MozSearch", input: "", searchQuery: ""}), title: "MozSearch", style: [ "action", "searchengine" ] },
+             ]
   });
 
   Services.prefs.clearUserPref("browser.urlbar.suggest.history");

toolkit/components/places/tests/unifiedcomplete/test_searchEngine_alias.js

@@ -3,10 +3,8 @@
 
 
 add_task(function*() {
-  Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
-                                       "http://s.example.com/search");
-  let engine = Services.search.getEngineByName("MozSearch");
-  Services.search.currentEngine = engine;
+  // Note that head_autocomplete.js has already added a MozSearch engine.
+  // Here we add another engine with a search alias.
   Services.search.addEngineWithDetails("AliasedMozSearch", "", "doit", "",
                                        "GET", "http://s.example.com/search");
 

toolkit/components/places/tests/unifiedcomplete/test_searchEngine_current.js

@@ -3,10 +3,8 @@
 
 
 add_task(function*() {
-  Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
-                                       "http://s.example.com/search");
-  let engine = Services.search.getEngineByName("MozSearch");
-  Services.search.currentEngine = engine;
+  // Note that head_autocomplete.js has already added a MozSearch engine.
+  // Here we add another engine with a search alias.
   Services.search.addEngineWithDetails("AliasedMozSearch", "", "doit", "",
                                        "GET", "http://s.example.com/search");
 

toolkit/components/places/tests/unifiedcomplete/test_tabmatches.js

@@ -7,11 +7,6 @@
 let gTabRestrictChar = "%";
 
 add_task(function* test_tab_matches() {
-  Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
-                                       "http://s.example.com/search");
-  let engine = Services.search.getEngineByName("MozSearch");
-  Services.search.currentEngine = engine;
-
   let uri1 = NetUtil.newURI("http://abc.com/");
   let uri2 = NetUtil.newURI("http://xyz.net/");
   let uri3 = NetUtil.newURI("about:mozilla");

toolkit/components/places/tests/unifiedcomplete/test_visiturl.js

@@ -38,10 +38,6 @@ add_task(function*() {
 
   // And hosts with no dot in them are special, due to requiring whitelisting.
   do_print("visit url, host matching visited host but not visited url, non-whitelisted host");
-  Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
-                                       "http://s.example.com/search");
-  let engine = Services.search.getEngineByName("MozSearch");
-  Services.search.currentEngine = engine;
   yield PlacesTestUtils.addVisits([
     { uri: NetUtil.newURI("http://mozilla/bourbon/"), title: "Mozilla Bourbon", transition: TRANSITION_TYPED },
   ]);
@@ -50,4 +46,19 @@ add_task(function*() {
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("searchengine", {engineName: "MozSearch", input: "mozilla/rum", searchQuery: "mozilla/rum"}), title: "MozSearch", style: [ "action", "searchengine" ] } ]
   });
+
+  // ipv4 and ipv6 literal addresses should offer to visit.
+  do_print("visit url, ipv4 literal");
+  yield check_autocomplete({
+    search: "127.0.0.1",
+    searchParam: "enable-actions",
+    matches: [ { uri: makeActionURI("visiturl", {url: "http://127.0.0.1/", input: "127.0.0.1"}), title: "http://127.0.0.1/", style: ["action", "visiturl"] } ]
+  });
+
+  do_print("visit url, ipv6 literal");
+  yield check_autocomplete({
+    search: "[2001:db8::1]",
+    searchParam: "enable-actions",
+    matches: [ { uri: makeActionURI("visiturl", {url: "http://[2001:db8::1]/", input: "[2001:db8::1]"}), title: "http://[2001:db8::1]/", style: ["action", "visiturl"] } ]
+  });
 });