diff --git a/docshell/base/nsDefaultURIFixup.cpp b/docshell/base/nsDefaultURIFixup.cpp index 0061f3ea8863..f233c03d6210 100644 --- a/docshell/base/nsDefaultURIFixup.cpp +++ b/docshell/base/nsDefaultURIFixup.cpp @@ -386,14 +386,35 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup 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 (info->mFixedURI) - { - info->mPreferredURI = info->mFixedURI; - } - else if (sFixupKeywords && (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP)) - { + if (sFixupKeywords && (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP)) { rv = KeywordToURI(aStringURI, aPostData, getter_AddRefs(info->mPreferredURI)); if (NS_SUCCEEDED(rv) && info->mPreferredURI) { @@ -964,26 +985,17 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString, (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()) - { - // 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)) - { + !asciiHost.IsEmpty()) { + + if (IsDomainWhitelisted(asciiHost, dotLoc)) { 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, @@ -995,6 +1007,25 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString, } } +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) { diff --git a/docshell/base/nsDefaultURIFixup.h b/docshell/base/nsDefaultURIFixup.h index ab92e242bcd2..1088349ff595 100644 --- a/docshell/base/nsDefaultURIFixup.h +++ b/docshell/base/nsDefaultURIFixup.h @@ -37,6 +37,8 @@ private: 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 diff --git a/docshell/base/nsIURIFixup.idl b/docshell/base/nsIURIFixup.idl index 71a24d37a123..7384f8c99d83 100644 --- a/docshell/base/nsIURIFixup.idl +++ b/docshell/base/nsIURIFixup.idl @@ -63,7 +63,7 @@ interface nsIURIFixupInfo : nsISupports /** * Interface implemented by objects capable of fixing up strings into URIs */ -[scriptable, uuid(80d4932e-bb2e-4afb-98e0-de9cc9ea7d82)] +[scriptable, uuid(49298f2b-3630-4874-aecc-522300a7fead)] interface nsIURIFixup : nsISupports { /** No fixup flags. */ @@ -83,12 +83,18 @@ 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; - /* 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. diff --git a/docshell/test/unit/test_nsDefaultURIFixup_info.js b/docshell/test/unit/test_nsDefaultURIFixup_info.js index c63f5cb22078..fb1b7ddb67be 100644 --- a/docshell/test/unit/test_nsDefaultURIFixup_info.js +++ b/docshell/test/unit/test_nsDefaultURIFixup_info.js @@ -3,7 +3,8 @@ let urifixup = Cc["@mozilla.org/docshell/urifixup;1"]. Components.utils.import("resource://gre/modules/Services.jsm"); -let prefList = ["browser.fixup.typo.scheme", "keyword.enabled"]; +let prefList = ["browser.fixup.typo.scheme", "keyword.enabled", + "browser.fixup.domainwhitelist.whitelisted"]; for (let pref of prefList) { Services.prefs.setBoolPref(pref, true); } @@ -34,7 +35,8 @@ do_register_cleanup(function() { let flagInputs = [ urifixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP, urifixup.FIXUP_FLAGS_MAKE_ALTERNATE_URI, - urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS + urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS, + urifixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST, ]; flagInputs.concat([ @@ -44,42 +46,192 @@ flagInputs.concat([ flagInputs[0] | flagInputs[1] | flagInputs[2] ]); -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] -]; +/* + 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, + }]; if (Services.appinfo.OS.toLowerCase().startsWith("win")) { - 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]); + 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, + }); } else { - 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]); + 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/", + keywordLookup: true, + protocolChange: true, + affectedByWhitelist: true, + }); } function sanitize(input) { @@ -87,8 +239,20 @@ function sanitize(input) { } function run_test() { - for (let [testInput, expectedFixedURI, alternativeURI, - expectKeywordLookup, expectProtocolChange] of testcases) { + 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 flags of flagInputs) { let info; let fixupURIOnly = null; @@ -109,10 +273,12 @@ function run_test() { 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.spec, info.preferredURI.spec); + do_check_eq(!!fixupURIOnly, !!info.preferredURI); + if (fixupURIOnly) + do_check_eq(fixupURIOnly.spec, info.preferredURI.spec); let isFileURL = expectedFixedURI && expectedFixedURI.startsWith("file"); @@ -131,10 +297,25 @@ function run_test() { do_check_eq(info.fixupCreatedAlternateURI, makeAlternativeURI && alternativeURI != null); // Check the preferred URI - if (couldDoKeywordLookup && expectKeywordLookup) { + let requiresWhitelistedDomain = flags & urifixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST + if (couldDoKeywordLookup) { + if (expectKeywordLookup) { + if (!affectedByWhitelist || (affectedByWhitelist && !inWhitelist)) { 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: