Bug 1335638 - Fix crash when accessing invalid IMAP URI. r=rkent a=jorgk
authorJorg K <jorgk@jorgk.com>
Fri, 10 Feb 2017 16:06:20 +0100
changeset 27750 9807cf77ed6948b29c4750a3351b8b2484388a88
parent 27749 fe3301a68e98c555ea25c304c20cd156e48e6b5f
child 27751 a4c1629a5108273f1e0a4b81d8b07a4d9cf2a955
push id1850
push userclokep@gmail.com
push dateWed, 08 Mar 2017 19:29:12 +0000
treeherdercomm-esr52@028df196b2d9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrkent, jorgk
bugs1335638
Bug 1335638 - Fix crash when accessing invalid IMAP URI. r=rkent a=jorgk
mailnews/imap/src/nsIMAPNamespace.cpp
mailnews/imap/src/nsImapIncomingServer.cpp
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapProtocol.h
--- a/mailnews/imap/src/nsIMAPNamespace.cpp
+++ b/mailnews/imap/src/nsIMAPNamespace.cpp
@@ -638,13 +638,13 @@ char *nsIMAPNamespaceList::GenerateFullF
     else
     {
       NS_ASSERTION(false, "couldn't allocate server folder name");
     }
   }
   else
   {
     // Could not find other users namespace on the given host
-    NS_ASSERTION(false, "couldn't find namespace for given host");
+    NS_WARNING("couldn't find namespace for given host");
   }
   return (fullFolderName);
 }
 
--- a/mailnews/imap/src/nsImapIncomingServer.cpp
+++ b/mailnews/imap/src/nsImapIncomingServer.cpp
@@ -445,17 +445,17 @@ nsImapIncomingServer::GetImapConnectionA
 
   nsCOMPtr<nsIMsgMailNewsUrl> mailnewsurl = do_QueryInterface(aImapUrl, &rv);
   if (aProtocol)
   {
     rv = aProtocol->LoadImapUrl(mailnewsurl, aConsumer);
     // *** jt - in case of the time out situation or the connection gets
     // terminated by some unforseen problems let's give it a second chance
     // to run the url
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv) && rv != NS_ERROR_ILLEGAL_VALUE)
     {
       NS_ASSERTION(false, "shouldn't get an error loading url");
       rv = aProtocol->LoadImapUrl(mailnewsurl, aConsumer);
     }
   }
   else
   {   // unable to get an imap connection to run the url; add to the url
      // queue
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -613,17 +613,17 @@ nsImapProtocol::GetImapServerKey()
 {
   if (m_serverKey.IsEmpty() && m_imapServerSink)
   {
     m_imapServerSink->GetServerKey(m_serverKey);
   }
   return m_serverKey.get();
 }
 
-void
+nsresult
 nsImapProtocol::SetupSinkProxy()
 {
   nsresult res;
   if (m_runningUrl)
   {
     if (!m_imapMailFolderSink)
     {
       nsCOMPtr<nsIImapMailFolderSink> aImapMailFolderSink;
@@ -633,30 +633,39 @@ nsImapProtocol::SetupSinkProxy()
         m_imapMailFolderSink = new ImapMailFolderSinkProxy(aImapMailFolderSink);
       }
     }
 
     if (!m_imapMessageSink)
     {
       nsCOMPtr<nsIImapMessageSink> aImapMessageSink;
       (void) m_runningUrl->GetImapMessageSink(getter_AddRefs(aImapMessageSink));
-      m_imapMessageSink = new ImapMessageSinkProxy(aImapMessageSink);
+      if (aImapMessageSink) {
+        m_imapMessageSink = new ImapMessageSinkProxy(aImapMessageSink);
+      } else {
+        return NS_ERROR_ILLEGAL_VALUE;
+      }
     }
     if (!m_imapServerSink)
     {
-       nsCOMPtr<nsIImapServerSink> aImapServerSink;
-       res = m_runningUrl->GetImapServerSink(getter_AddRefs(aImapServerSink));
-       m_imapServerSink = new ImapServerSinkProxy(aImapServerSink);
+      nsCOMPtr<nsIImapServerSink> aImapServerSink;
+      res = m_runningUrl->GetImapServerSink(getter_AddRefs(aImapServerSink));
+      if (aImapServerSink) {
+        m_imapServerSink = new ImapServerSinkProxy(aImapServerSink);
+      } else {
+        return NS_ERROR_ILLEGAL_VALUE;
+      }
     }
     if (!m_imapProtocolSink)
     {
       nsCOMPtr<nsIImapProtocolSink> anImapProxyHelper(do_QueryInterface(NS_ISUPPORTS_CAST(nsIImapProtocolSink*, this), &res));
       m_imapProtocolSink = new ImapProtocolSinkProxy(anImapProxyHelper);
     }
   }
+  return NS_OK;
 }
 
 static void SetSecurityCallbacksFromChannel(nsISocketTransport* aTrans, nsIChannel* aChannel)
 {
   nsCOMPtr<nsIInterfaceRequestor> callbacks;
   aChannel->GetNotificationCallbacks(getter_AddRefs(callbacks));
 
   nsCOMPtr<nsILoadGroup> loadGroup;
@@ -1153,18 +1162,17 @@ NS_IMETHODIMP nsImapProtocol::GetUrlWind
 {
   NS_ENSURE_ARG_POINTER(aUrl);
   NS_ENSURE_ARG_POINTER(aMsgWindow);
   return aUrl->GetMsgWindow(aMsgWindow);
 }
 
 NS_IMETHODIMP nsImapProtocol::SetupMainThreadProxies()
 {
-  SetupSinkProxy();
-  return NS_OK;
+  return SetupSinkProxy();
 }
 
 NS_IMETHODIMP nsImapProtocol::OnInputStreamReady(nsIAsyncInputStream *inStr)
 {
   // should we check if it's a close vs. data available?
   if (m_idle)
   {
     uint64_t bytesAvailable = 0;
@@ -1606,17 +1614,18 @@ bool nsImapProtocol::ProcessCurrentURL()
       }
       return false;
     }
   }
 
   if (!m_imapMailFolderSink && m_imapProtocolSink)
   {
     // This occurs when running another URL in the main thread loop
-    m_imapProtocolSink->SetupMainThreadProxies();
+    rv = m_imapProtocolSink->SetupMainThreadProxies();
+    NS_ENSURE_SUCCESS(rv, false);
   }
 
   // Reinitialize the parser
   GetServerStateParser().InitializeState();
   GetServerStateParser().SetConnected(true);
 
   // acknowledge that we are running the url now..
   nsCOMPtr<nsIMsgMailNewsUrl> mailnewsurl = do_QueryInterface(m_runningUrl, &rv);
@@ -2144,17 +2153,20 @@ NS_IMETHODIMP nsImapProtocol::LoadImapUr
       return NS_OK;
     m_urlInProgress = true;
     m_imapMailFolderSink = nullptr;
     rv = SetupWithUrl(aURL, aConsumer);
     NS_ASSERTION(NS_SUCCEEDED(rv), "error setting up imap url");
     if (NS_FAILED(rv))
       return rv;
 
-    SetupSinkProxy(); // generate proxies for all of the event sinks in the url
+    rv = SetupSinkProxy(); // generate proxies for all of the event sinks in the url
+    if (NS_FAILED(rv)) // URL can be invalid.
+      return rv;
+
     m_lastActiveTime = PR_Now();
     if (m_transport && m_runningUrl)
     {
       nsImapAction imapAction;
       m_runningUrl->GetImapAction(&imapAction);
       // if we're shutting down, and not running the kinds of urls we run at
       // shutdown, then this should fail because running urls during
       // shutdown will very likely fail and potentially hang.
--- a/mailnews/imap/src/nsImapProtocol.h
+++ b/mailnews/imap/src/nsImapProtocol.h
@@ -368,17 +368,17 @@ private:
   nsWeakPtr   m_server;
 
   RefPtr<ImapMailFolderSinkProxy> m_imapMailFolderSink;
   RefPtr<ImapMessageSinkProxy>    m_imapMessageSink;
   RefPtr<ImapServerSinkProxy>     m_imapServerSink;
   RefPtr<ImapProtocolSinkProxy>   m_imapProtocolSink;
 
   // helper function to setup imap sink interface proxies
-  void SetupSinkProxy();
+  nsresult SetupSinkProxy();
   // End thread support stuff
 
   bool GetDeleteIsMoveToTrash();
   bool GetShowDeletedMessages();
   nsCString m_currentCommand;
   nsImapServerResponseParser m_parser;
   nsImapServerResponseParser& GetServerStateParser() { return m_parser; }