Bug 422277 - assertions in nsNavHistoryAutocomplete ("not a UTF8 string", etc.). r=dietrich, a1.9=beltzner

toolkit/components/places/src/Makefile.in

@@ -65,6 +65,7 @@ REQUIRES = xpcom \
            intl \
            layout \
            locale \
+           uconv \
            unicharutil \
            autocomplete \
            storage \

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<nsITextToSubURI> mTextURIService;
+  nsString FixupURIText(const nsAString &aURIText);
+
   PRInt32 mExpireDaysMin;
   PRInt32 mExpireDaysMax;
   PRInt32 mExpireSites;

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;
+}

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 <edward.lee@engineering.uiuc.edu>.
+ * 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]]],
+];