fixes bug 250900 "nsIBrowserHistory corrupts URI strings [was: crash when a long malformed URL is entered in the Location Bar [@js_CloneFunctionObject]]" r=biesi sr=neil

browser/base/content/browser.js

@@ -2118,7 +2118,7 @@ function addToUrlbarHistory()
    try {
      if (urlToAdd.indexOf(" ") == -1) {
        var fixedUpURI = gURIFixup.createFixupURI(urlToAdd, 0);
-       gGlobalHistory.markPageAsTyped(fixedUpURI.spec);
+       gGlobalHistory.markPageAsTyped(fixedUpURI);
      }
    }
    catch(ex) {

browser/components/migration/src/nsIEProfileMigrator.cpp

@@ -510,6 +510,7 @@ nsresult
 nsIEProfileMigrator::CopyHistory(PRBool aReplace) 
 {
   nsCOMPtr<nsIBrowserHistory> hist(do_GetService(kGlobalHistoryCID));
+  nsCOMPtr<nsIIOService> ios(do_GetService(NS_IOSERVICE_CONTRACTID));
 
   // First, Migrate standard IE History entries...
   ::CoInitialize(NULL);
@@ -527,11 +528,10 @@ nsIEProfileMigrator::CopyHistory(PRBool aReplace)
       STATURL statURL;
       ULONG fetched;
       _bstr_t url, title;
-      nsCAutoString str;
+      nsCAutoString scheme;
       SYSTEMTIME st;
       PRBool validScheme = PR_FALSE;
       PRUnichar* tempTitle = nsnull;
-      nsCOMPtr<nsIURI> uri(do_CreateInstance("@mozilla.org/network/standard-url;1"));
 
       for (int count = 0; (hr = enumURLs->Next(1, &statURL, &fetched)) == S_OK; ++count) {
         if (statURL.pwcsUrl) {
@@ -557,26 +557,32 @@ nsIEProfileMigrator::CopyHistory(PRBool aReplace)
           // 3 - URL
           url = statURL.pwcsUrl;
 
-          str = (char*)(url);
+          nsDependentCString urlStr((const char *) url);
-          uri->SetSpec(str);
+          if (NS_FAILED(ios->ExtractScheme(urlStr, scheme)))
+            continue;
+          ToLowerCase(scheme);
+
           // XXXben - 
           // MSIE stores some types of URLs in its history that we can't handle, like HTMLHelp
           // and others. At present Necko isn't clever enough to delegate handling of these types
           // to the system, so we should just avoid importing them. 
           const char* schemes[] = { "http", "https", "ftp", "file" };
           for (int i = 0; i < 4; ++i) {
-            uri->SchemeIs(schemes[i], &validScheme);
+            if (validScheme = scheme.EqualsASCII(schemes[i]))
-            if (validScheme)
               break;
           }
           
           // 4 - Now add the page
           if (validScheme) {
+            nsCOMPtr<nsIURI> uri;
+            ios->NewURI(urlStr, nsnull, nsnull, getter_AddRefs(uri));
+            if (uri) {
               if (tempTitle) 
-              hist->AddPageWithDetails((char*)url, tempTitle, lastVisited);
+                hist->AddPageWithDetails(uri, tempTitle, lastVisited);
               else {
-              nsAutoString urlTitle; urlTitle.AssignWithConversion(url);
+                NS_ConvertUTF8toUTF16 urlTitle(urlStr);
-              hist->AddPageWithDetails((char*)url, urlTitle.get(), lastVisited);
+                hist->AddPageWithDetails(uri, urlTitle.get(), lastVisited);
+              }
             }
           }
         }
@@ -612,8 +618,13 @@ nsIEProfileMigrator::CopyHistory(PRBool aReplace)
 
       nsCAutoString valueNameStr(valueName);
       const nsACString& prefix = Substring(valueNameStr, 0, 3);
-      if (prefix.Equals("url"))
+      if (prefix.Equals("url")) {
-        hist->MarkPageAsTyped((const char*)data);
+        nsCOMPtr<nsIURI> uri;
+        ios->NewURI(nsDependentCString((const char *) data), nsnull, nsnull,
+                                       getter_AddRefs(uri));
+        if (uri)
+          hist->MarkPageAsTyped(uri);
+      }
       ++offset;
 
     }

browser/components/migration/src/nsOperaProfileMigrator.cpp

@@ -971,7 +971,10 @@ nsOperaProfileMigrator::CopyHistory(PRBool aReplace)
       LL_I2L(million, PR_USEC_PER_SEC);
       LL_MUL(lastVisitDate, temp, million);
 
-      hist->AddPageWithDetails(url.get(), title.get(), lastVisitDate);
+      nsCOMPtr<nsIURI> uri;
+      NS_NewURI(getter_AddRefs(uri), url);
+      if (uri)
+        hist->AddPageWithDetails(uri, title.get(), lastVisitDate);
       
       state = TITLE;
       break;

camino/src/browser/BrowserWindowController.mm

@@ -1469,11 +1469,7 @@ static NSArray* sToolbarDefaults = nil;
     nsCOMPtr<nsIURI> fixedURI;
     mURIFixer->CreateFixupURI(utf8URL, 0, getter_AddRefs(fixedURI));
     if (fixedURI)
-    {
+      mGlobalHistory->MarkPageAsTyped(fixedURI);
-      nsCAutoString spec;
-      fixedURI->GetSpec(spec);
-      mGlobalHistory->MarkPageAsTyped(spec.get());
-    }
   }
 }
 

camino/src/history/HistoryItem.mm

@@ -51,6 +51,7 @@
 
 #include "nsXPIDLString.h"
 #include "nsString.h"
+#include "nsNetUtil.h"
 
 #include "nsComponentManagerUtils.h"
 
@@ -98,7 +99,10 @@
   histDataSource->BeginUpdateBatch();
   if( ![self isExpandable] ) {
     NSString* urlString = [self url];
-    historyService->RemovePage([urlString UTF8String]);
+    nsCOMPtr<nsIURI> uri;
+    NS_NewURI(getter_AddRefs(uri), [urlString UTF8String]);
+    if (uri)
+      historyService->RemovePage(uri);
   }
   else {
     // delete a folder by iterating over each of its children and deleting them. There

content/xul/templates/src/nsXULContentUtils.cpp

@@ -330,7 +330,7 @@ nsXULContentUtils::GetTextForNode(nsIRDFNode* aNode, nsAString& aResult)
         const char* p;
         rv = resource->GetValueConst(&p);
         if (NS_FAILED(rv)) return rv;
-        CopyASCIItoUTF16(p, aResult);
+        CopyUTF8toUTF16(p, aResult);
         return NS_OK;
     }
 

toolkit/components/history/public/nsIBrowserHistory.idl

@@ -42,7 +42,7 @@
 #include "nsISupports.idl"
 #include "nsIGlobalHistory2.idl"
 
-[scriptable, uuid(68c59967-d043-4659-a973-f8f11b1d2aca)]
+[scriptable, uuid(c43079c3-3d8d-4b7c-af14-0e30ab46865f)]
 interface nsIBrowserHistory : nsIGlobalHistory2
 {
     /** 
@@ -50,7 +50,7 @@ interface nsIBrowserHistory : nsIGlobalHistory2
      * Adds a page to history with specific time stamp information. This is used in
      * the History migrator. 
      */
-    void addPageWithDetails(in string aURL, in wstring aTitle, in long long aLastVisited);
+    void addPageWithDetails(in nsIURI aURI, in wstring aTitle, in long long aLastVisited);
 
     /**
      * lastPageVisited
@@ -67,7 +67,7 @@ interface nsIBrowserHistory : nsIGlobalHistory2
     /**
      * remove a page from history
      */
-    void removePage(in string aURI);
+    void removePage(in nsIURI aURI);
 
     /**
      * removePagesFromHost
@@ -75,7 +75,7 @@ interface nsIBrowserHistory : nsIGlobalHistory2
      * If aEntireDomain is true, will assume aHost is a domain,
      * and remove all pages from the entire domain.
      */
-    void removePagesFromHost(in string aHost, in boolean aEntireDomain);
+    void removePagesFromHost(in AUTF8String aHost, in boolean aEntireDomain);
 
     /**
      * removeAllPages
@@ -98,5 +98,5 @@ interface nsIBrowserHistory : nsIGlobalHistory2
      * Designate the url as having been explicitly typed in by
      * the user, so it's okay to be an autocomplete result.
      */
-    void markPageAsTyped(in string url);
+    void markPageAsTyped(in nsIURI aURI);
 };

toolkit/components/history/src/nsGlobalHistory.cpp

@@ -869,6 +869,40 @@ nsGlobalHistory::AddNewPageToDatabase(nsIURI* aURI,
   return NS_OK;
 }
 
+nsresult
+nsGlobalHistory::RemovePageInternal(const char *aSpec)
+{
+  if (!mTable) return NS_ERROR_NOT_INITIALIZED;
+  // find the old row, ignore it if we don't have it
+  nsCOMPtr<nsIMdbRow> row;
+  nsresult rv = FindRow(kToken_URLColumn, aSpec, getter_AddRefs(row));
+  if (NS_FAILED(rv)) return NS_OK;
+
+  // remove the row
+  mdb_err err = mTable->CutRow(mEnv, row);
+  NS_ENSURE_TRUE(err == 0, NS_ERROR_FAILURE);
+
+  // if there are batches in progress, we don't want to notify
+  // observers that we're deleting items. the caller promises
+  // to handle whatever UI updating is necessary when we're finished.  
+  if (!mBatchesInProgress) {
+    // get the resource so we can do the notification
+    nsCOMPtr<nsIRDFResource> oldRowResource;
+    gRDFService->GetResource(nsDependentCString(aSpec), getter_AddRefs(oldRowResource));
+    NotifyFindUnassertions(oldRowResource, row);
+  }
+
+  // not a fatal error if we can't cut all column
+  err = row->CutAllColumns(mEnv);
+  NS_ASSERTION(err == 0, "couldn't cut all columns");
+
+  // Sigh. This is pretty bad - if the user selects a bunch of things then
+  // hits delete we'll be re-writing history over and over. Still, we will
+  // be whacking global history pretty hard after 1.0 so I don't feel too 
+  // bad.
+  return Commit(kCompressCommit);
+}
+
 nsresult
 nsGlobalHistory::SetRowValue(nsIMdbRow *aRow, mdb_column aCol, const PRInt64& aValue)
 {
@@ -1023,17 +1057,13 @@ nsGlobalHistory::GetRowValue(nsIMdbRow *aRow, mdb_column aCol,
 }
 
 NS_IMETHODIMP
-nsGlobalHistory::AddPageWithDetails(const char *aURL, const PRUnichar *aTitle, 
+nsGlobalHistory::AddPageWithDetails(nsIURI *aURI, const PRUnichar *aTitle, 
                                     PRInt64 aLastVisitDate)
 {
-  nsCOMPtr<nsIURI> uri;
+  nsresult rv = AddPageToDatabase(aURI, PR_FALSE, PR_TRUE, aLastVisitDate);
-  nsresult rv = NS_NewURI(getter_AddRefs(uri), aURL);
   if (NS_FAILED(rv)) return rv;
 
-  rv = AddPageToDatabase(uri, PR_FALSE, PR_TRUE, aLastVisitDate);
+  return SetPageTitle(aURI, nsDependentString(aTitle));
-  if (NS_FAILED(rv)) return rv;
-
-  return SetPageTitle(uri, nsDependentString(aTitle));
 }
 
 NS_IMETHODIMP
@@ -1108,49 +1138,24 @@ nsGlobalHistory::SetPageTitle(nsIURI *aURI, const nsAString& aTitle)
 
 
 NS_IMETHODIMP
-nsGlobalHistory::RemovePage(const char *aURL)
+nsGlobalHistory::RemovePage(nsIURI *aURI)
 {
-  mdb_err err;
+  nsCAutoString spec;
-  nsresult rv;
+  nsresult rv = aURI->GetSpec(spec);
-
+  if (NS_SUCCEEDED(rv))
-  if (!mTable) return NS_ERROR_NOT_INITIALIZED;
+    rv = RemovePageInternal(spec.get());
-  // find the old row, ignore it if we don't have it
+  return rv;
-  nsCOMPtr<nsIMdbRow> row;
-  rv = FindRow(kToken_URLColumn, aURL, getter_AddRefs(row));
-  if (NS_FAILED(rv)) return NS_OK;
-
-  // remove the row
-  err = mTable->CutRow(mEnv, row);
-  NS_ENSURE_TRUE(err == 0, NS_ERROR_FAILURE);
-
-  // if there are batches in progress, we don't want to notify
-  // observers that we're deleting items. the caller promises
-  // to handle whatever UI updating is necessary when we're finished.  
-  if (!mBatchesInProgress) {
-    // get the resource so we can do the notification
-    nsCOMPtr<nsIRDFResource> oldRowResource;
-    gRDFService->GetResource(nsDependentCString(aURL), getter_AddRefs(oldRowResource));
-    NotifyFindUnassertions(oldRowResource, row);
-  }
-
-  // not a fatal error if we can't cut all column
-  err = row->CutAllColumns(mEnv);
-  NS_ASSERTION(err == 0, "couldn't cut all columns");
-
-  // Sigh. This is pretty bad - if the user selects a bunch of things then
-  // hits delete we'll be re-writing history over and over. Still, we will
-  // be whacking global history pretty hard after 1.0 so I don't feel too 
-  // bad.
-  return Commit(kCompressCommit);
 }
 
 NS_IMETHODIMP
-nsGlobalHistory::RemovePagesFromHost(const char *aHost, PRBool aEntireDomain)
+nsGlobalHistory::RemovePagesFromHost(const nsACString &aHost, PRBool aEntireDomain)
 {
+  const nsCString &host = PromiseFlatCString(aHost);
+
   matchHost_t hostInfo;
   hostInfo.history = this;
   hostInfo.entireDomain = aEntireDomain;
-  hostInfo.host = aHost;
+  hostInfo.host = host.get();
   
   nsresult rv = RemoveMatchingRows(matchHostCallback, (void *)&hostInfo, PR_TRUE);
   if (NS_FAILED(rv)) return rv;
@@ -1361,16 +1366,16 @@ nsGlobalHistory::HidePage(nsIURI *aURI)
 }
 
 NS_IMETHODIMP
-nsGlobalHistory::MarkPageAsTyped(const char* aURL)
+nsGlobalHistory::MarkPageAsTyped(nsIURI *aURI)
 {
-  nsCOMPtr<nsIMdbRow> row;
+  nsCAutoString spec;
-  nsresult rv = FindRow(kToken_URLColumn, aURL, getter_AddRefs(row));
+  nsresult rv = aURI->GetSpec(spec);
-  if (NS_FAILED(rv)) {
+  if (NS_FAILED(rv)) return rv;
-    nsCOMPtr<nsIURI> uri;
-    rv = NS_NewURI(getter_AddRefs(uri), nsDependentCString(aURL), nsnull, nsnull);
-    NS_ENSURE_SUCCESS(rv, rv);
   
-    rv = AddNewPageToDatabase(uri, GetNow(), PR_FALSE, PR_TRUE, getter_AddRefs(row));
+  nsCOMPtr<nsIMdbRow> row;
+  rv = FindRow(kToken_URLColumn, spec.get(), getter_AddRefs(row));
+  if (NS_FAILED(rv)) {
+    rv = AddNewPageToDatabase(aURI, GetNow(), PR_FALSE, PR_TRUE, getter_AddRefs(row));
     NS_ENSURE_SUCCESS(rv, rv);
 
     // We don't know if this is a valid URI yet. Hide it until it finishes
@@ -1775,7 +1780,6 @@ nsGlobalHistory::GetTarget(nsIRDFResource* aSource,
             // if the top of a site does not have a title
             // (common for redirections) then return the hostname
             return GetTarget(aSource, kNC_Hostname, aTruthValue, aTarget);
-            
           }
         }
 
@@ -2003,7 +2007,7 @@ nsGlobalHistory::Unassert(nsIRDFResource* aSource,
     }
 
     // ignore any error
-    rv = RemovePage(targetUrl);
+    rv = RemovePageInternal(targetUrl);
     if (NS_FAILED(rv)) return NS_RDF_ASSERTION_REJECTED;
 
     if (!mBatchesInProgress && IsFindResource(aSource)) {

toolkit/components/history/src/nsGlobalHistory.h

@@ -312,6 +312,9 @@ protected:
                                 PRBool aRedirect,
                                 PRBool aTopLevel,
                                 nsIMdbRow **aResult);
+
+  nsresult RemovePageInternal(const char *aSpec);
+
   //
   // generic routines for setting/retrieving various datatypes
   //

xpfe/browser/resources/content/sessionHistoryUI.js

@@ -223,7 +223,7 @@ function addToUrlbarHistory()
     var url = getShortcutOrURI(urlToAdd);
     var fixedUpURI = gURIFixup.createFixupURI(url, 0);
     if (!fixedUpURI.schemeIs("data"))
-      gGlobalHistory.markPageAsTyped(fixedUpURI.spec);
+      gGlobalHistory.markPageAsTyped(fixedUpURI);
   }
   catch(ex) {
   }

xpfe/components/history/public/nsIBrowserHistory.idl

@@ -42,7 +42,7 @@
 #include "nsISupports.idl"
 #include "nsIGlobalHistory2.idl"
 
-[scriptable, uuid(460947aa-7636-43b5-949a-415a7c59da97)]
+[scriptable, uuid(cf937d0d-befa-4105-8a55-ba8cf1bae427)]
 interface nsIBrowserHistory : nsIGlobalHistory2
 {
     /**
@@ -54,7 +54,7 @@ interface nsIBrowserHistory : nsIGlobalHistory2
     /**
      * remove a page from history
      */
-    void removePage(in string aURI);
+    void removePage(in nsIURI aURI);
 
     /**
      * count
@@ -68,7 +68,7 @@ interface nsIBrowserHistory : nsIGlobalHistory2
      * If aEntireDomain is true, will assume aHost is a domain,
      * and remove all pages from the entire domain.
      */
-    void removePagesFromHost(in string aHost, in boolean aEntireDomain);
+    void removePagesFromHost(in AUTF8String aHost, in boolean aEntireDomain);
 
     /**
      * removeAllPages
@@ -84,14 +84,14 @@ interface nsIBrowserHistory : nsIGlobalHistory2
      * if the page hasn't been visited yet, then it will be added
      * as if it was visited, and then marked as hidden
      */
-    void hidePage(in nsIURI url);
+    void hidePage(in nsIURI aURI);
 
     /**
      * markPageAsTyped
      * Designate the url as having been explicitly typed in by
      * the user, so it's okay to be an autocomplete result.
      */
-    void markPageAsTyped(in string url);
+    void markPageAsTyped(in nsIURI aURI);
 };
 
 %{ C++

xpfe/components/history/src/nsGlobalHistory.cpp

@@ -807,6 +807,36 @@ nsGlobalHistory::AddNewPageToDatabase(const char *aURL,
   return NS_OK;
 }
 
+nsresult
+nsGlobalHistory::RemovePageInternal(const char *aSpec)
+{
+  if (!mTable) return NS_ERROR_NOT_INITIALIZED;
+  // find the old row, ignore it if we don't have it
+  nsCOMPtr<nsIMdbRow> row;
+  nsresult rv = FindRow(kToken_URLColumn, aSpec, getter_AddRefs(row));
+  if (NS_FAILED(rv)) return NS_OK;
+
+  // remove the row
+  mdb_err err = mTable->CutRow(mEnv, row);
+  NS_ENSURE_TRUE(err == 0, NS_ERROR_FAILURE);
+
+  // if there are batches in progress, we don't want to notify
+  // observers that we're deleting items. the caller promises
+  // to handle whatever UI updating is necessary when we're finished.  
+  if (!mBatchesInProgress) {
+    // get the resource so we can do the notification
+    nsCOMPtr<nsIRDFResource> oldRowResource;
+    gRDFService->GetResource(nsDependentCString(aSpec), getter_AddRefs(oldRowResource));
+    NotifyFindUnassertions(oldRowResource, row);
+  }
+
+  // not a fatal error if we can't cut all column
+  err = row->CutAllColumns(mEnv);
+  NS_ASSERTION(err == 0, "couldn't cut all columns");
+
+  return Commit(kCompressCommit);
+}
+
 nsresult
 nsGlobalHistory::SetRowValue(nsIMdbRow *aRow, mdb_column aCol, const PRInt64& aValue)
 {
@@ -1074,45 +1104,24 @@ nsGlobalHistory::SetPageTitle(nsIURI *aURI, const nsAString& aTitle)
 
 
 NS_IMETHODIMP
-nsGlobalHistory::RemovePage(const char *aURL)
+nsGlobalHistory::RemovePage(nsIURI *aURI)
 {
-  mdb_err err;
+  nsCAutoString spec;
-  nsresult rv;
+  nsresult rv = aURI->GetSpec(spec);
-
+  if (NS_SUCCEEDED(rv))
-  if (!mTable) return NS_ERROR_NOT_INITIALIZED;
+    rv = RemovePageInternal(spec.get());
-  // find the old row, ignore it if we don't have it
+  return rv;
-  nsCOMPtr<nsIMdbRow> row;
-  rv = FindRow(kToken_URLColumn, aURL, getter_AddRefs(row));
-  if (NS_FAILED(rv)) return NS_OK;
-
-  // remove the row
-  err = mTable->CutRow(mEnv, row);
-  NS_ENSURE_TRUE(err == 0, NS_ERROR_FAILURE);
-
-  // if there are batches in progress, we don't want to notify
-  // observers that we're deleting items. the caller promises
-  // to handle whatever UI updating is necessary when we're finished.  
-  if (!mBatchesInProgress) {
-    // get the resource so we can do the notification
-    nsCOMPtr<nsIRDFResource> oldRowResource;
-    gRDFService->GetResource(nsDependentCString(aURL), getter_AddRefs(oldRowResource));
-    NotifyFindUnassertions(oldRowResource, row);
-  }
-
-  // not a fatal error if we can't cut all column
-  err = row->CutAllColumns(mEnv);
-  NS_ASSERTION(err == 0, "couldn't cut all columns");
-
-  return Commit(kCompressCommit);
 }
 
 NS_IMETHODIMP
-nsGlobalHistory::RemovePagesFromHost(const char *aHost, PRBool aEntireDomain)
+nsGlobalHistory::RemovePagesFromHost(const nsACString &aHost, PRBool aEntireDomain)
 {
+  const nsCString &host = PromiseFlatCString(aHost);
+
   matchHost_t hostInfo;
   hostInfo.history = this;
   hostInfo.entireDomain = aEntireDomain;
-  hostInfo.host = aHost;
+  hostInfo.host = host.get();
   
   nsresult rv = RemoveMatchingRows(matchHostCallback, (void *)&hostInfo, PR_TRUE);
   if (NS_FAILED(rv)) return rv;
@@ -1359,12 +1368,16 @@ nsGlobalHistory::HidePage(nsIURI *aURI)
 }
 
 NS_IMETHODIMP
-nsGlobalHistory::MarkPageAsTyped(const char* aURL)
+nsGlobalHistory::MarkPageAsTyped(nsIURI *aURI)
 {
+  nsCAutoString spec;
+  nsresult rv = aURI->GetSpec(spec);
+  if (NS_FAILED(rv)) return rv;
+
   nsCOMPtr<nsIMdbRow> row;
-  nsresult rv = FindRow(kToken_URLColumn, aURL, getter_AddRefs(row));
+  rv = FindRow(kToken_URLColumn, spec.get(), getter_AddRefs(row));
   if (NS_FAILED(rv)) {
-    rv = AddNewPageToDatabase(aURL, GetNow(), getter_AddRefs(row));
+    rv = AddNewPageToDatabase(spec.get(), GetNow(), getter_AddRefs(row));
     NS_ENSURE_SUCCESS(rv, rv);
 
     // We don't know if this is a valid URI yet. Hide it until it finishes
@@ -1985,7 +1998,7 @@ nsGlobalHistory::Unassert(nsIRDFResource* aSource,
     }
 
     // ignore any error
-    rv = RemovePage(targetUrl);
+    rv = RemovePageInternal(targetUrl);
     if (NS_FAILED(rv)) return NS_RDF_ASSERTION_REJECTED;
 
     return NS_OK;

xpfe/components/history/src/nsGlobalHistory.h

@@ -310,6 +310,9 @@ protected:
   nsresult AddNewPageToDatabase(const char *aURL,
                                 PRInt64 aDate,
                                 nsIMdbRow **aResult);
+
+  nsresult RemovePageInternal(const char *aSpec);
+
   //
   // generic routines for setting/retrieving various datatypes
   //