From 21e2de87e0b86595e7295b14dd829f128b071d21 Mon Sep 17 00:00:00 2001 From: "edward.lee@engineering.uiuc.edu" Date: Mon, 7 Apr 2008 14:25:48 -0700 Subject: [PATCH] Bug 422277 - assertions in nsNavHistoryAutocomplete ("not a UTF8 string", etc.). r=dietrich, a1.9=beltzner --- toolkit/components/places/src/Makefile.in | 1 + toolkit/components/places/src/nsNavHistory.h | 5 + .../places/src/nsNavHistoryAutoComplete.cpp | 32 ++- .../places/tests/unit/test_422277.js | 229 ++++++++++++++++++ 4 files changed, 262 insertions(+), 5 deletions(-) create mode 100644 toolkit/components/places/tests/unit/test_422277.js diff --git a/toolkit/components/places/src/Makefile.in b/toolkit/components/places/src/Makefile.in index 6ddcc4d663b..c0fb71d65d1 100644 --- a/toolkit/components/places/src/Makefile.in +++ b/toolkit/components/places/src/Makefile.in @@ -65,6 +65,7 @@ REQUIRES = xpcom \ intl \ layout \ locale \ + uconv \ unicharutil \ autocomplete \ storage \ diff --git a/toolkit/components/places/src/nsNavHistory.h b/toolkit/components/places/src/nsNavHistory.h index c446fe31d69..8edd08a9827 100644 --- a/toolkit/components/places/src/nsNavHistory.h +++ b/toolkit/components/places/src/nsNavHistory.h @@ -69,6 +69,7 @@ #include "nsIObserverService.h" #include "nsServiceManagerUtils.h" #include "nsIStringBundle.h" +#include "nsITextToSubURI.h" #include "nsITimer.h" #ifdef MOZ_XUL #include "nsITreeSelection.h" @@ -711,6 +712,10 @@ protected: PRBool mAutoCompleteFinishedSearch; void DoneSearching(PRBool aFinished); + // Used to unescape encoded URI strings for searching + nsCOMPtr mTextURIService; + nsString FixupURIText(const nsAString &aURIText); + PRInt32 mExpireDaysMin; PRInt32 mExpireDaysMax; PRInt32 mExpireSites; diff --git a/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp b/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp index e4488d0751f..3ddfe62e3ef 100644 --- a/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp +++ b/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp @@ -418,6 +418,10 @@ nsNavHistory::StartSearch(const nsAString & aSearchString, NS_ENSURE_ARG_POINTER(aListener); + // Lazily init nsITextToSubURI service + if (!mTextURIService) + mTextURIService = do_GetService(NS_ITEXTTOSUBURI_CONTRACTID); + // Keep track of the previous search results to try optimizing PRUint32 prevMatchCount = mCurrentResultURLs.Count(); nsAutoString prevSearchString(mCurrentSearchString); @@ -682,11 +686,6 @@ nsNavHistory::AutoCompleteProcessSearch(mozIStorageStatement* aQuery, // XXX bug 412734 PRBool dummy; if (!mCurrentResultURLs.Get(escapedEntryURL, &dummy)) { - // Convert the escaped UTF16 (all ascii) to ASCII then unescape to UTF16 - NS_LossyConvertUTF16toASCII cEntryURL(escapedEntryURL); - NS_UnescapeURL(cEntryURL); - NS_ConvertUTF8toUTF16 entryURL(cEntryURL); - PRInt64 parentId = 0; nsAutoString entryTitle, entryFavicon, entryBookmarkTitle; rv = aQuery->GetString(kAutoCompleteIndex_Title, entryTitle); @@ -717,6 +716,9 @@ nsNavHistory::AutoCompleteProcessSearch(mozIStorageStatement* aQuery, if (aHasMoreResults) *aHasMoreResults = PR_TRUE; + // Unescape the url to search for unescaped terms + nsString entryURL = FixupURIText(escapedEntryURL); + // Determine if every token matches either the bookmark title, tags, // page title, or page url PRBool matchAll = PR_TRUE; @@ -827,3 +829,23 @@ nsNavHistory::AutoCompleteFeedback(PRInt32 aIndex, return NS_OK; } + +nsString +nsNavHistory::FixupURIText(const nsAString &aURIText) +{ + // Unescaping utilities expect UTF8 strings + NS_ConvertUTF16toUTF8 escaped(aURIText); + + nsString fixedUp; + // Use the service if we have it to avoid invalid UTF8 strings + if (mTextURIService) { + mTextURIService->UnEscapeURIForUI(NS_LITERAL_CSTRING("UTF-8"), + escaped, fixedUp); + return fixedUp; + } + + // Fallback on using this if the service is unavailable for some reason + NS_UnescapeURL(escaped); + CopyUTF8toUTF16(escaped, fixedUp); + return fixedUp; +} diff --git a/toolkit/components/places/tests/unit/test_422277.js b/toolkit/components/places/tests/unit/test_422277.js new file mode 100644 index 00000000000..9fe3f17d309 --- /dev/null +++ b/toolkit/components/places/tests/unit/test_422277.js @@ -0,0 +1,229 @@ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is Places Test Code. + * + * The Initial Developer of the Original Code is + * Edward Lee . + * Portions created by the Initial Developer are Copyright (C) 2008 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +/** + * Test bug 422277 to make sure bad escaped uris don't get escaped. This makes + * sure we don't hit an assertion for "not a UTF8 string". + */ + +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); +let current_test = 0; + +function AutoCompleteInput(aSearches) { + this.searches = aSearches; +} +AutoCompleteInput.prototype = { + timeout: 10, + textValue: "", + searches: null, + searchParam: "", + popupOpen: false, + minResultsForPopup: 0, + invalidate: function() {}, + disableAutoComplete: false, + completeDefaultIndex: false, + get popup() { return this; }, + onSearchBegin: function() {}, + onSearchComplete: function() {}, + setSelectedIndex: function() {}, + get searchCount() { return this.searches.length; }, + getSearchAt: function(aIndex) this.searches[aIndex], + QueryInterface: XPCOMUtils.generateQI([Ci.nsIAutoCompleteInput, Ci.nsIAutoCompletePopup]) +}; + +function ensure_results(aSearch, aExpected) +{ + let controller = Cc["@mozilla.org/autocomplete/controller;1"]. + getService(Ci.nsIAutoCompleteController); + + // Make an AutoCompleteInput that uses our searches + // and confirms results on search complete + let input = new AutoCompleteInput(["history"]); + + controller.input = input; + + let numSearchesStarted = 0; + input.onSearchBegin = function() { + numSearchesStarted++; + do_check_eq(numSearchesStarted, 1); + }; + + input.onSearchComplete = function() { + do_check_eq(numSearchesStarted, 1); + aExpected = aExpected.slice(); + + // Check to see the expected uris and titles match up (in any order) + for (let i = 0; i < controller.matchCount; i++) { + let value = controller.getValueAt(i); + let comment = controller.getCommentAt(i); + + print("Looking for an expected result of " + value + ", " + comment + "..."); + let j; + for (j = 0; j < aExpected.length; j++) { + let [uri, title, tags] = aExpected[j]; + + // Skip processed expected results + if (uri == undefined) continue; + + // Load the real uri and titles and tags if necessary + uri = iosvc.newURI(kURIs[uri], null, null).spec; + title = kTitles[title]; + if (tags) + title += " \u2013 " + tags.map(function(aTag) kTitles[aTag]); + + // Got a match on both uri and title? + if (uri == value && title == comment) { + print("Got it at index " + j + "!!"); + // Make it undefined so we don't process it again + aExpected[j] = [,]; + break; + } + } + + // We didn't hit the break, so we must have not found it + if (j == aExpected.length) + do_throw("Didn't find the current result (" + value + ", " + comment + ") in expected: " + aExpected); + } + + // Make sure we have the right number of results + do_check_eq(controller.matchCount, aExpected.length); + + // If we expect results, make sure we got matches + do_check_eq(controller.searchStatus, aExpected.length ? + Ci.nsIAutoCompleteController.STATUS_COMPLETE_MATCH : + Ci.nsIAutoCompleteController.STATUS_COMPLETE_NO_MATCH); + + // Fetch the next test if we have more + if (++current_test < gTests.length) + run_test(); + + do_test_finished(); + }; + + print("Searching for.. " + aSearch); + controller.startSearch(aSearch); +} + +// Get history services +try { + var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"]. + getService(Ci.nsINavHistoryService); + var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory); + var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]. + getService(Ci.nsINavBookmarksService); + var tagsvc = Cc["@mozilla.org/browser/tagging-service;1"]. + getService(Ci.nsITaggingService); + var iosvc = Cc["@mozilla.org/network/io-service;1"]. + getService(Ci.nsIIOService); +} catch(ex) { + do_throw("Could not get services\n"); +} + +// Some date not too long ago +let gDate = new Date(Date.now() - 1000 * 60 * 60) * 1000; + +function addPageBook(aURI, aTitle, aBook, aTags, aKey) +{ + let uri = iosvc.newURI(kURIs[aURI], null, null); + let title = kTitles[aTitle]; + + let out = [aURI, aTitle, aBook, aTags, aKey]; + out.push("\nuri=" + kURIs[aURI]); + out.push("\ntitle=" + title); + + // Add the page and a visit for good measure + bhist.addPageWithDetails(uri, title, gDate); + + // Add a bookmark if we need to + if (aBook != undefined) { + let book = kTitles[aBook]; + let bmid = bmsvc.insertBookmark(bmsvc.unfiledBookmarksFolder, uri, + bmsvc.DEFAULT_INDEX, book); + out.push("\nbook=" + book); + + // Add a keyword to the bookmark if we need to + if (aKey != undefined) + bmsvc.setKeywordForBookmark(bmid, aKey); + + // Add tags if we need to + if (aTags != undefined && aTags.length > 0) { + // Convert each tag index into the title + let tags = aTags.map(function(aTag) kTitles[aTag]); + tagsvc.tagURI(uri, tags); + out.push("\ntags=" + tags); + } + } + + print("\nAdding page/book/tag: " + out.join(", ")); +} + +function run_test() { + print("\n"); + // Search is asynchronous, so don't let the test finish immediately + do_test_pending(); + + // Load the test and print a description then run the test + let [description, search, expected, func] = gTests[current_test]; + print(description); + + // Do an extra function if necessary + if (func) + func(); + + ensure_results(search, expected); +} + +// ************************************************* +// *** vvv Custom Test Stuff Goes Below Here vvv *** +// ************************************************* + +// Define some shared uris and titles (each page needs its own uri) +let kURIs = [ + "http://site/%EAid", +]; +let kTitles = [ + "title", +]; + +addPageBook(0, 0); + +// For each test, provide a title, the search terms, and an array of +// [uri,title] indices of the pages that should be returned, followed by an +// optional function +let gTests = [ + ["0: Bad escaped uri stays escaped", + "site", [[0,0]]], +];