1609690 - I upgraded to Thunderbird 68.4.1 and now Thunderbird crashes on every start (when mail.imap.use_envelope_cmd set to true)

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Jerry Raack]

Attachment: Thunderbird Files Contents Version 68.4.1.JPG

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

Steps to reproduce:

I got a message from Thunderbird saying there was a new version on Jan 15. I acknowledged that I wanted to upgrade. When the upgrade finished, Thunderbird crashed upon starting. Now it crashes every time I try to start it. So, I tried to go back to the previous version of Thunderbird (68.3.1), but then it complains that the profile was created by a later version of Thunderbird and wants me to create a new profile! But, I have 26.8 GB of data in what should be my default profile, and I do NOT want to start over and loose all my local files! So, how do I fix this mess without losing all the data that is in the current default profile?????? PLEASE HELP ME!

Actual results:

The upgrade to version 68.4.1 resulted in a Thunderbird that is not usable with my profile, and I can not revert back to an earlier Thunderbird version because 68.4.1 changed the profile to make it unusable with earlier versions.

Expected results:

The upgrade should have gone smoothly and I should be able to use Thunderbird. And I should be able to revert to an earlier version to run, but the profile that was altered in the upgrade will not allow an earlier version to run. As you can see by the attached picture, there are 2 profiles showing, and both are marked "default", but only one file "default" shows when I invoke Thunderbird with Alt R at startup, and it is the newer "default" file that will not run with the older Thunderbird. PLEASE HELP ME GET BACK UP AND RUNNING. I have no email client now!

Comment 1

[Wayne Mery (:wsmwk)]

What AV do you run? Bitdefender is one common issue.
I need at least one crash ID to nail the cause of your crashes. The crash IDs are in the "submitted" crashes folder. see https://support.mozilla.org/en-US/kb/mozilla-crash-reporter-tb#w_viewing-reports-outside-of-thunderbird

Comment 2

[Jerry Raack]

Hi Wayne, here are 2 of the many crash ids: 1579135238 and 1579107263
Hope you can help me,
Jerry

Comment 3

[Jerry Raack]

I run ESET Smart Security as my security software. Currently on product version 13.0.24.0 of that product.

Comment 4

[Wayne Mery (:wsmwk)]

Unfortunately those are not crash IDs. (I have no idea what your numbers are) Crash ID has the format bp-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxx YYMMDD for example bp-f470709a-26ab-48ec-84cb-ffd380191221. look for bp-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxx YYMMDD.txt files in the "submitted" directory.

Comment 5

[Jerry Raack]

Sorry for the confusion. Here are a couple:
bp-1be549b1-8107-40f6-bc85-6ced40200116.txt
and
bp-3f30930a-bce9-45cd-83a7-3a4760200115.txt

Comment 6

[Jerry Raack]

Attachment: Thunderbird Crash IDs.JPG

My Thunderbird crash file contents as of now (10:55AM 1/16/2020)

Comment 7

[Wayne Mery (:wsmwk)]

Ben, can you have a look?

~ #60 crash. Signature doesn't exist prior to version 68. But it could be a dup of one of our other crash bug reports.

bp-1be549b1-8107-40f6-bc85-6ced40200116

0 ucrtbase.dll strlen context
1 xul.dll nsTSubstring<char>::Append(char const*, unsigned int, std::nothrow_t const&) xpcom/string/nsTSubstring.cpp:765 cfi
2 xul.dll nsTSubstring<char>::Append(char const*, unsigned int) xpcom/string/nsTSubstring.cpp:754 cfi
3 xul.dll nsImapServerResponseParser::parse_address(nsTAutoStringN<char, 64>&) comm/mailnews/imap/src/nsImapServerResponseParser.cpp:1409 cfi
4 xul.dll nsImapServerResponseParser::envelope_data() comm/mailnews/imap/src/nsImapServerResponseParser.cpp:1316 cfi
5 xul.dll nsImapServerResponseParser::msg_fetch() comm/mailnews/imap/src/nsImapServerResponseParser.cpp:1189 cfi
6 xul.dll nsImapServerResponseParser::response_data() comm/mailnews/imap/src/nsImapServerResponseParser.cpp:653 cfi
7 xul.dll nsImapServerResponseParser::ParseIMAPServerResponse(char const*, bool, char*) comm/mailnews/imap/src/nsImapServerResponseParser.cpp:188 cfi
8 xul.dll nsImapProtocol::ParseIMAPandCheckForNewMail(char const*, bool) comm/mailnews/imap/src/nsImapProtocol.cpp:1903 cfi
9 xul.dll nsImapProtocol::FetchMessage(nsTString<char> const&, nsIMAPeFetchFields, char const*, unsigned int, unsigned int, char*) comm/mailnews/imap/src/nsImapProtocol.cpp:3583 cfi
10 xul.dll nsImapProtocol::FolderMsgDumpLoop(unsigned int*, unsigned int, nsIMAPeFetchFields) comm/mailnews/imap/src/nsImapProtocol.cpp:4354 cfi
11 xul.dll nsImapProtocol::ProcessMailboxUpdate(bool) comm/mailnews/imap/src/nsImapProtocol.cpp:4233 cfi
12 xul.dll nsImapProtocol::ProcessSelectedStateURL() comm/mailnews/imap/src/nsImapProtocol.cpp:0 cfi
13 xul.dll nsImapProtocol::ProcessCurrentURL() comm/mailnews/imap/src/nsImapProtocol.cpp:1757 cfi
14 xul.dll nsImapProtocol::ImapThreadMainLoop() comm/mailnews/imap/src/nsImapProtocol.cpp:1419 cfi
15 xul.dll nsImapProtocol::Run() comm/mailnews/imap/src/nsImapProtocol.cpp:1102 cfi

Comment 8

[Jerry Raack]

I greatly appreciate you investigating this to get a fix. In the meantime, is there any steps I can take to get back to a working version of Thunderbird on my main computer? Right now I have to use my traveling PC for Thunderbird email which does not have all my files on it.
Thanks, Jerry

Comment 9

[Ben Campbell]

Hmm. Both crash logs are crashing at the same place, which is good :-)

It seems to be a bug in the IMAP response parsing: the IMAP server is sending a response which causes TB to choke.
It does seem to be a TB bug - what I think the server is sending is technically valid. But the odd thing is that this code hasn't changed for a looooong time (it was imported into the current source control in 2008, and hasn't been modified since, except for a minor whitespace auto-reformatting last year, which looks sane to me).

Doesn't explain why Jerry only hit it after upgrading to 68.4.1... perhaps thunderbird identifies itself to the IMAP server slightly differently, and causes the server to alter the responses it sends? Sunspots? Just me grasping at straws there ;-)

Jerry:

• it sounds like you're still able to connect to the same IMAP account with your laptop (presumably running an older TB). Can you confirm that?
• The crash occurs because of a response received from the IMAP server. Does your 68.4.1 install it still crash if you start up TB while offline, say?

Technical notes to myself:
Looks like it's crashing because this call...
https://searchfox.org/comm-central/source/mailnews/imap/src/nsImapServerResponseParser.cpp#1404
...is setting mailboxName to NULL. It'll do this if (and only if) the server sent "NIL" as the address mailbox.
(see CreateNilString())
It looks like it's an actual bug in TB because RFC3501 specifically says that addr-mailbox can be "NIL".
So we should be checking for NIL and dealing with it.

I'll try and work up a fix, the main issue then is getting it to Jerry to try out. I guess a nightly build? Or is there some procedure for backporting patches to particular versions? (it should be an easy change to backport, given how little that code has changed!)

Comment 10

[Jerry Raack]

Hi Ben,
Thanks for taking a look at my crash reports. Here is some additional information for you:

1. My laptop is running Thunderbird version 60.9.1 and has its own profile, which does not include the immense local files I have messages stored in on my main computer.
2. I tried opening Thunderbird 68.4.1 on my main computer in "offline mode", and it DOES open and remain stable. I can read all my locally stored files, and all of my emails in my inbox, sent mail folders, etc.. Of course, in "Offline" mode I can not get more recent emails, nor can I send emails. That is to be expected. The last email I received in my inbox on my main PC was at 8:28AM on 1/15/2020.

I am more than happy to try any nightly build or any other things you want me to try. While I have written my share of code in my earlier years, I admit I feel pretty helpless about my present predicament due to the fact that I did not participate in writing Thunderbird and don't know the underlying logic/design. I did write parts of the early C compiler while working for AT&T Bell Labs/Lucent Technologies, as well as various assemblers, linking loaders, etc.. So, fire away if you want me to try something out for you, or if you need more info.

Lastly, thanks to all of you at Mozilla/Thunderbird for what you have created and for your continued help. It is MUCH appreciated on a daily basis!
Jerry

Comment 11

[Ben Campbell]

Ahh... I think the offending code is only run when the server sends an ENVELOPE data item. And the server only sends this in response to a FETCH command from the client which requests "ENVELOPE" in it's fetch attributes. And TB never does this by default.

There is an mail.imap.use_envelope_cmd prefs setting which turns on ENVELOPE requesting, so I bet that's set to true for Jerry's case.
So, a quick workaround might be to look for this setting and turn it to false if it's set.
Jerry: want to give it a try? (I think it'll be in prefs.js in your profile dir). Might also be worth checking on the laptop - I'd guess it won't be set there.

This would account for the fact that we haven't seen this crash all over the place. I think it's a combination of having this pref set plus having a message in the account which uses RFC-2822 group syntax in one of it's envelope fields...
Doesn't account for why it's just crashing after the upgrade to 68.4.1... but maybe that's a freak coincidence (a change on the server side, a particular email arriving, sunspots...)

In any case it's still a bug to fix. We should be able to cope with ENVELOPE data items properly (perhaps even if we don't ask for them - not all servers are well behaved ;-)

Comment 12

[Ben Campbell]

Attachment: 1609690-imap-envelope-crash-1.patch

Here's a patch which I think will prevent this crash.
It doesn't fix the problem I think there is with parsing group addresses from some servers (which I've written up in Bug 1609846), but I don't think anyone will notice in practice. It's a pretty obscure set of circumstances, and I'd say it could wait until someone decides to do a complete IMAP rewrite...

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 9121452 [details] [diff] [review]
1609690-imap-envelope-crash-1.patch

Review of attachment 9121452 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good thx! r=mkmelin

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 9121452 [details] [diff] [review]
1609690-imap-envelope-crash-1.patch

Review of attachment 9121452 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/imap/src/nsImapServerResponseParser.cpp
@@ +1414,4 @@
>          if (hostName) {
>            addressLine += '@';
>            addressLine += hostName;
>            free(hostName);

I see mailboxName is not freed the same way. Are we leaking?

Comment 15

[Arnaud]

Hi.
Just wanted to tell you that I have the exact same bug since upgrading to 68.4.1.
If that matters, IMAP server is Zimbra ZCS 7 (latest minor revision).
I did have mail.imap.use_envelope_cmd set to true (no idea why), so I simply removed this line. So far so good, no crash for the last 30 minutes (it happened very often otherwise).

Comment 16

[Jerry Raack]

OK, I found the variable mail.imap.use_envelope_cmd in my prefs.js file in profile, and it WAS set to "true". So, I changed it to "false", saved the file back and invoked Thunderbird 68.4.1. And IT WORKS NOW! So, that either "fixed" the problem, or "masked" the problem. Either way, I can now use this version of Thunderbird on my main PC.
THANKS EVERYONE! Hope the next version fixes the problem.

Comment 17

[Wayne Mery (:wsmwk)]

Doesn't explain why Jerry only hit it after upgrading to 68.4.1... perhaps thunderbird identifies itself to the IMAP server slightly differently, and causes the server to alter the responses it sends? Sunspots? Just me grasping at straws there ;-)

Gene, maybe not a big deal, but does any change come readily to mind?

Comment 18

[Ben Campbell]

Attachment: 1609690-imap-envelope-crash-2.patch

Magnus: you're right about the missing PR_Free(). This patch adds it in (and also another place with a free() which should have been PR_Free()).

Comment 19

[gene smith]

(In reply to Wayne Mery (:wsmwk) from comment #17)

Doesn't explain why Jerry only hit it after upgrading to 68.4.1... perhaps thunderbird identifies itself to the IMAP server slightly differently, and causes the server to alter the responses it sends? Sunspots? Just me grasping at straws there ;-)

Gene, maybe not a big deal, but does any change come readily to mind?

I guess the reporter has probably been using the same profile for over 10 years back when use_envelope_cmd actually existed in the config editor and defaulted to true, per bug 402594 comment 38. Or maybe it has always been a completely hidden but user definable pref?. Anyhow, no I don't know of anything specific that would cause this.

I did try defining and setting it and when I repair a folder with 17k messages, tb trunk crashes. Based on gdb backtrace the crash wasn't in imap code. I did notice that with ENVELOPE in the header fetches, the amount of data retrieved was much more than with the default header fetch. This was to a my local dovecot server. The same folder repairs OK with mail.imap.use_envelope_cmd undefined.

I would suggest just changing the code so that the ENVELOPE thing is never requested regardless of the pref setting, as currently tb does by default. (I'll duplicate the crash again tomorrow and provide more info if there is interest.)

Edit: My crash and debug info are here: bug 1609846 comment 1

Comment 20

[Ben Campbell]

Are there still some cases where mail.imap.use_envelope_cmd=true is needed?
In Bug 402426 there's some reference to it being required for gmail, but I'd guess that's no longer a motivation given that the vast bulk of users will have it set to false.
Looks like the pref was added in Bug 42863 (20 years ago) defaulting to true, then changed to default to false instead (19 years ago)...

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c3804bf34462
Fix address-parsing crash on some IMAP servers when mail.imap.use_envelope_cmd is set. r=mkmelin DONTBUILD

Comment 22

[gene smith]

(In reply to Ben Campbell from comment #20)

Are there still some cases where mail.imap.use_envelope_cmd=true is needed?
In Bug 402426 there's some reference to it being required for gmail, but I'd guess that's no longer a motivation given that the vast bulk of users will have it set to false.

Maybe, but I haven't run into any after several year looking at imap stuff. This is first time I ever heard of imap ENVELOPE fetch request. New users or clean profiles never send out ENVELOPE header fetches.

Looks like the pref was added in Bug 42863 (20 years ago) defaulting to true, then changed to default to false instead (19 years ago)...

I think originally NS mail client (whatever it was called) always sent out ENVELOPE hard coded. Then in Bug 42863 they defined the truly hidden pref set to false which caused client to not send ENVELOPE unless a user created the pref and set it true. Maybe it is still needed for correct gmail chat listing (Bug 402426) but that's also something I've never used or seen, assuming gmail chat still exists. Also, I see it referenced to fix non-ascii chars display in gmail headers. So I guess keeping it for very special cases is OK.

Comment 23

[Geoff Lankow (:darktrojan)]

68.4.2:
https://hg.mozilla.org/releases/comm-esr68/rev/72e751b740f18dcceb4f3ed53849129929be24c7

Comment 24

[Wayne Mery (:wsmwk)]

linux signature might be libc-2.30.so@0x161d05 | nsTSubstring<T>::Append bp-2512f36c-01fe-4f8e-9330-42ce80200123
but I found only one from Gene testing Bug 1609846 IMAP ENVELOPE response parsing doesn't correctly

Comment 25

[Rob Lemley [:rjl]]

Thunderbird 73.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/412d38997e6342a26a1cdc83fc2a0d46058cad49