GNOME Bugzilla – Bug 618514
Forwarded messages as attachments encoded incorrectly
Last modified: 2011-03-15 09:48:50 UTC
Forwarding from a downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=591734 when forwarding a message from evolution, using the mapi connector, the message is forwarded as an eml file that is unrecognizable by most mail clients. When forwarding a simple plain-text message to myself at Gmail, Yahoo!, and another IMAP account, I only received a blank email with an attachment: ATT60520..eml. Gmail, Yahoo!, and SeaMonkey 2 (same code base as TB3), didn't know what to do with this message.
*** Bug 632900 has been marked as a duplicate of this bug. ***
Hi, I can still see this problem, and looking more closely, it seems that the forwarded message attachment contains the text, plus one extra null byte. Here's a copy of the attachment that I right-click-saved from OWA: Thread-Topic: orig subj Thread-Index: Acvc2IYRa2EdVrAZTjaVes/6bDzIcQ== Accept-Language: en-US, sv-SE X-MS-Has-Attach: X-MS-Exchange-Organization-SCL: -1 X-MS-TNEF-Correlator: <F8E420FCC4E2DB4FA107EB2FE156BD7E86F16F@somehost> MIME-Version: 1.0 Content-class: urn:content-classes:message Subject: orig subj Reply-To: "Finney, Sean" <Sean.Finney@somedomain> From: "Finney, Sean" <Sean.Finney@somedomain> Message-ID: <F8E420FCC4E2DB4FA107EB2FE156BD7E86F16F@somehost> Content-Type: multipart/mixed; boundary="=-RBDTtRvROjM6gtxxX5fe" --=-RBDTtRvROjM6gtxxX5fe Content-Transfer-Encoding: 8bit Content-Type: text/plain orig body ^@ --=-RBDTtRvROjM6gtxxX5fe-- where "^@" is less highlighting a null byte. Off by one error somewhere, maybe?
From another test message, i set a bp on camel_mime_part_set_content, and can see that indeed the length seems to be the strlen +1 for the message contents. Interestingly, this seems to be the case for not just the attachment but also for the message body too! Here is the first of two backtraces, with the message body from the main message (the one containing the forwarded message as an attachment):
+ Trace 226217
okay bugzilla seems to think that's one backtrace, but there's two in there. sorry for the mess.
(In reply to comment #2) > where "^@" is less highlighting a null byte. > > Off by one error somewhere, maybe? It's not off by one, it's because evolution-mapi is adding this \0 intentionally, because some parts, like calendar part, expects null-terminated string. The other part, like mailer, doesn't expect/need this extra \0, but it doesn't remove it.
Created attachment 182730 [details] [review] proposed ema patch for evolution-mapi; Could you try with this patch, whether it'll fix both issues, please? This is fixing extra \0 for newly downloaded messages (for old messages is used local cache, so remove ~/.local/share/evolution/mail/mapi/<account>/folders/* and refetch your messages) and then try if also forwarding will work as expected on both sides, the other client and evolution client, maybe owa too, or not. Thanks in advance.
OK, the above patch is fixing only the extra \0, but I recalled what's the fix for this - the attached message cannot be stored as raw data, but as an image object in the attachment, thus it requires a bit more work to make it behave as we want. I'm looking on this today.
Created commit 1172bf3 in ema master (2.91.92+) for the unrelated patch
just to confirm: yes, that does seem to fix the extra null byte, and yes, you're correct that it's not the underlying cause the forwarding problem--so a bugfix nonetheless, but not the bug that was reported :) Let me know if you need me to test anything else!
OK, I got it finally. The patch is lightly larger, but only because of reorganizing the code, moving relevant parts from camel to libexchangemapi. It works nicely for me, but please let me know if anything doesn't work for you. Thanks in advance. (I also tried attaching a message which has as attachments other messages, one with real attachments, and the OWA together with evo shows them properly.) Created commit 918f7d4 in ema master (2.91.92+)
Created attachment 182831 [details] [review] Same patch as above, but for gnome-2-32 As no further changes are going into the gnome-2-32 branch, and as many people need to still support gnome-2-32 for a while, here's the same patch on top of the .32 version, for those who were fortunate enough to dig their way to this BR :)
Review of attachment 182831 [details] [review]: .
If patch is safe, we should commit patch in branch 2-32. It's always easy to pick patch from git sources instead of searching for the bug id.
(In reply to comment #13) > If patch is safe, we should commit patch in branch 2-32. It's always easy to > pick patch from git sources instead of searching for the bug id. I'm against this approach personally, because if we are not going to do a release on gnome-2-32 branch, then the commit is just waste of time and disk space in git.
(In reply to comment #14) > I'm against this approach personally, because if we are not going to do a > release on gnome-2-32 branch, then the commit is just waste of time and disk > space in git. I disagree. Even if *we* don't do a new release, having the commits right there in the gnome-2-32 branch means that every distributor can cherry-pick them if they see fit. And we do have the *option* of doing a new 2.32.x release if we want to. (Please could we try not to mingle code reorganisation and bug fixes in the same patch, btw? I'm looking at your commit in evo-mapi, since we're going to need to do something similar in evo-ews, and I can't actually see what you changed, amongst the code reorg. When moving code, best practice is to *just* move it in one commit, and if you want to make changes to it, do that in a separate commit.)
(In reply to comment #15) > I disagree. Even if *we* don't do a new release, having the commits right there > in the gnome-2-32 branch means that every distributor can cherry-pick them if > they see fit. And we do have the *option* of doing a new 2.32.x release if we > want to. Well, your opinion, my opinion. Nothing relevant to this bug. Anyway, please do not commit on your own to modules which you do not maintain unless you've an approval. Do you remember the fiasco with nss being committed to stable version? > (Please could we try not to mingle code reorganisation and bug fixes in the > same patch, btw? ...) Maybe we can, but I cannot. The code move was indivisible part of the fix. In other words, taking only the change, and not the move of the code will not fix the bug, but break build.
Normally I would expect to see code moved without changing it, and then the bug fix in a second commit. Was that not possible in this case?
Within the "reorg" part of the commit (i.e. the large blocks of moved code), iirc the significant part of the change was the block starting with CAMEL_IS_MIME_MESSAGE. I don't recall there being more. The changes for the .32 version of the patch were basically to cope with the camel API changes in 2.9x, (the GCancellable stuff and some seeking related stuff). And PS: while I've already made my opinion about the lack of any stable+maintained version of evolution to others via IRC, I also agree that it's mostly tangential to this bug. However, I have been keeping a patch queue of ~12-15 other fixes in our locally maintained .32 tree. So if you do change your mind, or have somewhere less official to place it, just let me know and I'm happy to share.