From 7706868f9a97b0be63d6af30a6b14d7c81ec9882 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Tue, 22 Sep 2015 16:26:15 +0100 Subject: [PATCH] Bug 1199289 - r=valentin,dao --- .../test/general/browser_urlHighlight.js | 10 ++- .../test/general/browser_urlbarTrimURLs.js | 4 +- browser/base/content/urlbarBindings.xml | 58 +++++++++------ docshell/base/nsDefaultURIFixup.cpp | 40 ++++++++++- .../unit/test_nsDefaultURIFixup_search.js | 70 +++++++++++++++++-- 5 files changed, 152 insertions(+), 30 deletions(-) diff --git a/browser/base/content/test/general/browser_urlHighlight.js b/browser/base/content/test/general/browser_urlHighlight.js index a73ff43fcf1..725f04dc689 100644 --- a/browser/base/content/test/general/browser_urlHighlight.js +++ b/browser/base/content/test/general/browser_urlHighlight.js @@ -16,7 +16,8 @@ function testVal(aExpected) { 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() { @@ -45,6 +46,9 @@ function test() { testVal("mozilla.org"); testVal("mozilla.org"); testVal("mozilla.org"); + testVal("mozilla.com"); + testVal("mozilla.com"); + testVal("mozilla.com"); testVal("mozilla.org"); testVal("mozilla.org"); @@ -53,6 +57,8 @@ function test() { testVal("mozilla.org"); testVal("mozilla.org"); testVal("mozilla.org"); + testVal("mozilla.org"); + testVal("mozilla.org"); testVal("mozilla.org"); testVal("mozilla.org"); @@ -93,7 +99,7 @@ function test() { testVal("" + IP); testVal("" + IP + ""); testVal("" + IP + "<:666/file.ext>"); - testVal("" + IP + "<:666/file.ext>"); + testVal("" + IP + "<:666/file.ext>"); }); testVal("mailto:admin@mozilla.org"); diff --git a/browser/base/content/test/general/browser_urlbarTrimURLs.js b/browser/base/content/test/general/browser_urlbarTrimURLs.js index f3276022d24..781729ed0ad 100644 --- a/browser/base/content/test/general/browser_urlbarTrimURLs.js +++ b/browser/base/content/test/general/browser_urlbarTrimURLs.js @@ -42,14 +42,14 @@ function test() { 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/"); diff --git a/browser/base/content/urlbarBindings.xml b/browser/base/content/urlbarBindings.xml index ebd6dd3daf5..6e5f61b4b25 100644 --- a/browser/base/content/urlbarBindings.xml +++ b/browser/base/content/urlbarBindings.xml @@ -228,11 +228,32 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. let textNode = this.editor.rootElement.firstChild; let value = textNode.textContent; - - let protocol = value.match(/^[a-z\d.+\-]+:(?=[^\d])/); - if (protocol && - ["http:", "https:", "ftp:"].indexOf(protocol[0]) == -1) + if (!value) return; + + // 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; @@ -251,23 +272,20 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. 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); @@ -275,11 +293,11 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. 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); } ]]> diff --git a/docshell/base/nsDefaultURIFixup.cpp b/docshell/base/nsDefaultURIFixup.cpp index 23e5fd05656..648f0dbaa84 100644 --- a/docshell/base/nsDefaultURIFixup.cpp +++ b/docshell/base/nsDefaultURIFixup.cpp @@ -22,6 +22,7 @@ #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" @@ -124,6 +125,35 @@ nsDefaultURIFixup::CreateFixupURI(const nsACString& aStringURI, 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, @@ -327,7 +357,14 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, // 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; + } } } } @@ -749,6 +786,7 @@ nsDefaultURIFixup::FixupURIProtocol(const nsACString& aURIString, // 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("/:"); diff --git a/docshell/test/unit/test_nsDefaultURIFixup_search.js b/docshell/test/unit/test_nsDefaultURIFixup_search.js index 6c95ec71627..acf35ad9434 100644 --- a/docshell/test/unit/test_nsDefaultURIFixup_search.js +++ b/docshell/test/unit/test_nsDefaultURIFixup_search.js @@ -1,6 +1,7 @@ -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); @@ -9,10 +10,10 @@ 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() { @@ -26,7 +27,9 @@ do_register_cleanup(function() { 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', @@ -37,14 +40,71 @@ var data = [ 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) {