User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

Create a new message and attach two messages, see screenshot.

Actual results:

In the sent e-mail, another message was attached, albeit with the same subject (but different content).

This is a catastrophic bug.

Expected results:

Of course the message(s) that were attached should have been sent, nothing else.

This is what you can see in the sent message. Same "file name", different size and content.

Changing severity to S1 because of total privacy obliteration. Makes the product dangerous and useless.

A bit more detail. This is not reproducible reliably, but it happens ever second time or so starting with a particular first message. The second message is not important. Looking at the sent message, the MIME structure is corrupted, where the first one ends and the second starts I see:

arVCqjz3et4UUhUMWWVc1hS8p+ZEpqDaK7iJKKK6BH2V55B57lsQPY2VDDYKEW2xBoyNV6oY
G+8UbuKxZm6ReJwKpUTOU8fpTs7p06ezdetWjDFvfbIngXwaEStY3CJkqw0ifUjXSvpq/4yw
YgLU+EhVgK1KINUJd7xOIRUKqhNIVRmsS8ATP--------------s0crb6N1zdN7FeAR8nfudLdn
Content-Type: message/rfc822; name="Themenvorschlag: ...

So basically there is a line break missing after the base64 part of the first message. The original problem is more severe: That a wrong first message is attached in the first place. That happens even after repairing the folder of that message.

This is not reproducible reliably, but it happens ever second time or so starting with a particular first message.

Confirmed. Happens the second time after start, so STR:
Start TB, new message, attach message 1, attach message 2, send. All good.
Mew message, attach message 1, attach message 2, send. Bad.

Surprisingly it happens with mailnews.send.jsmodule set to false as well. It doesn't appear to happen in TB 78.

Hi Alice, I hope you can reproduce the issue and find the regression. Repeating the STR:

1. Set pref mailnews.send.jsmodule to false. That will use the old C++ sending module. (You can also try with mailnews.send.jsmodule at true, see below.)
2. Start TB
3. New message to yourself, drag two messages from a folder as attachment. Make sure you remember which ones you used.
4. Press Ctrl+Shift+Enter. That will send the message to the outbox to avoid sending it out for real. Inspect the message in the outbox. Does it have the correct message attachments? The first time around, it should be correct. Delete message from outbox.
5. Repeat step 3 and 4. The second time around, the message in the outbox should be incorrect, one attachment missing.

Note that TB 78 didn't have mailnews.send.jsmodule. Then the new JS-based sending module was introduced, still with mailnews.send.jsmodule at false. At some stage, that was switched to true. So to compare apples with apples, and not pears, for the purpose of the test, we should leave it at false, but you can try with true as well.

P.S.: Point 3: We've been attaching messages from a local folder, so please try that. The behaviour may be different when you attach from an IMAP folder.

(In reply to Zoe Martin from comment #5)

Hi Alice, I hope you can reproduce the issue and find the regression. Repeating the STR:

1. Set pref mailnews.send.jsmodule to false. That will use the old C++ sending module. (You can also try with mailnews.send.jsmodule at true, see below.)
2. Start TB
3. New message to yourself, drag two messages from a folder as attachment. Make sure you remember which ones you used.
4. Press Ctrl+Shift+Enter. That will send the message to the outbox to avoid sending it out for real. Inspect the message in the outbox. Does it have the correct message attachments? The first time around, it should be correct. Delete message from outbox.
5. Repeat step 3 and 4. The second time around, the message in the outbox should be incorrect, one attachment missing.

Note that TB 78 didn't have mailnews.send.jsmodule. Then the new JS-based sending module was introduced, still with mailnews.send.jsmodule at false. At some stage, that was switched to true. So to compare apples with apples, and not pears, for the purpose of the test, we should leave it at false, but you can try with true as well.

I cannot reproduce the issue in Tb91.0.3 Windows10. I tried several times with different profiles, but could not reproduce the problem.

Alice, thanks for trying. A very weird bug. Before we could reproduce it, it happened on every second send. It's documented in comment #0, 2 attachments in compose window, one after sending. We also still have the message with the corrupt MIME structure from comment #3. It continued happening after repairing the folder that contains the first message. However, now we took a copy of that folder including the MSF file, and then removed the MSF file of the original folder. That led to a rebuild of the MSF of the original folder, and apparently also the copied MSF file was rebuilt. Now we can't reproduce the error any more. Very strange.

Of course a message attachment is just a URL pointing to a message in a folder, and if that folder is compacted or repaired while the compose window references a message in the folder, strange things will happen. But we didn't compact or repair while composing. Also, a damaged MSF file may lead to strange effects, but the repair should have fixed that. Finally nothing explains why a stale/wrong reference into a folder should cause a corrupt MIME structure of the composed message.

Sadly this bug is rather dramatic. Image that the wrong attachment is sent out as happened to us. Worst case that can cost someone's life or livelihood.

I can't reproduce as well, but the patch should fix it.

Looks like you added some precaution, but that won't fix the issue here, which now no one can reproduce. Even with the existing code, the output in comment #3 can't have happened since _encodeBase64() always appended \r\n to any line. As an effect this bug will be marked "fixed" when there is a catastrophic failure lurking somewhere.

There are checks in pickEncoding, so that message/rfc822 part won't go through _encodeBase64, but kept as it is.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/9bef6d65c895
Make sure MimePart ends with a line break. r=mkmelin

Comment on attachment 9239967 [details]
Bug 1729432 - Make sure MimePart ends with a line break. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: No one can reproduce it, I think it's still good to land it to prevent conflicts in future uplifts
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): low

Comment on attachment 9239967 [details]
Bug 1729432 - Make sure MimePart ends with a line break. r=mkmelin

[Triage Comment]
Approved for beta

Thunderbird 93.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/f19b1d33b23c

Comment on attachment 9239967 [details]
Bug 1729432 - Make sure MimePart ends with a line break. r=mkmelin

[Triage Comment]
Approved for esr91

Thunderbird 91.1.2:
https://hg.mozilla.org/releases/comm-esr91/rev/0e4421ad216e