Created attachment 152612 [details] mail demo basic sending emails with attachments via "com.sun.star.system.SimpleSystemMail" can cause data loss, if file paths instead of file urls are added. > void setAttachement([in] sequence< string > aAttachement) > raises (::com::sun::star::lang::IllegalArgumentException) > aAttachement Sets a sequence of file URLs specifying the files that should > be attached to the mail. The given file URLs must conform to Rfc1738. > The method does not check if the specified file or files really exist. steps to reproduce: - add some pdf files: a.pdf, b.pdf, c.pdf, d.pdf to LO working directory - run attached macro (-> mail_demo.bas) -> b.pdf is removed, cause it is added as file path root cause: "senddoc.exe" tries to copy attachments to a temp dir before calling email client. these files are removed on exit (). but if copy fails (e.g. attachment is a file path instead of a file url) it uses the original file path. expectation: either setAttachment() throws an exception if a file path (or invalid url) is added or "senddoc.exe" does not remove original file. smplmailclient.cxx: OUString CSmplMailClient::CopyAttachment(const OUString& sOrigAttachURL, OUString& sUserVisibleName) { // We do two things here: // 1. Make the attachment temporary filename to not contain any fancy characters possible in // original filename, that could confuse mailer, and extract the original filename to explicitly // define it; // 2. Allow the copied files be outside of the session's temporary directory, and thus not be // removed in Desktop::RemoveTemporaryDirectory() if soffice process gets closed before the // mailer finishes using them. maAttachmentFiles.emplace_back(std::make_unique<utl::TempFile>(&GetBaseTempDirURL())); maAttachmentFiles.back()->EnableKillingFile(); INetURLObject aFilePathObj(maAttachmentFiles.back()->GetURL()); OUString sNewAttachmentURL = aFilePathObj.GetMainURL(INetURLObject::DecodeMechanism::NONE); if (osl::File::copy(sOrigAttachURL, sNewAttachmentURL) == osl::FileBase::RC::E_None) { INetURLObject url(sOrigAttachURL, INetURLObject::EncodeMechanism::WasEncoded); sUserVisibleName = url.getName(INetURLObject::LAST_SEGMENT, true, INetURLObject::DecodeMechanism::WithCharset); } else { // Failed to copy original; the best effort is to use original file. It is possible that // the file gets deleted before used in spawned process; but let's hope... the worst thing // is the absent attachment file anyway. sNewAttachmentURL = sOrigAttachURL; maAttachmentFiles.pop_back(); } return sNewAttachmentURL; }
Created attachment 152613 [details] calling senddoc.exe
Created attachment 152614 [details] senddoc.exe finished
@Jan-marek, since you fixed bug 126597, I thought you might be interested in this issue...
(In reply to Xisco Faulí from comment #3) > @Jan-marek, since you fixed bug 126597, I thought you might be interested in > this issue... Maybe this is actually the same bug. The MAPI_DIALOG_MODELESS handling causes many problems. Can you or the reporter recheck this bug with a fixed LO version? Backports for 6.3 and 6.2 are pending in Gerrit, FWIW.
Dear Oliver, Could you please try to reproduce it with a master build from http://dev-builds.libreoffice.org/daily/master/ ? You can install it alongside the standard version. I have set the bug's status to 'NEEDINFO'. Please change it back to 'UNCONFIRMED' if the bug is still present in the master build
the issue still exits in: Version: 6.4.0.0.alpha0+ (x64) Build ID: 81963b5c68b492f6a75dd17fb0bec80e5dad9955 CPU threads: 12; OS: Windows 10.0; UI render: GL; VCL: win; Locale: de-DE (de_DE); UI-Language: en-US Calc: threaded after closing
Created attachment 153095 [details] deleted attachment
Dear Oliver Brinzing, To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year. There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present. If you have time, please do the following: Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/ If the bug is present, please leave a comment that includes the information from Help - About LibreOffice. If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice. Please DO NOT Update the version field Reply via email (please reply directly on the bug tracker) Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not appropriate in this case) If you want to do more to help you can test to see if your issue is a REGRESSION. To do so: 1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from https://downloadarchive.documentfoundation.org/libreoffice/old/ 2. Test your bug 3. Leave a comment with your results. 4a. If the bug was present with 3.3 - set version to 'inherited from OOo'; 4b. If the bug was not present in 3.3 - add 'regression' to keyword Feel free to come ask questions or to say hello in our QA chat: https://kiwiirc.com/nextclient/irc.freenode.net/#libreoffice-qa Thank you for helping us make LibreOffice even better for everyone! Warm Regards, QA Team MassPing-UntouchedBug
reproducible with: Version: 7.1.5.2 (x64) / LibreOffice Community Build ID: 85f04e9f809797b8199d13c421bd8a2b025d52b5 CPU threads: 4; OS: Windows 10.0 Build 19043; UI render: Skia/Raster; VCL: win Locale: de-DE (de_DE); UI: de-DE Calc: threaded
(In reply to Xisco Faulí from comment #3) > @Jan-marek, since you fixed bug 126597, I thought you might be interested in > this issue... I suppose that that could be "Mike, since you implemented the CopyAttachmentcode, you could take a look here" ;)
Mike Kaganski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/738e0bd68c4b4286b85d3eb5c1a638247fcc26bc tdf#126263: make sure to convert system path to file URLs It will be available in 7.5.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Mike Kaganski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/2a719774948fc1762e60cf7dd2ee3393c8922bb2 tdf#126263: do not try to delete non-temporary files It will be available in 7.5.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Mike Kaganski committed a patch related to this issue. It has been pushed to "libreoffice-7-4": https://git.libreoffice.org/core/commit/6038875c512480679c1d0ae2cae205df17bb3236 tdf#126263: make sure to convert system path to file URLs It will be available in 7.4.0.0.beta2. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Mike Kaganski committed a patch related to this issue. It has been pushed to "libreoffice-7-4": https://git.libreoffice.org/core/commit/530ebcde212dcedfa3435da088fcd33ff120a650 tdf#126263: do not try to delete non-temporary files It will be available in 7.4.0.0.beta2. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Mike Kaganski committed a patch related to this issue. It has been pushed to "libreoffice-7-3": https://git.libreoffice.org/core/commit/9cef4bd3795d9f98a00baaf06005a7b3ac064517 tdf#126263: make sure to convert system path to file URLs It will be available in 7.3.5. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Mike Kaganski committed a patch related to this issue. It has been pushed to "libreoffice-7-3": https://git.libreoffice.org/core/commit/e8a2be942f1c0f9cfb008d4ab81a57a3b3fb8da0 tdf#126263: do not try to delete non-temporary files It will be available in 7.3.5. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
A side note: while testing this with Thunderbird installed and set as default mail client, the b.pdf is named in the new mail attachment list "moz_mapi-1". The others are fine. This is in a few days old master, before the patches, and an own build with the patches seems to still have this behavior. This seems to have started sometime during 6.1, in 6.0 and before the attachment is named correctly b.pdf (also not removed from the original location). Maybe worth a followup report. Based on the "senddoc.exe finished" image, this does not affect Outlook, only TB.
(In reply to Gabor Kelemen (allotropia) from comment #17) Code pointer: use sCorrectedOrigAttachURL instead of sOrigAttachURL to obtain sUserVisibleName in CSmplMailClient::CopyAttachment [1]. [1] https://opengrok.libreoffice.org/xref/core/shell/source/win32/simplemail/smplmailclient.cxx?r=2a719774#204
(In reply to Mike Kaganski from comment #18) > Code pointer: use sCorrectedOrigAttachURL instead of sOrigAttachURL to > obtain sUserVisibleName in CSmplMailClient::CopyAttachment [1]. > > [1] > https://opengrok.libreoffice.org/xref/core/shell/source/win32/simplemail/ > smplmailclient.cxx?r=2a719774#204 It became quite important issue in case of Outlook: without --attach-name Outlook treats attachment from temporary ".tmp" file as "potentially unsafe attachment" and does not attach it. Fix is here https://gerrit.libreoffice.org/c/core/+/137480 Mike, thx for hint!