Backed out changeset 7f58c77bb165 (bug 1057186) for xpshell bustage
authorNigel Babu <nigelbabu@gmail.com>
Sun, 07 Sep 2014 21:00:33 +0530
changeset 204064 ea6607178b559a93b1bbc1141f25287cc5e08456
parent 204063 7f58c77bb165098ee212e7b1c70fe09cf995c074
child 204065 71ed0213343d8211c6b4c1d847e17f5df4b0db12
push id48824
push usercbook@mozilla.com
push dateMon, 08 Sep 2014 13:30:41 +0000
treeherdermozilla-inbound@ffabfe9d47a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1057186
milestone35.0a1
backs out7f58c77bb165098ee212e7b1c70fe09cf995c074
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
Backed out changeset 7f58c77bb165 (bug 1057186) for xpshell bustage
docshell/base/nsDefaultURIFixup.cpp
docshell/base/nsDefaultURIFixup.h
docshell/base/nsIURIFixup.idl
docshell/test/unit/test_nsDefaultURIFixup_info.js
--- a/docshell/base/nsDefaultURIFixup.cpp
+++ b/docshell/base/nsDefaultURIFixup.cpp
@@ -381,45 +381,24 @@ nsDefaultURIFixup::GetFixupURIInfo(const
 
     // Did the caller want us to try an alternative URI?
     // If so, attempt to fixup http://foo into http://www.foo.com
 
     if (info->mFixedURI && aFixupFlags & FIXUP_FLAGS_MAKE_ALTERNATE_URI) {
         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;
-        }
-
-        return NS_OK;
-    }
-
     // If we still haven't been able to construct a valid URI, try to force a
     // keyword match.  This catches search strings with '.' or ':' in them.
-    if (sFixupKeywords && (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP)) {
+    if (info->mFixedURI)
+    {
+        info->mPreferredURI = info->mFixedURI;
+    }
+    else if (sFixupKeywords && (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP))
+    {
         rv = KeywordToURI(aStringURI, aPostData, getter_AddRefs(info->mPreferredURI));
         if (NS_SUCCEEDED(rv) && info->mPreferredURI)
         {
             info->mFixupUsedKeyword = true;
             return NS_OK;
         }
     }
 
@@ -980,57 +959,47 @@ void nsDefaultURIFixup::KeywordURIFixup(
         }
     }
     // ... 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 ((dotLoc == uint32_t(kNotFound) ||
               (dotLoc == lastDotLoc && (dotLoc == 0 || dotLoc == aURIString.Length() - 1))) &&
              colonLoc == uint32_t(kNotFound) && qMarkLoc == uint32_t(kNotFound))
     {
-
         nsAutoCString asciiHost;
         if (aFixupInfo->mFixedURI &&
             NS_SUCCEEDED(aFixupInfo->mFixedURI->GetAsciiHost(asciiHost)) &&
-            !asciiHost.IsEmpty()) {
-
-            if (IsDomainWhitelisted(asciiHost, dotLoc)) {
+            !asciiHost.IsEmpty())
+        {
+            // 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 (dotLoc == aURIString.Length() - 1) {
+              pref.Append(Substring(asciiHost, 0, asciiHost.Length() - 1));
+            } else {
+              pref.Append(asciiHost);
+            }
+            if (Preferences::GetBool(pref.get(), false))
+            {
                 return;
             }
         }
-
         // If we get here, we don't have a valid URI, or we did but the
         // host is not whitelisted, so we do a keyword search *anyway*:
         rv = KeywordToURI(aFixupInfo->mOriginalInput, aPostData,
                           getter_AddRefs(aFixupInfo->mPreferredURI));
         if (NS_SUCCEEDED(rv) && aFixupInfo->mPreferredURI)
         {
             aFixupInfo->mFixupUsedKeyword = true;
         }
     }
 }
 
-bool nsDefaultURIFixup::IsDomainWhitelisted(const nsAutoCString aAsciiHost,
-                                            const uint32_t aDotLoc)
-{
-    // 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) {
-      pref.Append(Substring(aAsciiHost, 0, aAsciiHost.Length() - 1));
-    } else {
-      pref.Append(aAsciiHost);
-    }
-
-    return Preferences::GetBool(pref.get(), false);
-}
-
 
 nsresult NS_NewURIFixup(nsIURIFixup **aURIFixup)
 {
     nsDefaultURIFixup *fixup = new nsDefaultURIFixup;
     if (fixup == nullptr)
     {
         return NS_ERROR_OUT_OF_MEMORY;
     }
--- a/docshell/base/nsDefaultURIFixup.h
+++ b/docshell/base/nsDefaultURIFixup.h
@@ -32,18 +32,16 @@ private:
                               nsIURI** aURI);
     void KeywordURIFixup(const nsACString &aStringURI,
                          nsDefaultURIFixupInfo* aFixupInfo,
                          nsIInputStream** aPostData);
     bool PossiblyByteExpandedFileName(const nsAString& aIn);
     bool PossiblyHostPortUrl(const nsACString& aUrl);
     bool MakeAlternateURI(nsIURI *aURI);
     bool IsLikelyFTP(const nsCString& aHostSpec);
-    bool IsDomainWhitelisted(const nsAutoCString aAsciiHost,
-                             const uint32_t aDotLoc);
 };
 
 class nsDefaultURIFixupInfo : public nsIURIFixupInfo
 {
 public:
     NS_DECL_ISUPPORTS
     NS_DECL_NSIURIFIXUPINFO
 
--- a/docshell/base/nsIURIFixup.idl
+++ b/docshell/base/nsIURIFixup.idl
@@ -58,17 +58,17 @@ interface nsIURIFixupInfo : nsISupports
    */
   readonly attribute AUTF8String originalInput;
 };
 
 
 /**
  * Interface implemented by objects capable of fixing up strings into URIs
  */
-[scriptable, uuid(49298f2b-3630-4874-aecc-522300a7fead)]
+[scriptable, uuid(80d4932e-bb2e-4afb-98e0-de9cc9ea7d82)]
 interface nsIURIFixup : nsISupports
 {
     /** No fixup flags. */
     const unsigned long FIXUP_FLAG_NONE = 0;
 
     /**
      * Allow the fixup to use a keyword lookup service to complete the URI.
      * The fixup object implementer should honour this flag and only perform
@@ -78,28 +78,22 @@ interface nsIURIFixup : nsISupports
 
     /**
      * Tell the fixup to make an alternate URI from the input URI, for example
      * to turn foo into www.foo.com.
      */
     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;
 
+    /* Note that flag 4 is available. */
+
     /**
      * Converts an internal URI (e.g. a wyciwyg URI) into one which we can
      * expose to the user, for example on the URL bar.
      *
      * @param  aURI       The URI to be converted
      * @return nsIURI     The converted, exposable URI
      * @throws NS_ERROR_MALFORMED_URI when the exposable portion of aURI is malformed
      * @throws NS_ERROR_UNKNOWN_PROTOCOL when we can't get a protocol handler service
--- a/docshell/test/unit/test_nsDefaultURIFixup_info.js
+++ b/docshell/test/unit/test_nsDefaultURIFixup_info.js
@@ -1,15 +1,14 @@
 let urifixup = Cc["@mozilla.org/docshell/urifixup;1"].
                getService(Ci.nsIURIFixup);
 
 Components.utils.import("resource://gre/modules/Services.jsm");
 
-let prefList = ["browser.fixup.typo.scheme", "keyword.enabled",
-                "browser.fixup.domainwhitelist.whitelisted"];
+let prefList = ["browser.fixup.typo.scheme", "keyword.enabled"];
 for (let pref of prefList) {
   Services.prefs.setBoolPref(pref, true);
 }
 
 const kSearchEngineID = "test_urifixup_search_engine";
 const kSearchEngineURL = "http://www.example.org/?search={searchTerms}";
 Services.search.addEngineWithDetails(kSearchEngineID, "", "", "", "get",
                                      kSearchEngineURL);
@@ -30,233 +29,71 @@ do_register_cleanup(function() {
   }
   Services.prefs.clearUserPref("keyword.enabled");
   Services.prefs.clearUserPref("browser.fixup.typo.scheme");
 });
 
 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,
+  urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS
 ];
 
 flagInputs.concat([
   flagInputs[0] | flagInputs[1],
   flagInputs[1] | flagInputs[2],
   flagInputs[0] | flagInputs[2],
   flagInputs[0] | flagInputs[1] | flagInputs[2]
 ]);
 
-/*
-  The following properties are supported for these test cases:
-  {
-    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
-  }
-*/
-let testcases = [ {
-    input: "http://www.mozilla.org",
-    fixedURI: "http://www.mozilla.org/",
-  }, {
-    input: "http://127.0.0.1/",
-    fixedURI: "http://127.0.0.1/",
-  }, {
-    input: "file:///foo/bar",
-    fixedURI: "file:///foo/bar",
-  }, {
-    input: "://www.mozilla.org",
-    fixedURI: "http://www.mozilla.org/",
-    protocolChange: true,
-  }, {
-    input: "www.mozilla.org",
-    fixedURI: "http://www.mozilla.org/",
-    protocolChange: true,
-  }, {
-    input: "http://mozilla/",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-  }, {
-    input: "http://test./",
-    fixedURI: "http://test./",
-    alternateURI: "http://www.test./",
-  }, {
-    input: "127.0.0.1",
-    fixedURI: "http://127.0.0.1/",
-    protocolChange: true,
-  }, {
-    input: "1234",
-    fixedURI: "http://1234/",
-    alternateURI: "http://www.1234.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: 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,
-  }, {
-    input: "test.",
-    fixedURI: "http://test./",
-    alternateURI: "http://www.test./",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: ".test",
-    fixedURI: "http://.test/",
-    alternateURI: "http://www..test/",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla is amazing",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla ",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "   mozilla  ",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "mozilla \\",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "mozilla \\ foo.txt",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla \\\r foo.txt",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla\n",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "mozilla \r\n",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "moz\r\nfirefox\nos\r",
-    fixedURI: "http://mozfirefoxos/",
-    alternateURI: "http://www.mozfirefoxos.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "moz\r\n firefox\n",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "[]",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "http://whitelisted/",
-    fixedURI: "http://whitelisted/",
-    alternateURI: "http://www.whitelisted.com/",
-    affectedByWhitelist: true,
-    inWhitelist: true,
-  }];
+let testcases = [
+  ["http://www.mozilla.org", "http://www.mozilla.org/", null, false, false],
+  ["http://127.0.0.1/", "http://127.0.0.1/", null, false, false],
+  ["file:///foo/bar", "file:///foo/bar", null, false, false],
+  ["://www.mozilla.org", "http://www.mozilla.org/", null, false, true],
+  ["www.mozilla.org", "http://www.mozilla.org/", null, false, true],
+  ["http://mozilla/", "http://mozilla/", "http://www.mozilla.com/", false, false],
+  ["http://test./", "http://test./", "http://www.test./", false, false],
+  ["127.0.0.1", "http://127.0.0.1/", null, false, true],
+  ["1234", "http://1234/", "http://www.1234.com/", true, true],
+  ["host/foo.txt", "http://host/foo.txt", "http://www.host.com/foo.txt", false, true],
+  ["mozilla", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["test.", "http://test./", "http://www.test./", true, true],
+  [".test", "http://.test/", "http://www..test/", true, true],
+  ["mozilla is amazing", null, null, true, true],
+  ["mozilla ", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["   mozilla  ", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["mozilla \\", null, null, true, true],
+  ["mozilla \\ foo.txt", null, null, true, true],
+  ["mozilla \\\r foo.txt", null, null, true, true],
+  ["mozilla\n", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["mozilla \r\n", "http://mozilla/", "http://www.mozilla.com/", true, true],
+  ["moz\r\nfirefox\nos\r", "http://mozfirefoxos/", "http://www.mozfirefoxos.com/", true, true],
+  ["moz\r\n firefox\n", null, null, true, true],
+  ["", null, null, true, true],
+  ["[]", null, null, true, true]
+];
 
 if (Services.appinfo.OS.toLowerCase().startsWith("win")) {
-  testcases.push({
-    input: "C:\\some\\file.txt",
-    fixedURI: "file:///C:/some/file.txt",
-    protocolChange: true,
-  });
-  testcases.push({
-    input: "//mozilla",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    protocolChange: true,
-    affectedByWhitelist: true,
-  });
-  testcases.push({
-    input: "mozilla\\",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  });
+  testcases.push(["C:\\some\\file.txt", "file:///C:/some/file.txt", null, false, true]);
+  testcases.push(["//mozilla", "http://mozilla/", "http://www.mozilla.com/", false, true]);
+  testcases.push(["mozilla\\", "http://mozilla/", "http://www.mozilla.com/", true, true]);
 } else {
-  testcases.push({
-    input: "/some/file.txt",
-    fixedURI: "file:///some/file.txt",
-    protocolChange: true,
-  });
-  testcases.push({
-    input: "//mozilla",
-    fixedURI: "file:////mozilla",
-    protocolChange: true,
-  });
-  testcases.push({
-    input: "mozilla\\",
-    fixedURI: "http://mozilla\\/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-  });
+  testcases.push(["/some/file.txt", "file:///some/file.txt", null, false, true]);
+  testcases.push(["//mozilla", "file:////mozilla", null, false, true]);
+  testcases.push(["mozilla\\", "http://mozilla\\/", "http://www.mozilla/", true, true]);
 }
 
 function sanitize(input) {
   return input.replace(/\r|\n/g, "").trim();
 }
 
 function run_test() {
-  for (let { input: testInput,
-             fixedURI: expectedFixedURI,
-             alternateURI: alternativeURI,
-             keywordLookup: expectKeywordLookup,
-             protocolChange: expectProtocolChange,
-             affectedByWhitelist: affectedByWhitelist,
-             inWhitelist: inWhitelist } of testcases) {
-
-    // Explicitly force these into a boolean
-    expectKeywordLookup = !!expectKeywordLookup;
-    expectProtocolChange = !!expectProtocolChange;
-    affectedByWhitelist = !!affectedByWhitelist;
-    inWhitelist = !!inWhitelist;
-
+  for (let [testInput, expectedFixedURI, alternativeURI,
+            expectKeywordLookup, expectProtocolChange] of testcases) {
     for (let flags of flagInputs) {
       let info;
       let fixupURIOnly = null;
       try {
         fixupURIOnly = urifixup.createFixupURI(testInput, flags);
       } catch (ex) {
         do_print("Caught exception: " + ex);
         do_check_eq(expectedFixedURI, null);
@@ -267,22 +104,20 @@ 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);
 
       // 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);
+      do_check_eq(fixupURIOnly.spec, info.preferredURI.spec);
 
       let isFileURL = expectedFixedURI && expectedFixedURI.startsWith("file");
 
       // Check the fixedURI:
       let makeAlternativeURI = flags & urifixup.FIXUP_FLAGS_MAKE_ALTERNATE_URI;
       if (makeAlternativeURI && alternativeURI != null) {
         do_check_eq(info.fixedURI.spec, alternativeURI);
       } else {
@@ -291,36 +126,21 @@ function run_test() {
 
       // Check booleans on input:
       let couldDoKeywordLookup = flags & urifixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
       do_check_eq(info.fixupUsedKeyword, couldDoKeywordLookup && expectKeywordLookup);
       do_check_eq(info.fixupChangedProtocol, expectProtocolChange);
       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 (couldDoKeywordLookup && expectKeywordLookup) {
         let urlparamInput = encodeURIComponent(sanitize(testInput)).replace("%20", "+", "g");
         let searchURL = kSearchEngineURL.replace("{searchTerms}", urlparamInput);
         do_check_eq(info.preferredURI.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))
-          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);
       }
       do_check_eq(sanitize(testInput), info.originalInput);
     }
   }
 }