Bug 1199289 - r=valentin,dao, a=sylvestre
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 22 Sep 2015 16:26:15 +0100
changeset 296127 9034d20bf95da1d0cd25e00e90f5d4b9640213c5
parent 296126 a363a3510295ceaa61afb325d4e257b669dd83f2
child 296128 6ea3a333b3883d06ad496b0f6d4b10a57aa932e9
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin, dao, sylvestre
bugs1199289
milestone43.0a2
Bug 1199289 - r=valentin,dao, a=sylvestre
browser/base/content/test/general/browser_urlHighlight.js
browser/base/content/test/general/browser_urlbarTrimURLs.js
browser/base/content/urlbarBindings.xml
docshell/base/nsDefaultURIFixup.cpp
docshell/test/unit/test_nsDefaultURIFixup_search.js
--- a/browser/base/content/test/general/browser_urlHighlight.js
+++ b/browser/base/content/test/general/browser_urlHighlight.js
@@ -11,17 +11,18 @@ function testVal(aExpected) {
   let result = "";
   for (let i = 0; i < selection.rangeCount; i++) {
     let range = selection.getRangeAt(i).toString();
     let pos = value.indexOf(range);
     result += value.substring(0, pos) + "<" + range + ">";
     value = value.substring(pos + range.length);
   }
   result += value;
-  is(result, aExpected, "Correct part of the urlbar contents is highlighted");
+  is(result, aExpected,
+     "Correct part of the urlbar contents is highlighted");
 }
 
 function test() {
   const prefname = "browser.urlbar.formatting.enabled";
 
   registerCleanupFunction(function () {
     Services.prefs.clearUserPref(prefname);
     URLBarSetURI();
@@ -40,24 +41,29 @@ function test() {
   testVal("<https://>mozilla.imaginatory");
 
   testVal("<https://www.>mozilla.org");
   testVal("<https://sub.>mozilla.org");
   testVal("<https://sub1.sub2.sub3.>mozilla.org");
   testVal("<www.>mozilla.org");
   testVal("<sub.>mozilla.org");
   testVal("<sub1.sub2.sub3.>mozilla.org");
+  testVal("<mozilla.com.>mozilla.com");
+  testVal("<https://mozilla.com:mozilla.com@>mozilla.com");
+  testVal("<mozilla.com:mozilla.com@>mozilla.com");
 
   testVal("<http://ftp.>mozilla.org");
   testVal("<ftp://ftp.>mozilla.org");
 
   testVal("<https://sub.>mozilla.org");
   testVal("<https://sub1.sub2.sub3.>mozilla.org");
   testVal("<https://user:pass@sub1.sub2.sub3.>mozilla.org");
   testVal("<https://user:pass@>mozilla.org");
+  testVal("<user:pass@sub1.sub2.sub3.>mozilla.org");
+  testVal("<user:pass@>mozilla.org");
 
   testVal("<https://>mozilla.org</file.ext>");
   testVal("<https://>mozilla.org</sub/file.ext>");
   testVal("<https://>mozilla.org</sub/file.ext?foo>");
   testVal("<https://>mozilla.org</sub/file.ext?foo&bar>");
   testVal("<https://>mozilla.org</sub/file.ext?foo&bar#top>");
   testVal("<https://>mozilla.org</sub/file.ext?foo&bar#top>");
 
@@ -88,17 +94,17 @@ function test() {
              "[1:2:3:4:5:6:255.255.255.255]"];
   IPs.forEach(function (IP) {
     testVal(IP);
     testVal(IP + "</file.ext>");
     testVal(IP + "<:666/file.ext>");
     testVal("<https://>" + IP);
     testVal("<https://>" + IP + "</file.ext>");
     testVal("<https://user:pass@>" + IP + "<:666/file.ext>");
-    testVal("<http://user:pass@>" + IP + "<:666/file.ext>");
+    testVal("<user:pass@>" + IP + "<:666/file.ext>");
   });
 
   testVal("mailto:admin@mozilla.org");
   testVal("gopher://mozilla.org/");
   testVal("about:config");
   testVal("jar:http://mozilla.org/example.jar!/");
   testVal("view-source:http://mozilla.org/");
   testVal("foo9://mozilla.org/");
--- a/browser/base/content/test/general/browser_urlbarTrimURLs.js
+++ b/browser/base/content/test/general/browser_urlbarTrimURLs.js
@@ -37,24 +37,24 @@ function test() {
   testVal("http://ftpx.mozilla.org/", "ftpx.mozilla.org");
   testVal("ftp://ftp.mozilla.org/", "ftp://ftp.mozilla.org");
   testVal("ftp://ftp1.mozilla.org/", "ftp://ftp1.mozilla.org");
   testVal("ftp://ftp42.mozilla.org/", "ftp://ftp42.mozilla.org");
   testVal("ftp://ftpx.mozilla.org/", "ftp://ftpx.mozilla.org");
 
   testVal("https://user:pass@mozilla.org/", "https://user:pass@mozilla.org");
   testVal("https://user@mozilla.org/", "https://user@mozilla.org");
-  testVal("http://user:pass@mozilla.org/", "http://user:pass@mozilla.org");
+  testVal("http://user:pass@mozilla.org/", "user:pass@mozilla.org");
   testVal("http://user@mozilla.org/", "user@mozilla.org");
   testVal("http://sub.mozilla.org:666/", "sub.mozilla.org:666");
 
   testVal("https://[fe80::222:19ff:fe11:8c76]/file.ext");
   testVal("http://[fe80::222:19ff:fe11:8c76]/", "[fe80::222:19ff:fe11:8c76]");
   testVal("https://user:pass@[fe80::222:19ff:fe11:8c76]:666/file.ext");
-  testVal("http://user:pass@[fe80::222:19ff:fe11:8c76]:666/file.ext");
+  testVal("http://user:pass@[fe80::222:19ff:fe11:8c76]:666/file.ext", "user:pass@[fe80::222:19ff:fe11:8c76]:666/file.ext");
 
   testVal("mailto:admin@mozilla.org");
   testVal("gopher://mozilla.org/");
   testVal("about:config");
   testVal("jar:http://mozilla.org/example.jar!/");
   testVal("view-source:http://mozilla.org/");
 
   // Behaviour for hosts with no dots depends on the whitelist:
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -223,21 +223,42 @@ file, You can obtain one at http://mozil
           let selection = controller.getSelection(controller.SELECTION_URLSECONDARY);
           selection.removeAllRanges();
 
           if (this.focused)
             return;
 
           let textNode = this.editor.rootElement.firstChild;
           let value = textNode.textContent;
+          if (!value)
+            return;
 
-          let protocol = value.match(/^[a-z\d.+\-]+:(?=[^\d])/);
-          if (protocol &&
-              ["http:", "https:", "ftp:"].indexOf(protocol[0]) == -1)
+          // Get the URL from the fixup service:
+          let flags = Services.uriFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS |
+                      Services.uriFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
+          let uriInfo = Services.uriFixup.getFixupURIInfo(value, flags);
+          // Ignore if we couldn't make a URI out of this, the URI resulted in a search,
+          // or the URI has a non-http(s)/ftp protocol.
+          if (!uriInfo.fixedURI ||
+              uriInfo.keywordProviderName ||
+              ["http", "https", "ftp"].indexOf(uriInfo.fixedURI.scheme) == -1) {
             return;
+          }
+
+          // If we trimmed off the http scheme, ensure we stick it back on before
+          // trying to figure out what domain we're accessing, so we don't get
+          // confused by user:pass@host http URLs. We later use
+          // trimmedLength to ensure we don't count the length of a trimmed protocol
+          // when determining which parts of the URL to highlight as "preDomain".
+          let trimmedLength = 0;
+          if (uriInfo.fixedURI.scheme == "http" && !value.startsWith("http://")) {
+            value = "http://" + value;
+            trimmedLength = "http://".length;
+          }
+
           let matchedURL = value.match(/^((?:[a-z]+:\/\/)?(?:[^\/]+@)?)(.+?)(?::\d+)?(?:\/|$)/);
           if (!matchedURL)
             return;
 
           // Strike out the "https" part if mixed active content is loaded.
           if (this.getAttribute("pageproxystate") == "valid" &&
               value.startsWith("https:") &&
               gBrowser.securityUI.state &
@@ -246,45 +267,42 @@ file, You can obtain one at http://mozil
             range.setStart(textNode, 0);
             range.setEnd(textNode, 5);
             strikeOut.addRange(range);
           }
 
           let [, preDomain, domain] = matchedURL;
           let baseDomain = domain;
           let subDomain = "";
-          // getBaseDomainFromHost doesn't recognize IPv6 literals in brackets as IPs (bug 667159)
-          if (domain[0] != "[") {
-            try {
-              baseDomain = Services.eTLD.getBaseDomainFromHost(domain);
-              if (!domain.endsWith(baseDomain)) {
-                // getBaseDomainFromHost converts its resultant to ACE.
-                let IDNService = Cc["@mozilla.org/network/idn-service;1"]
-                                 .getService(Ci.nsIIDNService);
-                baseDomain = IDNService.convertACEtoUTF8(baseDomain);
-              }
-            } catch (e) {}
-          }
+          try {
+            baseDomain = Services.eTLD.getBaseDomainFromHost(uriInfo.fixedURI.host);
+            if (!domain.endsWith(baseDomain)) {
+              // getBaseDomainFromHost converts its resultant to ACE.
+              let IDNService = Cc["@mozilla.org/network/idn-service;1"]
+                               .getService(Ci.nsIIDNService);
+              baseDomain = IDNService.convertACEtoUTF8(baseDomain);
+            }
+          } catch (e) {}
           if (baseDomain != domain) {
             subDomain = domain.slice(0, -baseDomain.length);
           }
 
-          let rangeLength = preDomain.length + subDomain.length;
+          let rangeLength = preDomain.length + subDomain.length - trimmedLength;
           if (rangeLength) {
             let range = document.createRange();
             range.setStart(textNode, 0);
             range.setEnd(textNode, rangeLength);
             selection.addRange(range);
           }
 
-          let startRest = preDomain.length + domain.length;
-          if (startRest < value.length) {
+          let startRest = preDomain.length + domain.length - trimmedLength;
+          if (startRest < value.length - trimmedLength) {
             let range = document.createRange();
             range.setStart(textNode, startRest);
-            range.setEnd(textNode, value.length);
+            range.setEnd(textNode, value.length - trimmedLength);
             selection.addRange(range);
           }
         ]]></body>
       </method>
 
       <method name="handleRevert">
         <body><![CDATA[
           var isScrolling = this.popupOpen;
--- a/docshell/base/nsDefaultURIFixup.cpp
+++ b/docshell/base/nsDefaultURIFixup.cpp
@@ -17,16 +17,17 @@
 #endif
 
 #include "nsIURIFixup.h"
 #include "nsDefaultURIFixup.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/ContentChild.h"
 #include "mozilla/ipc/InputStreamUtils.h"
 #include "mozilla/ipc/URIUtils.h"
+#include "mozilla/Tokenizer.h"
 #include "nsIObserverService.h"
 #include "nsXULAppAPI.h"
 
 // Used to check if external protocol schemes are usable
 #include "nsCExternalHandlerService.h"
 #include "nsIExternalProtocolService.h"
 
 using namespace mozilla;
@@ -119,16 +120,45 @@ nsDefaultURIFixup::CreateFixupURI(const 
   nsresult rv = GetFixupURIInfo(aStringURI, aFixupFlags, aPostData,
                                 getter_AddRefs(fixupInfo));
   NS_ENSURE_SUCCESS(rv, rv);
 
   fixupInfo->GetPreferredURI(aURI);
   return rv;
 }
 
+// Returns true if the URL contains a user:password@ or user@
+static bool
+HasUserPassword(const nsACString& aStringURI)
+{
+  mozilla::Tokenizer parser(aStringURI);
+  mozilla::Tokenizer::Token token;
+
+  // May start with any of "protocol:", "protocol://",  "//", "://"
+  if (parser.Check(Tokenizer::TOKEN_WORD, token)) { // Skip protocol if any
+  }
+  if (parser.CheckChar(':')) { // Skip colon if found
+  }
+  while (parser.CheckChar('/')) { // Skip all of the following slashes
+  }
+
+  while (parser.Next(token)) {
+    if (token.Type() == Tokenizer::TOKEN_CHAR) {
+      if (token.AsChar() == '/') {
+        return false;
+      }
+      if (token.AsChar() == '@') {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 NS_IMETHODIMP
 nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI,
                                    uint32_t aFixupFlags,
                                    nsIInputStream** aPostData,
                                    nsIURIFixupInfo** aInfo)
 {
   NS_ENSURE_ARG(!aStringURI.IsEmpty());
 
@@ -322,17 +352,24 @@ nsDefaultURIFixup::GetFixupURIInfo(const
       if (NS_FAILED(rv)) {
         return rv;
       }
       // This basically means we're dealing with a theoretically valid
       // URI... but we have no idea how to load it. (e.g. "christmas:humbug")
       // It's more likely the user wants to search, and so we
       // chuck this over to their preferred search provider instead:
       if (!handlerExists) {
-        TryKeywordFixupForURIInfo(uriString, info, aPostData);
+        bool hasUserPassword = HasUserPassword(uriString);
+        if (!hasUserPassword) {
+          TryKeywordFixupForURIInfo(uriString, info, aPostData);
+        } else {
+          // If the given URL has a user:password we can't just pass it to the
+          // external protocol handler; we'll try using it with http instead later
+          info->mFixedURI = nullptr;
+        }
       }
     }
   }
 
   if (info->mFixedURI) {
     if (!info->mPreferredURI) {
       if (aFixupFlags & FIXUP_FLAGS_MAKE_ALTERNATE_URI) {
         info->mFixupCreatedAlternateURI = MakeAlternateURI(info->mFixedURI);
@@ -744,16 +781,17 @@ nsDefaultURIFixup::FixupURIProtocol(cons
   // Add ftp:// or http:// to front of url if it has no spec
   //
   // Should fix:
   //
   //   no-scheme.com
   //   ftp.no-scheme.com
   //   ftp4.no-scheme.com
   //   no-scheme.com/query?foo=http://www.foo.com
+  //   user:pass@no-scheme.com
   //
   int32_t schemeDelim = uriString.Find("://", 0);
   int32_t firstDelim = uriString.FindCharInSet("/:");
   if (schemeDelim <= 0 ||
       (firstDelim != -1 && schemeDelim > firstDelim)) {
     // find host name
     int32_t hostPos = uriString.FindCharInSet("/:?#");
     if (hostPos == -1) {
--- a/docshell/test/unit/test_nsDefaultURIFixup_search.js
+++ b/docshell/test/unit/test_nsDefaultURIFixup_search.js
@@ -1,55 +1,115 @@
-var urifixup = Cc["@mozilla.org/docshell/urifixup;1"].
+let urifixup = Cc["@mozilla.org/docshell/urifixup;1"].
                getService(Ci.nsIURIFixup);
 Components.utils.import("resource://gre/modules/Services.jsm");
+Components.utils.import("resource://gre/modules/AppConstants.jsm");
 
 Services.prefs.setBoolPref("keyword.enabled", true);
 
 const kSearchEngineID = "test_urifixup_search_engine";
 const kSearchEngineURL = "http://www.example.org/?search={searchTerms}";
 Services.search.addEngineWithDetails(kSearchEngineID, "", "", "", "get",
                                      kSearchEngineURL);
 
-var oldDefaultEngine = Services.search.defaultEngine;
+let oldDefaultEngine = Services.search.defaultEngine;
 Services.search.defaultEngine = Services.search.getEngineByName(kSearchEngineID);
 
-var selectedName = Services.search.defaultEngine.name;
+let selectedName = Services.search.defaultEngine.name;
 do_check_eq(selectedName, kSearchEngineID);
 
 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");
 });
 
-var data = [
+let isWin = AppConstants.platform == "win";
+
+let data = [
   {
     // Valid should not be changed.
     wrong: 'https://example.com/this/is/a/test.html',
     fixed: 'https://example.com/this/is/a/test.html',
   },
   {
     // Unrecognized protocols should be changed.
     wrong: 'whatever://this/is/a/test.html',
     fixed: kSearchEngineURL.replace("{searchTerms}", encodeURIComponent('whatever://this/is/a/test.html')),
   },
+
+  // The following tests check that when a user:password is present in the URL
+  // `user:` isn't treated as an unknown protocol thus leaking the user and
+  // password to the search engine.
+  {
+    wrong: 'user:pass@example.com/this/is/a/test.html',
+    fixed: 'http://user:pass@example.com/this/is/a/test.html',
+  },
+  {
+    wrong: 'user@example.com:8080/this/is/a/test.html',
+    fixed: 'http://user@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: 'https:pass@example.com/this/is/a/test.html',
+    fixed: 'https://pass@example.com/this/is/a/test.html',
+  },
+  {
+    wrong: 'user:pass@example.com:8080/this/is/a/test.html',
+    fixed: 'http://user:pass@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: 'http:user:pass@example.com:8080/this/is/a/test.html',
+    fixed: 'http://user:pass@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: 'ttp:user:pass@example.com:8080/this/is/a/test.html',
+    fixed: 'http://user:pass@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: 'gobbledygook:user:pass@example.com:8080/this/is/a/test.html',
+    fixed: 'http://gobbledygook:user%3Apass@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: 'user:@example.com:8080/this/is/a/test.html',
+    fixed: 'http://user:@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: '//user:pass@example.com:8080/this/is/a/test.html',
+    fixed: (isWin ? "http:" : "file://") + '//user:pass@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: '://user:pass@example.com:8080/this/is/a/test.html',
+    fixed: 'http://user:pass@example.com:8080/this/is/a/test.html',
+  },
+  {
+    wrong: 'whatever://this/is/a@b/test.html',
+    fixed: kSearchEngineURL.replace("{searchTerms}", encodeURIComponent('whatever://this/is/a@b/test.html')),
+  },
 ];
 
+let extProtocolSvc = Cc["@mozilla.org/uriloader/external-protocol-service;1"]
+                     .getService(Ci.nsIExternalProtocolService);
+
+if (extProtocolSvc && extProtocolSvc.externalProtocolHandlerExists("mailto")) {
+  data.push({
+    wrong: "mailto:foo@bar.com",
+    fixed: "mailto:foo@bar.com"
+  });
+}
 
 function run_test() {
   run_next_test();
 }
 
-var len = data.length;
+let len = data.length;
 // Make sure we fix what needs fixing
 add_task(function test_fix_unknown_schemes() {
   for (let i = 0; i < len; ++i) {
     let item = data[i];
     let result =
       urifixup.createFixupURI(item.wrong,
                               urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS).spec;
     do_check_eq(result, item.fixed);