Bug 1042519 - test. should result in a keyword lookup instead of an error page, r=bz,mak

Also taking the opportunity to rework the nsIURIFixupInfo interface to use more sensible booleans, and the relevant test to specify outcomes directly, instead of trying to use the same logic as the implementation to infer them from the input.

--HG--
extra : rebase_source : 43b54b9ff412c05dc3fd2575a80919fc085d2d49
This commit is contained in:
Gijs Kruitbosch 2014-07-25 11:46:07 +01:00
parent 2a83f254b9
commit a1fcbf0cd2
6 changed files with 119 additions and 106 deletions

View File

@ -795,6 +795,13 @@ function gKeywordURIFixup(fixupInfo, topic, data) {
let hostName = alternativeURI.host;
// and the ascii-only host for the pref:
let asciiHost = alternativeURI.asciiHost;
// Normalize out a single trailing dot - NB: not using endsWith/lastIndexOf
// because we need to be sure this last dot is the *only* dot, too.
// More generally, this is used for the pref and should stay in sync with
// the code in nsDefaultURIFixup::KeywordURIFixup .
if (asciiHost.indexOf('.') == asciiHost.length - 1) {
asciiHost = asciiHost.slice(0, -1);
}
let onLookupComplete = (request, record, status) => {
let browser = weakBrowser.get();

View File

@ -48,29 +48,34 @@ add_task(function* test_navigate_full_domain() {
gBrowser.removeTab(tab);
});
add_task(function* test_navigate_single_host() {
Services.prefs.setBoolPref("browser.fixup.domainwhitelist.localhost", false);
let tab = gBrowser.selectedTab = gBrowser.addTab();
yield* runURLBarSearchTest("localhost", true, true);
function get_test_function_for_localhost_with_hostname(hostName) {
return function* test_navigate_single_host() {
const pref = "browser.fixup.domainwhitelist.localhost";
Services.prefs.setBoolPref(pref, false);
let tab = gBrowser.selectedTab = gBrowser.addTab();
yield* runURLBarSearchTest(hostName, true, true);
let notificationBox = gBrowser.getNotificationBox(tab.linkedBrowser);
let notification = notificationBox.getNotificationWithValue("keyword-uri-fixup");
let docLoadPromise = waitForDocLoadAndStopIt("http://localhost/");
notification.querySelector(".notification-button-default").click();
let notificationBox = gBrowser.getNotificationBox(tab.linkedBrowser);
let notification = notificationBox.getNotificationWithValue("keyword-uri-fixup");
let docLoadPromise = waitForDocLoadAndStopIt("http://" + hostName + "/");
notification.querySelector(".notification-button-default").click();
// check pref value
let pref = "browser.fixup.domainwhitelist.localhost";
let prefValue = Services.prefs.getBoolPref(pref);
ok(prefValue, "Pref should have been toggled");
// check pref value
let prefValue = Services.prefs.getBoolPref(pref);
ok(prefValue, "Pref should have been toggled");
yield docLoadPromise;
gBrowser.removeTab(tab);
yield docLoadPromise;
gBrowser.removeTab(tab);
// Now try again with the pref set.
let tab = gBrowser.selectedTab = gBrowser.addTab();
yield* runURLBarSearchTest("localhost", false, false);
gBrowser.removeTab(tab);
});
// Now try again with the pref set.
let tab = gBrowser.selectedTab = gBrowser.addTab();
yield* runURLBarSearchTest(hostName, false, false);
gBrowser.removeTab(tab);
}
}
add_task(get_test_function_for_localhost_with_hostname("localhost"));
add_task(get_test_function_for_localhost_with_hostname("localhost."));
add_task(function* test_navigate_invalid_url() {
let tab = gBrowser.selectedTab = gBrowser.addTab();

View File

@ -185,10 +185,13 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
// Check for if it is a file URL
nsCOMPtr<nsIURI> uri;
FileURIFixup(uriString, getter_AddRefs(uri));
// NB: FileURIFixup only returns a URI if it had to fix the protocol to
// do so, so passing in file:///foo/bar will not hit this path:
if (uri)
{
uri.swap(info->mFixedURI);
info->mPreferredURI = info->mFixedURI;
info->mFixupChangedProtocol = true;
return NS_OK;
}
@ -245,8 +248,6 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
sInitializedPrefCaches = true;
}
info->mInputHasProtocol = !scheme.IsEmpty();
// Fix up common scheme typos.
if (sFixTypos && (aFixupFlags & FIXUP_FLAG_FIX_SCHEME_TYPOS)) {
@ -261,26 +262,32 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
// ttp -> http.
uriString.Replace(0, 3, "http");
scheme.AssignLiteral("http");
info->mFixupChangedProtocol = true;
} else if (scheme.LowerCaseEqualsLiteral("ttps")) {
// ttps -> https.
uriString.Replace(0, 4, "https");
scheme.AssignLiteral("https");
info->mFixupChangedProtocol = true;
} else if (scheme.LowerCaseEqualsLiteral("tps")) {
// tps -> https.
uriString.Replace(0, 3, "https");
scheme.AssignLiteral("https");
info->mFixupChangedProtocol = true;
} else if (scheme.LowerCaseEqualsLiteral("ps")) {
// ps -> https.
uriString.Replace(0, 2, "https");
scheme.AssignLiteral("https");
info->mFixupChangedProtocol = true;
} else if (scheme.LowerCaseEqualsLiteral("ile")) {
// ile -> file.
uriString.Replace(0, 3, "file");
scheme.AssignLiteral("file");
info->mFixupChangedProtocol = true;
} else if (scheme.LowerCaseEqualsLiteral("le")) {
// le -> file.
uriString.Replace(0, 2, "file");
scheme.AssignLiteral("file");
info->mFixupChangedProtocol = true;
}
}
@ -297,10 +304,6 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
rv = NS_NewURI(getter_AddRefs(uri), uriString, nullptr);
if (NS_SUCCEEDED(rv)) {
info->mFixedURI = uri;
// Figure out whether this had a domain or just a single hostname:
nsAutoCString host;
uri->GetHost(host);
info->mInputHostHasDot = host.FindChar('.') != kNotFound;
}
if (!uri && rv != NS_ERROR_MALFORMED_URI) {
@ -333,7 +336,7 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
if (uri) {
if (aFixupFlags & FIXUP_FLAGS_MAKE_ALTERNATE_URI)
MakeAlternateURI(uri);
info->mFixupCreatedAlternateURI = MakeAlternateURI(uri);
info->mPreferredURI = uri;
return NS_OK;
}
@ -344,12 +347,9 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
// NB: this rv gets returned at the end of this method if we never
// do a keyword fixup after this (because the pref or the flags passed
// might not let us).
rv = FixupURIProtocol(uriString, getter_AddRefs(uriWithProtocol));
rv = FixupURIProtocol(uriString, info, getter_AddRefs(uriWithProtocol));
if (uriWithProtocol) {
info->mFixedURI = uriWithProtocol;
nsAutoCString host;
uriWithProtocol->GetHost(host);
info->mInputHostHasDot = host.FindChar('.') != kNotFound;
}
// See if it is a keyword
@ -364,7 +364,7 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
// If so, attempt to fixup http://foo into http://www.foo.com
if (info->mFixedURI && aFixupFlags & FIXUP_FLAGS_MAKE_ALTERNATE_URI) {
MakeAlternateURI(info->mFixedURI);
info->mFixupCreatedAlternateURI = MakeAlternateURI(info->mFixedURI);
}
// If we still haven't been able to construct a valid URI, try to force a
@ -625,7 +625,7 @@ bool nsDefaultURIFixup::IsLikelyFTP(const nsCString &aHostSpec)
return likelyFTP;
}
nsresult nsDefaultURIFixup::FileURIFixup(const nsACString& aStringURI,
nsresult nsDefaultURIFixup::FileURIFixup(const nsACString& aStringURI,
nsIURI** aURI)
{
nsAutoCString uriSpecOut;
@ -722,7 +722,8 @@ nsresult nsDefaultURIFixup::ConvertFileToStringURI(const nsACString& aIn,
nsresult
nsDefaultURIFixup::FixupURIProtocol(const nsACString & aURIString,
nsIURI** aURI)
nsDefaultURIFixupInfo* aFixupInfo,
nsIURI** aURI)
{
nsAutoCString uriString(aURIString);
*aURI = nullptr;
@ -768,6 +769,7 @@ nsDefaultURIFixup::FixupURIProtocol(const nsACString & aURIString,
uriString.InsertLiteral("ftp://", 0);
else
uriString.InsertLiteral("http://", 0);
aFixupInfo->mFixupChangedProtocol = true;
} // end if checkprotocol
return NS_NewURI(aURI, uriString, nullptr);
@ -907,6 +909,7 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
// "?site:mozilla.org docshell"
// Things that have a quote before the first dot/colon
// "mozilla" - checked against a whitelist to see if it's a host or not
// ".mozilla", "mozilla." - ditto
// These are not keyword formatted strings
// "www.blah.com" - first space-separated substring contains a dot, doesn't start with "?"
@ -916,12 +919,15 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
// "nonQualifiedHost?"
// "nonQualifiedHost?args"
// "nonQualifiedHost?some args"
// "blah.com."
// Note: uint32_t(kNotFound) is greater than any actual location
// in practice. So if we cast all locations to uint32_t, then a <
// b guarantees that either b is kNotFound and a is found, or both
// are found and a found before b.
uint32_t dotLoc = uint32_t(aURIString.FindChar('.'));
nsAutoCString tmpURIString(aURIString);
uint32_t lastDotLoc = uint32_t(tmpURIString.RFindChar('.'));
uint32_t colonLoc = uint32_t(aURIString.FindChar(':'));
uint32_t spaceLoc = uint32_t(aURIString.FindChar(' '));
if (spaceLoc == 0) {
@ -933,6 +939,8 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
uint32_t(aURIString.FindChar('\'')));
nsresult rv;
// We do keyword lookups if a space or quote preceded the dot, colon
// or question mark (or if the latter were not found):
if (((spaceLoc < dotLoc || quoteLoc < dotLoc) &&
(spaceLoc < colonLoc || quoteLoc < colonLoc) &&
(spaceLoc < qMarkLoc || quoteLoc < qMarkLoc)) ||
@ -945,8 +953,11 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
aFixupInfo->mFixupUsedKeyword = true;
}
}
else if (dotLoc == uint32_t(kNotFound) && colonLoc == uint32_t(kNotFound) &&
qMarkLoc == uint32_t(kNotFound))
// ... 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 (NS_SUCCEEDED(aFixupInfo->mFixedURI->GetAsciiHost(asciiHost)) &&
@ -954,8 +965,14 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
{
// 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.");
pref.Append(asciiHost);
if (dotLoc == aURIString.Length() - 1) {
pref.Append(Substring(asciiHost, 0, asciiHost.Length() - 1));
} else {
pref.Append(asciiHost);
}
if (Preferences::GetBool(pref.get(), false))
{
return;
@ -972,6 +989,7 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
}
}
nsresult NS_NewURIFixup(nsIURIFixup **aURIFixup)
{
nsDefaultURIFixup *fixup = new nsDefaultURIFixup;
@ -988,8 +1006,8 @@ NS_IMPL_ISUPPORTS(nsDefaultURIFixupInfo, nsIURIFixupInfo)
nsDefaultURIFixupInfo::nsDefaultURIFixupInfo(const nsACString& aOriginalInput):
mFixupUsedKeyword(false),
mInputHasProtocol(false),
mInputHostHasDot(false)
mFixupChangedProtocol(false),
mFixupCreatedAlternateURI(false)
{
mOriginalInput = aOriginalInput;
}
@ -1038,16 +1056,16 @@ nsDefaultURIFixupInfo::GetFixupUsedKeyword(bool* aOut)
}
NS_IMETHODIMP
nsDefaultURIFixupInfo::GetInputHasProtocol(bool* aOut)
nsDefaultURIFixupInfo::GetFixupChangedProtocol(bool* aOut)
{
*aOut = mInputHasProtocol;
*aOut = mFixupChangedProtocol;
return NS_OK;
}
NS_IMETHODIMP
nsDefaultURIFixupInfo::GetInputHostHasDot(bool* aOut)
nsDefaultURIFixupInfo::GetFixupCreatedAlternateURI(bool* aOut)
{
*aOut = mInputHostHasDot;
*aOut = mFixupCreatedAlternateURI;
return NS_OK;
}

View File

@ -27,7 +27,9 @@ private:
/* additional members */
nsresult FileURIFixup(const nsACString &aStringURI, nsIURI** aURI);
nsresult ConvertFileToStringURI(const nsACString& aIn, nsCString& aOut);
nsresult FixupURIProtocol(const nsACString& aIn, nsIURI** aURI);
nsresult FixupURIProtocol(const nsACString& aIn,
nsDefaultURIFixupInfo* aFixupInfo,
nsIURI** aURI);
void KeywordURIFixup(const nsACString &aStringURI,
nsDefaultURIFixupInfo* aFixupInfo,
nsIInputStream** aPostData);
@ -55,8 +57,8 @@ private:
nsCOMPtr<nsIURI> mPreferredURI;
nsCOMPtr<nsIURI> mFixedURI;
bool mFixupUsedKeyword;
bool mInputHasProtocol;
bool mInputHostHasDot;
bool mFixupChangedProtocol;
bool mFixupCreatedAlternateURI;
nsAutoCString mOriginalInput;
};
#endif

View File

@ -12,7 +12,7 @@ interface nsIInputStream;
/**
* Interface indicating what we found/corrected when fixing up a URI
*/
[scriptable, uuid(c9b6cc32-c24e-4283-adaa-9290577fd609)]
[scriptable, uuid(62aac1e0-3da8-4920-bd1b-a54fc2e2eb24)]
interface nsIURIFixupInfo : nsISupports
{
/**
@ -41,17 +41,17 @@ interface nsIURIFixupInfo : nsISupports
readonly attribute boolean fixupUsedKeyword;
/**
* Whether we think there was a protocol specified in some way,
* even if we corrected it (e.g. "ttp://foo.com/bar")
* Whether we changed the protocol instead of using one from the input as-is.
*/
readonly attribute boolean inputHasProtocol;
readonly attribute boolean fixupChangedProtocol;
/**
* Whether the input included a dot in the hostname, e.g. "mozilla.org"
* rather than just "mozilla". This makes a difference in terms of when we
* decide to do a keyword search or not.
* Whether we created an alternative URI. We might have added a prefix and/or
* suffix, the contents of which are controlled by the
* browser.fixup.alternate.prefix and .suffix prefs, with the defaults being
* "www." and ".com", respectively.
*/
readonly attribute boolean inputHostHasDot;
readonly attribute boolean fixupCreatedAlternateURI;
/**
* The original input

View File

@ -45,32 +45,39 @@ flagInputs.concat([
]);
let testcases = [
["http://www.mozilla.org", "http://www.mozilla.org/"],
["://www.mozilla.org", "http://www.mozilla.org/"],
["www.mozilla.org", "http://www.mozilla.org/"],
["http://mozilla/", "http://mozilla/"],
["127.0.0.1", "http://127.0.0.1/"],
["1234", "http://1234/"],
["host/foo.txt", "http://host/foo.txt"],
["mozilla", "http://mozilla/"],
["mozilla is amazing", null],
["", null],
["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],
["", null, null, true, true]
];
if (Services.appinfo.OS.toLowerCase().startsWith("win")) {
testcases.push(["C:\\some\\file.txt", "file:///C:/some/file.txt"]);
testcases.push(["C:\\some\\file.txt", "file:///C:/some/file.txt", null, false, true]);
} else {
testcases.push(["/some/file.txt", "file:///some/file.txt"]);
testcases.push(["/some/file.txt", "file:///some/file.txt", null, false, true]);
}
function run_test() {
for (let [testInput, expectedFixedURI] of testcases) {
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);
}
@ -78,68 +85,42 @@ function run_test() {
info = urifixup.getFixupURIInfo(testInput, flags);
} 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);
// Both APIs should then also be using the same spec.
do_check_eq(fixupURIOnly.spec, info.preferredURI.spec);
let isFileURL = expectedFixedURI && expectedFixedURI.startsWith("file");
// Check the fixedURI:
let alternateURI = flags & urifixup.FIXUP_FLAGS_MAKE_ALTERNATE_URI;
if (!isFileURL && alternateURI && !info.inputHostHasDot && info.fixedURI) {
let originalURI = Services.io.newURI(expectedFixedURI, null, null);
do_check_eq(info.fixedURI.host, "www." + originalURI.host + ".com");
let makeAlternativeURI = flags & urifixup.FIXUP_FLAGS_MAKE_ALTERNATE_URI;
if (makeAlternativeURI && alternativeURI != null) {
do_check_eq(info.fixedURI.spec, alternativeURI);
} else {
do_check_eq(info.fixedURI && info.fixedURI.spec, expectedFixedURI);
}
// Check booleans on input:
if (isFileURL) {
do_check_eq(info.inputHasProtocol, testInput.startsWith("file:"));
do_check_eq(info.inputHostHasDot, false);
} else {
// The duff protocol doesn't count, so > 0 rather than -1:
do_check_eq(info.inputHasProtocol, testInput.indexOf(":") > 0);
let dotIndex = testInput.indexOf(".");
let slashIndex = testInput.replace("://", "").indexOf("/");
slashIndex = slashIndex == -1 ? testInput.length : slashIndex;
do_check_eq(info.inputHostHasDot, dotIndex != -1 && slashIndex > dotIndex);
}
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
if (info.inputHostHasDot || info.inputHasProtocol) {
// 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);
} else if (!isFileURL && couldDoKeywordLookup && testInput.indexOf(".") == -1) {
// Otherwise, and assuming we're allowed, there will be a search URI:
if (couldDoKeywordLookup && expectKeywordLookup) {
let urlparamInput = testInput.replace(/ /g, '+');
let searchURL = kSearchEngineURL.replace("{searchTerms}", urlparamInput);
do_check_eq(info.preferredURI.spec, searchURL);
} else if (info.fixedURI) {
// This is for lack of keyword lookup, combined with hostnames with no
// protocol:
do_check_eq(info.fixedURI, info.preferredURI);
if (isFileURL) {
do_check_eq(info.fixedURI.host, "");
} else {
let hostMatch = testInput.match(/(?:[^:\/]*:\/\/)?([^\/]+)(\/|$)/);
let host = hostMatch ? hostMatch[1] : "";
if (alternateURI) {
do_check_eq(info.fixedURI.host, "www." + host + ".com");
} else {
do_check_eq(info.fixedURI.host, host);
}
}
} else {
do_check_true(false, "There should be no cases where we got here, " +
"there's no keyword lookup, and no fixed URI." +
"Offending input: " + testInput);
// 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(testInput, info.originalInput);
}