From 1d5ec356b3d5ec98177a76eaf2d32e728fd1a742 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sun, 10 Nov 2019 23:38:54 -0500 Subject: Bug 393302 - Correct memory handling in MAPISendMail() and CMapiImp::SendMail() to fix "Send to > Mail Recipient" crash. Tag #1273 --- mailnews/mapi/mapiDll/MapiDll.cpp | 35 +--------- mailnews/mapi/mapihook/build/msgMapi.idl | 6 +- mailnews/mapi/mapihook/src/msgMapiHook.cpp | 103 +++-------------------------- mailnews/mapi/mapihook/src/msgMapiHook.h | 3 +- mailnews/mapi/mapihook/src/msgMapiImp.cpp | 19 +++--- mailnews/mapi/mapihook/src/msgMapiImp.h | 2 - 6 files changed, 25 insertions(+), 143 deletions(-) (limited to 'mailnews') diff --git a/mailnews/mapi/mapiDll/MapiDll.cpp b/mailnews/mapi/mapiDll/MapiDll.cpp index b6b6ddbf7..ee527df4a 100644 --- a/mailnews/mapi/mapiDll/MapiDll.cpp +++ b/mailnews/mapi/mapiDll/MapiDll.cpp @@ -209,40 +209,7 @@ ULONG FAR PASCAL MAPISendMail (LHANDLE lhSession, ULONG ulUIParam, nsMapiMessage 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 !! diff --git a/mailnews/mapi/mapihook/build/msgMapi.idl b/mailnews/mapi/mapihook/build/msgMapi.idl index 3ca3fd493..e669b5452 100644 --- a/mailnews/mapi/mapihook/build/msgMapi.idl +++ b/mailnews/mapi/mapihook/build/msgMapi.idl @@ -14,8 +14,8 @@ 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; @@ -64,8 +64,6 @@ interface nsIMapi : IUnknown 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) ; HRESULT SendDocuments( [in] unsigned long aSession, diff --git a/mailnews/mapi/mapihook/src/msgMapiHook.cpp b/mailnews/mapi/mapihook/src/msgMapiHook.cpp index 02696fe94..54aae7954 100644 --- a/mailnews/mapi/mapihook/src/msgMapiHook.cpp +++ b/mailnews/mapi/mapihook/src/msgMapiHook.cpp @@ -365,91 +365,13 @@ nsresult nsMapiHook::BlindSendMail (unsigned long aSession, nsIMsgCompFields * a 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("") == 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 ; @@ -464,10 +386,7 @@ nsresult nsMapiHook::HandleAttachments (nsIMsgCompFields * aCompFields, int32_t 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) ; @@ -490,16 +409,16 @@ nsresult nsMapiHook::HandleAttachments (nsIMsgCompFields * aCompFields, int32_t // rename or copy the existing temp file with the real file name nsAutoString leafName ; + nsAutoCString platformCharSet; // 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 - ConvertToUnicode(nsMsgI18NFileSystemCharset(), (char *) aFiles[i].lpszFileName, wholeFileName); + nsAutoString wholeFileName; + if (platformCharSet.IsEmpty()) + platformCharSet.Assign(nsMsgI18NFileSystemCharset()); + rv = ConvertToUnicode(platformCharSet.get(), (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) @@ -629,7 +548,7 @@ nsresult nsMapiHook::PopulateCompFieldsWithConversion(lpnsMapiMessage aMessage, } // 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 diff --git a/mailnews/mapi/mapihook/src/msgMapiHook.h b/mailnews/mapi/mapihook/src/msgMapiHook.h index 8860d5227..296aef459 100644 --- a/mailnews/mapi/mapihook/src/msgMapiHook.h +++ b/mailnews/mapi/mapihook/src/msgMapiHook.h @@ -18,13 +18,12 @@ class nsMapiHook 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; diff --git a/mailnews/mapi/mapihook/src/msgMapiImp.cpp b/mailnews/mapi/mapihook/src/msgMapiImp.cpp index 3e2bcbbf6..39193701c 100644 --- a/mailnews/mapi/mapihook/src/msgMapiImp.cpp +++ b/mailnews/mapi/mapihook/src/msgMapiImp.cpp @@ -199,16 +199,20 @@ STDMETHODIMP CMapiImp::Login(unsigned long aUIArg, LOGIN_PW_TYPE aLogin, LOGIN_P } 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 : "")); @@ -217,10 +221,7 @@ STDMETHODIMP CMapiImp::SendMail( unsigned long aSession, lpnsMapiMessage aMessag nsCOMPtr 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)) { diff --git a/mailnews/mapi/mapihook/src/msgMapiImp.h b/mailnews/mapi/mapihook/src/msgMapiImp.h index 875e9c6cf..562ab6cad 100644 --- a/mailnews/mapi/mapihook/src/msgMapiImp.h +++ b/mailnews/mapi/mapihook/src/msgMapiImp.h @@ -38,8 +38,6 @@ public : 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, -- cgit v1.2.3