author | Marco Bonardo <mbonardo@mozilla.com> |
Tue, 28 Jul 2015 10:21:40 +0200 | |
changeset 254844 | 4be4031b55e3a399023cdb164f88f51534ca7542 |
parent 254843 | 2316676f94d54df4a7651c7ede57fa255439f23a |
child 254845 | 07132b9fbc109dbccf3e9bd967809b9e335aa64e |
child 255010 | 9727cedcfa9b94867a2962161f579b7d693290ef |
push id | 29122 |
push user | cbook@mozilla.com |
push date | Tue, 28 Jul 2015 14:13:05 +0000 |
treeherder | mozilla-central@07132b9fbc10 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | Mossop |
bugs | 1184960 |
milestone | 42.0a1 |
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
|
--- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -349,16 +349,20 @@ pref("browser.urlbar.suggest.history", pref("browser.urlbar.suggest.bookmark", true); pref("browser.urlbar.suggest.openpage", true); #ifdef NIGHTLY_BUILD pref("browser.urlbar.suggest.searches", true); #else pref("browser.urlbar.suggest.searches", false); #endif +// Limit the number of characters sent to the current search engine to fetch +// suggestions. +pref("browser.urlbar.maxCharsForSearchSuggestions", 20); + // Restrictions to current suggestions can also be applied (intersection). // Typed suggestion works only if history is set to true. pref("browser.urlbar.suggest.history.onlyTyped", false); pref("browser.urlbar.formatting.enabled", true); pref("browser.urlbar.trimURLs", true); pref("browser.altClickSave", false);
--- a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm +++ b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm @@ -128,17 +128,19 @@ const SearchAutocompleteProviderInternal function SearchSuggestionControllerWrapper(engine, searchToken, inPrivateContext, maxResults) { this._controller = new SearchSuggestionController(); this._controller.maxLocalResults = 0; this._controller.maxRemoteResults = maxResults; let promise = this._controller.fetch(searchToken, inPrivateContext, engine); this._suggestions = []; + this._success = false; this._promise = promise.then(results => { + this._success = true; this._suggestions = (results ? results.remote : null) || []; }).catch(err => { // fetch() rejects its promise if there's a pending request. }); } SearchSuggestionControllerWrapper.prototype = { @@ -160,16 +162,24 @@ SearchSuggestionControllerWrapper.protot */ consume() { return !this._suggestions.length ? [null, null] : [SearchAutocompleteProviderInternal.defaultMatch, this._suggestions.shift()]; }, /** + * Returns the number of fetched suggestions, or -1 if the fetching was + * incomplete or failed. + */ + get resultsCount() { + return this._success ? this._suggestions.length : -1; + }, + + /** * Stops the fetch. */ stop() { this._controller.stop(); }, }; let gInitializationPromise = null;
--- a/toolkit/components/places/UnifiedComplete.js +++ b/toolkit/components/places/UnifiedComplete.js @@ -33,16 +33,18 @@ const PREF_MATCH_TITLE = [ "m const PREF_MATCH_URL = [ "match.url", "@" ]; const PREF_SUGGEST_HISTORY = [ "suggest.history", true ]; const PREF_SUGGEST_BOOKMARK = [ "suggest.bookmark", true ]; const PREF_SUGGEST_OPENPAGE = [ "suggest.openpage", true ]; const PREF_SUGGEST_HISTORY_ONLYTYPED = [ "suggest.history.onlyTyped", false ]; const PREF_SUGGEST_SEARCHES = [ "suggest.searches", true ]; +const PREF_MAX_CHARS_FOR_SUGGEST = [ "maxCharsForSearchSuggestions", 20]; + // Match type constants. // These indicate what type of search function we should be using. const MATCH_ANYWHERE = Ci.mozIPlacesAutoComplete.MATCH_ANYWHERE; const MATCH_BOUNDARY_ANYWHERE = Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY_ANYWHERE; const MATCH_BOUNDARY = Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY; const MATCH_BEGINNING = Ci.mozIPlacesAutoComplete.MATCH_BEGINNING; const MATCH_BEGINNING_CASE_SENSITIVE = Ci.mozIPlacesAutoComplete.MATCH_BEGINNING_CASE_SENSITIVE; @@ -411,16 +413,17 @@ XPCOMUtils.defineLazyGetter(this, "Prefs store.restrictSearchesToken = prefs.get(...PREF_RESTRICT_SEARCHES); store.matchTitleToken = prefs.get(...PREF_MATCH_TITLE); store.matchURLToken = prefs.get(...PREF_MATCH_URL); store.suggestHistory = prefs.get(...PREF_SUGGEST_HISTORY); store.suggestBookmark = prefs.get(...PREF_SUGGEST_BOOKMARK); store.suggestOpenpage = prefs.get(...PREF_SUGGEST_OPENPAGE); store.suggestTyped = prefs.get(...PREF_SUGGEST_HISTORY_ONLYTYPED); store.suggestSearches = prefs.get(...PREF_SUGGEST_SEARCHES); + store.maxCharsForSearchSuggestions = prefs.get(...PREF_MAX_CHARS_FOR_SUGGEST); // If history is not set, onlyTyped value should be ignored. if (!store.suggestHistory) { store.suggestTyped = false; } store.defaultBehavior = types.concat("Typed").reduce((memo, type) => { let prefValue = store["suggest" + type]; return memo | (prefValue && @@ -593,19 +596,21 @@ function makeActionURL(action, params) { * possibly in permanent private-browsing mode. The search * should exclude privacy-sensitive results as appropriate. * @param autocompleteListener * An nsIAutoCompleteObserver. * @param resultListener * An nsIAutoCompleteSimpleResultListener. * @param autocompleteSearch * An nsIAutoCompleteSearch. + * @param prohibitSearchSuggestions + * Whether search suggestions are allowed for this search. */ function Search(searchString, searchParam, autocompleteListener, - resultListener, autocompleteSearch) { + resultListener, autocompleteSearch, prohibitSearchSuggestions) { // We want to store the original string for case sensitive searches. this._originalSearchString = searchString; this._trimmedOriginalSearchString = searchString.trim(); this._searchString = fixupSearchText(this._trimmedOriginalSearchString.toLowerCase()); this._matchBehavior = Prefs.matchBehavior; // Set the default behavior for this search. this._behavior = this._searchString ? Prefs.defaultBehavior @@ -627,16 +632,18 @@ function Search(searchString, searchPara // host, but the path must be matched in a case sensitive way. let pathIndex = this._trimmedOriginalSearchString.indexOf("/", this._strippedPrefix.length); this._autofillUrlSearchString = fixupSearchText( this._trimmedOriginalSearchString.slice(0, pathIndex).toLowerCase() + this._trimmedOriginalSearchString.slice(pathIndex) ); + this._prohibitSearchSuggestions = prohibitSearchSuggestions; + this._listener = autocompleteListener; this._autocompleteSearch = autocompleteSearch; // Create a new result to add eventual matches. Note we need a result // regardless having matches. let result = Cc["@mozilla.org/autocomplete/simple-result;1"] .createInstance(Ci.nsIAutoCompleteSimpleResult); result.setSearchString(searchString); @@ -904,28 +911,39 @@ Search.prototype = { } } // Ensure to fill any remaining space. yield Promise.all(this._remoteMatchesPromises); }), *_matchSearchSuggestions() { - if (!this.hasBehavior("searches") || this._inPrivateWindow) { + // Limit the string sent for search suggestions to a maximum length. + let searchString = this._searchTokens.join(" ") + .substr(0, Prefs.maxCharsForSearchSuggestions); + // Avoid fetching suggestions if they are not required, private browsing + // mode is enabled, or the search string may expose sensitive information. + if (!this.hasBehavior("searches") || this._inPrivateWindow || + this._prohibitSearchSuggestionsFor(searchString)) { return; } this._searchSuggestionController = PlacesSearchAutocompleteProvider.getSuggestionController( - this._searchTokens.join(" "), + searchString, this._inPrivateWindow, Prefs.maxRichResults ); let promise = this._searchSuggestionController.fetchCompletePromise .then(() => { + if (this._searchSuggestionController.resultsCount >= 0 && + this._searchSuggestionController.resultsCount < 2) { + // The original string is used to properly compare with the next search. + this._lastLowResultsSearchSuggestion = this._originalSearchString; + } while (this.pending && this._remoteMatchesCount < Prefs.maxRichResults) { let [match, suggestion] = this._searchSuggestionController.consume(); if (!suggestion) break; // Don't include the restrict token, if present. let searchString = this._searchTokens.join(" "); this._addSearchEngineMatch(match, searchString, suggestion); } @@ -935,16 +953,38 @@ Search.prototype = { // We're done if we're restricting to search suggestions. yield promise; this.stop(); } else { this._remoteMatchesPromises.push(promise); } }, + _prohibitSearchSuggestionsFor(searchString) { + if (this._prohibitSearchSuggestions) + return true; + + // Suggestions for a single letter are unlikely to be useful. + if (searchString.length < 2) + return true; + + let tokens = searchString.split(/\s/); + + // The first token may be a whitelisted host. + if (REGEXP_SINGLEWORD_HOST.test(tokens[0]) && + Services.uriFixup.isDomainWhitelisted(tokens[0], -1)) + return true; + + // Disallow fetching search suggestions for strings looking like URLs, to + // avoid disclosing information about networks or passwords. + return tokens.some(token => { + return ["/", "@", ":", "."].some(c => token.includes(c)); + }); + }, + _matchKnownUrl: function* (conn) { // Hosts have no "/" in them. let lastSlashIndex = this._searchString.lastIndexOf("/"); // Search only URLs if there's a slash in the search string... if (lastSlashIndex != -1) { // ...but not if it's exactly at the end of the search string. if (lastSlashIndex < this._searchString.length - 1) { // We don't want to execute this query right away because it needs to @@ -1696,18 +1736,25 @@ UnifiedComplete.prototype = { // Stop the search in case the controller has not taken care of it. if (this._currentSearch) { this.stopSearch(); } // Note: We don't use previousResult to make sure ordering of results are // consistent. See bug 412730 for more details. + // If the previous search didn't fetch enough search suggestions, it's + // unlikely a longer text would do. + let prohibitSearchSuggestions = + this._lastLowResultsSearchSuggestion && + searchString.length > this._lastLowResultsSearchSuggestion.length && + searchString.startsWith(this._lastLowResultsSearchSuggestion); + this._currentSearch = new Search(searchString, searchParam, listener, - this, this); + this, this, prohibitSearchSuggestions); // If we are not enabled, we need to return now. Notice we need an empty // result regardless, so we still create the Search object. if (!Prefs.enabled) { this.finishSearch(true); return; } @@ -1740,16 +1787,17 @@ UnifiedComplete.prototype = { * Indicates if we should notify the AutoComplete listener about our * results or not. */ finishSearch: function (notify=false) { TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT, this); TelemetryStopwatch.cancel(TELEMETRY_6_FIRST_RESULTS, this); // Clear state now to avoid race conditions, see below. let search = this._currentSearch; + this._lastLowResultsSearchSuggestion = search._lastLowResultsSearchSuggestion; delete this._currentSearch; if (!notify) return; // There is a possible race condition here. // When a search completes it calls finishSearch that notifies results // here. When the controller gets the last result it fires
--- a/toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js +++ b/toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js @@ -409,8 +409,76 @@ add_task(function* mixup_frecency() { title: "low frecency 1" }, { uri: NetUtil.newURI("http://example.com/lo0"), title: "low frecency 0" }, ], }); yield cleanUpSuggestions(); }); + +add_task(function* prohibit_suggestions() { + Services.prefs.setBoolPref(SUGGEST_PREF, true); + + yield check_autocomplete({ + search: "localhost", + matches: [ + { + uri: makeActionURI(("searchengine"), { + engineName: ENGINE_NAME, + input: "localhost foo", + searchQuery: "localhost", + searchSuggestion: "localhost foo", + }), + title: ENGINE_NAME, + style: ["action", "searchengine"], + icon: "", + }, + { + uri: makeActionURI(("searchengine"), { + engineName: ENGINE_NAME, + input: "localhost bar", + searchQuery: "localhost", + searchSuggestion: "localhost bar", + }), + title: ENGINE_NAME, + style: ["action", "searchengine"], + icon: "", + }, + ], + }); + Services.prefs.setBoolPref("browser.fixup.domainwhitelist.localhost", true); + do_register_cleanup(() => { + Services.prefs.clearUserPref("browser.fixup.domainwhitelist.localhost"); + }); + yield check_autocomplete({ + search: "localhost", + matches: [], + }); + + yield check_autocomplete({ + search: "1.2.3.4", + matches: [], + }); + yield check_autocomplete({ + search: "[2001::1]:30", + matches: [], + }); + yield check_autocomplete({ + search: "user:pass@test", + matches: [], + }); + yield check_autocomplete({ + search: "test/test", + matches: [], + }); + yield check_autocomplete({ + search: "data:text/plain,Content", + matches: [], + }); + + yield check_autocomplete({ + search: "a", + matches: [], + }); + + yield cleanUpSuggestions(); +});