Bug 393302 - Correct memory handling in MAPISendMail() and CMapiImp::SendMail() to fix "Send to > Mail Recipient" crash. r=jorgk a=IanN DONTBUILD SEAMONKEY_2_49_ESR_RELBRANCH
authorMike Kaganski <mikekaganski@gmail.com>
Thu, 17 Jan 2019 01:35:00 +0100
branchSEAMONKEY_2_49_ESR_RELBRANCH
changeset 28322 85b1819ab625bf70a80dddac8d4687f02d805181
parent 28321 8db1b7bb3957e05d5d4d9390a9f1a11fcdd43a07
child 28323 8172fe05882cf42de2eac87d6d315ace70c5d4da
push id2136
push userfrgrahl@gmx.net
push dateMon, 15 Jul 2019 16:13:42 +0000
treeherdercomm-esr52@6b30146aa411 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorgk, IanN
bugs393302
Bug 393302 - Correct memory handling in MAPISendMail() and CMapiImp::SendMail() to fix "Send to > Mail Recipient" crash. r=jorgk a=IanN DONTBUILD SeaMonkey ESR52 release branch only.
mailnews/mapi/mapiDll/MapiDll.cpp
mailnews/mapi/mapihook/build/msgMapi.idl
mailnews/mapi/mapihook/src/msgMapiHook.cpp
mailnews/mapi/mapihook/src/msgMapiHook.h
mailnews/mapi/mapihook/src/msgMapiImp.cpp
mailnews/mapi/mapihook/src/msgMapiImp.h
--- a/mailnews/mapi/mapiDll/MapiDll.cpp
+++ b/mailnews/mapi/mapiDll/MapiDll.cpp
@@ -204,50 +204,17 @@ ULONG FAR PASCAL MAPISendMail (LHANDLE l
             LoginFlag = MAPI_LOGON_UI ;
 
         hr = MAPILogon (ulUIParam, (LPTSTR) NULL, (LPTSTR) NULL, LoginFlag, 0, &lhSession) ;
         if (hr != SUCCESS_SUCCESS)
             return MAPI_E_LOGIN_FAILURE ;
         bTempSession = TRUE ;
     }
 
-    // we need to deal with null data passed in by MAPI clients, specially when MAPI_DIALOG is set.
-    // The MS COM type lib code generated by MIDL for the MS COM interfaces checks for these parameters
-    // to be non null, although null is a valid value for them here. 
-    nsMapiRecipDesc * lpRecips ;
-    nsMapiFileDesc * lpFiles ;
-
-    nsMapiMessage Message ;
-    memset (&Message, 0, sizeof (nsMapiMessage) ) ;
-    nsMapiRecipDesc Recipient ;
-    memset (&Recipient, 0, sizeof (nsMapiRecipDesc) );
-    nsMapiFileDesc Files ;
-    memset (&Files, 0, sizeof (nsMapiFileDesc) ) ;
-
-    if(!lpMessage)
-    {
-       lpMessage = &Message ;
-    }
-    if(!lpMessage->lpRecips)
-    {
-        lpRecips = &Recipient ;
-    }
-    else
-        lpRecips = lpMessage->lpRecips ;
-    if(!lpMessage->lpFiles)
-    {
-        lpFiles = &Files ;
-    }
-    else
-        lpFiles = lpMessage->lpFiles ;
-
-    hr = pNsMapi->SendMail (lhSession, lpMessage, 
-                            (short) lpMessage->nRecipCount, lpRecips,
-                            (short) lpMessage->nFileCount, lpFiles,
-                            flFlags, ulReserved);
+    hr = pNsMapi->SendMail(lhSession, lpMessage, flFlags, ulReserved);
 
     // we are seeing a problem when using Word, although we return success from the MAPI support
     // MS COM interface in mozilla, we are getting this error here. This is a temporary hack !!
     if (hr == 0x800703e6)
         hr = SUCCESS_SUCCESS;
     
     if (bTempSession)
         MAPILogoff (lhSession, ulUIParam, 0,0) ;
--- a/mailnews/mapi/mapihook/build/msgMapi.idl
+++ b/mailnews/mapi/mapihook/build/msgMapi.idl
@@ -9,18 +9,18 @@ import "unknwn.idl";
 
 typedef wchar_t LOGIN_PW_TYPE[256];
 
 typedef struct
 {
     unsigned long     ulReserved;            
     unsigned long     flFlags;               /* Flags */
     unsigned long     nPosition_NotUsed;     /* character in text to be replaced by attachment */
-    LPTSTR            lpszPathName;          /* Full path name including file name */
-    LPTSTR            lpszFileName;          /* Real (original) file name */
+    LPSTR             lpszPathName;          /* Full path name including file name */
+    LPSTR             lpszFileName;          /* Real (original) file name */
     unsigned char *   lpFileType_NotUsed ;
 } nsMapiFileDesc, * lpnsMapiFileDesc;
 
 
 typedef struct
 {
     unsigned long      ulReserved;           
     unsigned long      ulRecipClass;  /* MAPI_TO, MAPI_CC, MAPI_BCC, MAPI_ORIG    */
@@ -59,35 +59,30 @@ interface nsIMapi : IUnknown
                   [in, unique] LOGIN_PW_TYPE aPassWord, [in] unsigned long aFlags,
                   [out] unsigned long *aSessionId);
 
     HRESULT Initialize();
     HRESULT IsValid();
     HRESULT IsValidSession([in] unsigned long aSession);
 
     HRESULT SendMail([in] unsigned long aSession, [in, unique] lpnsMapiMessage aMessage,
-                  [in] short aRecipCount, [in, size_is(aRecipCount)] lpnsMapiRecipDesc aRecips, 
-                  [in] short aFileCount, [in, size_is(aFileCount)] lpnsMapiFileDesc aFiles, 
-                  [in] unsigned long aFlags, [in] unsigned long aReserved) ;
+                     [in] unsigned long aFlags, [in] unsigned long aReserved) ;
 
-    HRESULT SendDocuments( [in] unsigned long aSession, 
-                  [in, unique] LPTSTR aDelimChar, [in, unique] LPTSTR aFilePaths,
-                  [in, unique] LPTSTR aFileNames, [in] ULONG aFlags ) ;
+    HRESULT SendDocuments([in] unsigned long aSession,
+                          [in, unique] LPTSTR aDelimChar, [in, unique] LPTSTR aFilePaths,
+                          [in, unique] LPTSTR aFileNames, [in] ULONG aFlags ) ;
 
-    HRESULT FindNext(  [in] unsigned long aSession, [in] ULONG ulUIParam, [in, unique] LPTSTR lpszMessageType,
-                              [in, unique] LPTSTR lpszSeedMessageID, [in] ULONG flFlags, [in] ULONG ulReserved,
-                              [in] [out] char lpszMessageID[64] ) ;
+    HRESULT FindNext([in] unsigned long aSession, [in] ULONG ulUIParam, [in, unique] LPTSTR lpszMessageType,
+                     [in, unique] LPTSTR lpszSeedMessageID, [in] ULONG flFlags, [in] ULONG ulReserved,
+                     [in] [out] char lpszMessageID[64] ) ;
 
-    HRESULT ReadMail( [in] unsigned long lhSession, [in] ULONG ulUIParam, [in, unique] LPTSTR lpszMessageID,
-                              [in] ULONG flFlags, [in] ULONG ulReserved, [out] lpnsMapiMessage *lppMessage);
+    HRESULT ReadMail([in] unsigned long lhSession, [in] ULONG ulUIParam, [in, unique] LPTSTR lpszMessageID,
+                     [in] ULONG flFlags, [in] ULONG ulReserved, [out] lpnsMapiMessage *lppMessage);
 
-    HRESULT DeleteMail( [in] unsigned long lhSession, [in] ULONG ulUIParam, [in, unique] LPTSTR lpszMessageID,
-                              [in] ULONG flFlags, [in] ULONG ulReserved);
+    HRESULT DeleteMail([in] unsigned long lhSession, [in] ULONG ulUIParam, [in, unique] LPTSTR lpszMessageID,
+                       [in] ULONG flFlags, [in] ULONG ulReserved);
 
-    HRESULT SaveMail( [in] unsigned long lhSession, [in] ULONG ulUIParam, [in, unique] lpnsMapiMessage lppMessage,
-                      [in] ULONG flFlags, [in] ULONG ulReserved, [in, unique] LPTSTR lpszMessageID);
+    HRESULT SaveMail([in] unsigned long lhSession, [in] ULONG ulUIParam, [in, unique] lpnsMapiMessage lppMessage,
+                     [in] ULONG flFlags, [in] ULONG ulReserved, [in, unique] LPTSTR lpszMessageID);
 
     HRESULT Logoff (unsigned long aSession);
     HRESULT CleanUp();
 };
-
-
-
--- a/mailnews/mapi/mapihook/src/msgMapiHook.cpp
+++ b/mailnews/mapi/mapihook/src/msgMapiHook.cpp
@@ -361,119 +361,38 @@ nsresult nsMapiHook::BlindSendMail (unsi
     PR_CWait(pSendListener, PR_MicrosecondsToInterval(1000UL));
     PR_CExitMonitor(pSendListener);
     NS_ProcessPendingEvents(thread);
   }
 
   return rv ;
 }
 
-// this is used to populate comp fields with Unicode data
-nsresult nsMapiHook::PopulateCompFields(lpnsMapiMessage aMessage,
-                                    nsIMsgCompFields * aCompFields)
-{
-  nsresult rv = NS_OK ;
-
-  if (aMessage->lpOriginator)
-    aCompFields->SetFrom (NS_ConvertASCIItoUTF16((char *) aMessage->lpOriginator->lpszAddress));
-
-  nsAutoString To ;
-  nsAutoString Cc ;
-  nsAutoString Bcc ;
-
-  NS_NAMED_LITERAL_STRING(Comma, ",");
-
-  if (aMessage->lpRecips)
-  {
-    for (int i=0 ; i < (int) aMessage->nRecipCount ; i++)
-    {
-      if (aMessage->lpRecips[i].lpszAddress || aMessage->lpRecips[i].lpszName)
-      {
-        const char *addressWithoutType = (aMessage->lpRecips[i].lpszAddress)
-          ? aMessage->lpRecips[i].lpszAddress : aMessage->lpRecips[i].lpszName;
-        if (!PL_strncasecmp(addressWithoutType, "SMTP:", 5))
-          addressWithoutType += 5;
-        switch (aMessage->lpRecips[i].ulRecipClass)
-        {
-        case MAPI_TO :
-          if (!To.IsEmpty())
-            To += Comma;
-          To.Append(NS_ConvertASCIItoUTF16(addressWithoutType));
-          break;
-
-        case MAPI_CC :
-          if (!Cc.IsEmpty())
-            Cc += Comma;
-          Cc.Append(NS_ConvertASCIItoUTF16(addressWithoutType));
-          break;
-
-        case MAPI_BCC :
-          if (!Bcc.IsEmpty())
-            Bcc += Comma;
-          Bcc.Append(NS_ConvertASCIItoUTF16(addressWithoutType));
-          break;
-        }
-      }
-    }
-  }
-
-  MOZ_LOG(MAPI, mozilla::LogLevel::Debug, ("to: %s cc: %s bcc: %s \n", NS_ConvertUTF16toUTF8(To).get(), NS_ConvertUTF16toUTF8(Cc).get(), NS_ConvertUTF16toUTF8(Bcc).get()));
-  // set To, Cc, Bcc
-  aCompFields->SetTo (To) ;
-  aCompFields->SetCc (Cc) ;
-  aCompFields->SetBcc (Bcc) ;
-
-  // set subject
-  if (aMessage->lpszSubject)
-    aCompFields->SetSubject(NS_ConvertASCIItoUTF16(aMessage->lpszSubject));
-
-  // handle attachments as File URL
-  rv = HandleAttachments (aCompFields, aMessage->nFileCount, aMessage->lpFiles, true) ;
-  if (NS_FAILED(rv)) return rv ;
-
-  // set body
-  if (aMessage->lpszNoteText)
-  {
-      nsString Body;
-      CopyASCIItoUTF16(aMessage->lpszNoteText, Body);
-      if (Body.IsEmpty() || Body.Last() != '\n')
-        Body.AppendLiteral(CRLF);
-
-      // This is needed when Simple MAPI is used without a compose window.
-      // See bug 1366196.
-      if (Body.Find("<html>") == kNotFound)
-        aCompFields->SetForcePlainText(true);
-
-      rv = aCompFields->SetBody(Body) ;
-  }
-  return rv ;
-}
-
 nsresult nsMapiHook::HandleAttachments (nsIMsgCompFields * aCompFields, int32_t aFileCount,
-                                        lpnsMapiFileDesc aFiles, BOOL aIsUnicode)
+                                        lpnsMapiFileDesc aFiles)
 {
     nsresult rv = NS_OK ;
+    // Do nothing if there are no files to process.
+    if (!aFiles || aFileCount <= 0)
+        return NS_OK;
 
     nsAutoCString Attachments ;
     nsAutoCString TempFiles ;
 
     nsCOMPtr <nsIFile> pFile = do_CreateInstance (NS_LOCAL_FILE_CONTRACTID, &rv) ;
     if (NS_FAILED(rv) || (!pFile) ) return rv ;
     nsCOMPtr <nsIFile> pTempDir = do_CreateInstance (NS_LOCAL_FILE_CONTRACTID, &rv) ;
     if (NS_FAILED(rv) || (!pTempDir) ) return rv ;
 
     for (int i=0 ; i < aFileCount ; i++)
     {
         if (aFiles[i].lpszPathName)
         {
             // check if attachment exists
-            if (aIsUnicode)
-                pFile->InitWithPath (nsDependentString(aFiles[i].lpszPathName));
-            else
-                pFile->InitWithNativePath (nsDependentCString((const char*)aFiles[i].lpszPathName));
+            pFile->InitWithNativePath(nsDependentCString((const char*)aFiles[i].lpszPathName));
 
             bool bExist ;
             rv = pFile->Exists(&bExist) ;
             MOZ_LOG(MAPI, mozilla::LogLevel::Debug, ("nsMapiHook::HandleAttachments: filename: %s path: %s exists = %s \n", (const char*)aFiles[i].lpszFileName, (const char*)aFiles[i].lpszPathName, bExist ? "true" : "false"));
             if (NS_FAILED(rv) || (!bExist) ) return NS_ERROR_FILE_TARGET_DOES_NOT_EXIST ;
 
             //Temp Directory
             nsCOMPtr <nsIFile> pTempDir;
@@ -491,22 +410,19 @@ nsresult nsMapiHook::HandleAttachments (
             // rename or copy the existing temp file with the real file name
 
             nsAutoString leafName ;
             // convert to Unicode using Platform charset
             // leafName already contains a unicode leafName from lpszPathName. If we were given
             // a value for lpszFileName, use it. Otherwise stick with leafName
             if (aFiles[i].lpszFileName)
             {
-              nsAutoString wholeFileName;
-                if (aIsUnicode)
-                    wholeFileName.Assign(aFiles[i].lpszFileName);
-                else
-                    NS_CopyNativeToUnicode(nsDependentCString((char *) aFiles[i].lpszFileName),
-                                           wholeFileName);
+                nsAutoString wholeFileName;
+                NS_CopyNativeToUnicode(nsDependentCString((char *)aFiles[i].lpszFileName),
+                                       wholeFileName);
                 // need to find the last '\' and find the leafname from that.
                 int32_t lastSlash = wholeFileName.RFindChar(char16_t('\\'));
                 if (lastSlash != kNotFound)
                   leafName.Assign(Substring(wholeFileName, lastSlash + 1));
                 else
                   leafName.Assign(wholeFileName);
             }
             else
@@ -624,17 +540,17 @@ nsresult nsMapiHook::PopulateCompFieldsW
     nsAutoString Subject ;
     rv = NS_CopyNativeToUnicode(nsDependentCString((char *) aMessage->lpszSubject),
                                 Subject);
     if (NS_FAILED(rv)) return rv;
     aCompFields->SetSubject(Subject);
   }
 
   // handle attachments as File URL
-  rv = HandleAttachments (aCompFields, aMessage->nFileCount, aMessage->lpFiles, false) ;
+  rv = HandleAttachments(aCompFields, aMessage->nFileCount, aMessage->lpFiles);
   if (NS_FAILED(rv)) return rv ;
 
   // set body
   if (aMessage->lpszNoteText)
   {
     nsAutoString Body ;
     rv = NS_CopyNativeToUnicode(nsDependentCString((char *) aMessage->lpszNoteText),
                                 Body);
--- a/mailnews/mapi/mapihook/src/msgMapiHook.h
+++ b/mailnews/mapi/mapihook/src/msgMapiHook.h
@@ -13,21 +13,20 @@ class nsMapiHook
 
         static bool DisplayLoginDialog(bool aLogin, char16_t **aUsername, 
                         char16_t **aPassword);
         static bool VerifyUserName(const nsString& aUsername, nsCString& aIdKey);
 
         static bool IsBlindSendAllowed () ;
         static nsresult BlindSendMail (unsigned long aSession, nsIMsgCompFields * aCompFields) ;
         static nsresult ShowComposerWindow (unsigned long aSession, nsIMsgCompFields * aCompFields) ;
-        static nsresult PopulateCompFields(lpnsMapiMessage aMessage, nsIMsgCompFields * aCompFields) ;
         static nsresult PopulateCompFieldsWithConversion(lpnsMapiMessage aMessage, 
                                         nsIMsgCompFields * aCompFields) ;
         static nsresult PopulateCompFieldsForSendDocs(nsIMsgCompFields * aCompFields, 
                                         ULONG aFlags, LPTSTR aDelimChar, LPTSTR aFilePaths) ;
         static nsresult HandleAttachments (nsIMsgCompFields * aCompFields, int32_t aFileCount, 
-                                        lpnsMapiFileDesc aFiles, BOOL aIsUnicode) ;
+                                           lpnsMapiFileDesc aFiles) ;
         static void CleanUp();
 
         static bool isMapiService;
 };
 
 #endif  // MSG_MAPI_HOOK_H_
--- a/mailnews/mapi/mapihook/src/msgMapiImp.cpp
+++ b/mailnews/mapi/mapihook/src/msgMapiImp.cpp
@@ -191,39 +191,40 @@ STDMETHODIMP CMapiImp::Login(unsigned lo
             break;
         }
     }
 
     return S_OK;
 }
 
 STDMETHODIMP CMapiImp::SendMail( unsigned long aSession, lpnsMapiMessage aMessage,
-     short aRecipCount, lpnsMapiRecipDesc aRecips , short aFileCount, lpnsMapiFileDesc aFiles , 
      unsigned long aFlags, unsigned long aReserved)
 {
     nsresult rv = NS_OK ;
 
     MOZ_LOG(MAPI, mozilla::LogLevel::Debug, ("CMapiImp::SendMail using flags %d\n", aFlags));
-    // Assign the pointers in the aMessage struct to the array of Recips and Files
-    // received here from MS COM. These are used in BlindSendMail and ShowCompWin fns 
-    aMessage->lpRecips = aRecips ;
-    aMessage->lpFiles = aFiles ;
+
+    // Handle possible nullptr argument.
+    nsMapiMessage Message;
+    memset(&Message, 0, sizeof(nsMapiMessage));
+
+    if (!aMessage)
+    {
+        aMessage = &Message;
+    }
 
     MOZ_LOG(MAPI, mozilla::LogLevel::Debug, ("CMapiImp::SendMail flags=%x subject: %s sender: %s\n", 
       aFlags, (char *) aMessage->lpszSubject, (aMessage->lpOriginator) ? aMessage->lpOriginator->lpszAddress : ""));
 
     /** create nsIMsgCompFields obj and populate it **/
     nsCOMPtr<nsIMsgCompFields> pCompFields = do_CreateInstance(NS_MSGCOMPFIELDS_CONTRACTID, &rv) ;
     if (NS_FAILED(rv) || (!pCompFields) ) return MAPI_E_INSUFFICIENT_MEMORY ;
 
-    if (aFlags & MAPI_UNICODE)
-        rv = nsMapiHook::PopulateCompFields(aMessage, pCompFields) ;
-    else
-        rv = nsMapiHook::PopulateCompFieldsWithConversion(aMessage, pCompFields) ;
-    
+    rv = nsMapiHook::PopulateCompFieldsWithConversion(aMessage, pCompFields);
+
     if (NS_SUCCEEDED (rv))
     {
         // see flag to see if UI needs to be brought up
         if (!(aFlags & MAPI_DIALOG))
         {
             rv = nsMapiHook::BlindSendMail(aSession, pCompFields);
         }
         else
--- a/mailnews/mapi/mapihook/src/msgMapiImp.h
+++ b/mailnews/mapi/mapihook/src/msgMapiImp.h
@@ -33,18 +33,16 @@ public :
 
   // Interface INsMapi
 
   STDMETHODIMP Login(unsigned long aUIArg, LOGIN_PW_TYPE aLogin, 
                      LOGIN_PW_TYPE aPassWord, unsigned long aFlags,
                      unsigned long *aSessionId);
 
   STDMETHODIMP SendMail( unsigned long aSession, lpnsMapiMessage aMessage,
-       short aRecipCount, lpnsMapiRecipDesc aRecips , 
-       short aFileCount, lpnsMapiFileDesc aFiles , 
        unsigned long aFlags, unsigned long aReserved) ;
 
   STDMETHODIMP SendDocuments( unsigned long aSession, LPTSTR aDelimChar,
                               LPTSTR aFilePaths, LPTSTR aFileNames, ULONG aFlags);
 
   STDMETHODIMP FindNext(  unsigned long aSession, unsigned long ulUIParam, LPTSTR lpszMessageType,
                             LPTSTR lpszSeedMessageID, unsigned long flFlags, unsigned long ulReserved,
                             unsigned char lpszMessageID[64] );