128470 - Importing mail from outlook XP results in crash

Status: VERIFIED DUPLICATE of bug 144508

(MailNews Core :: Import, defect)

Description

[Vincent]

I did the following:
- Make outlook XP the default mail client
- Close outlook
- Import from outlook

After the progress-bar is at about 20%, mozilla.exe crashes. WinXP gives the
following feedback:
Appname: mozilla.exe
AppVer: 0.0.0.0
ModName: ntdll.dll
ModVer: 5.1.2600.0
Offset: 00017f26

There's a lot more info given by Windows XP, but I cannot simply copy-paste this
information (thank you Gates, for being helpfull). If really desired, I can copy
it by hand.

extra info: I have a lot of e-mail in my inbox and even more in a backup-folder.
I tested it in Mozilla 0.9.8 and in the latest nightly build.

Comment 1

[Roger W Turner]

0.9.8 crashed reproducibly importing mail from outlook express 6 on win98se.
However updating to build 2002030803 fixed it.

Comment 2

[Ninoschka Baca]

Marking Worksforme per reporters last statement.

Comment 3

[Ninoschka Baca]

Verified Worksforme.

Comment 4

[Vincent]

I know it works for a lot of other machines, but I have WinXP, outlook XP and
more than 1000 e-mails per box. Bet you have or a non-winXP-machine or just a
few e-mails in your boxes.

Comment 5

[Victor Wong]

I also have experienced this crash.  I had a talkback build so I submitted a 
report.  Basically I was importing mail from Outlook 2002 using build 0.9.9 in 
Windows XP.  My Outlook folders contained 20,000+ messages each.  I just 
downloaded the nightly build and also experienced the same crash.

Comment 6

[Derek N Slone]

Be kind... this is my first ever contribution to Mozilla.

I was getting a protection exception under Win2k, importing from Outlook 98 
(lots of big PSTs) using 1.0 RC 1, so went and downloaded the source, rebuilt, 
got the PDBs in the right places and repro'd the problem.  The root of the 
problem was the invocation of the following line in 
mozilla\mailnews\compose\src\nsMsgSend.cpp

    body_is_us_ascii = nsMsgI18N7bit_data_part(mCompFields->GetCharacterSet(), 
m_attachment1_body, m_attachment1_body_length);

This line was being invoked with m_attachment1_body == NULL & 
m_attachment1_body_length == 0x3c1.  The rest of "this" was:

0:000> dt this
Local var @ 0x12c980 Type nsMsgComposeAndSend*
0x04e4b5b0
   +0x000 __VFN_table : 0x0495f390
   +0x004 m_attachments_done_callback : (null) 
   +0x008 mRefCnt          : 4
   +0x00c _mOwningThread   : 0x04be8d48
   +0x010 m_messageKey     : 0xffffffff
   +0x014 mUserIdentity    : nsCOMPtr<nsIMsgIdentity>
   +0x018 mCompFields      : nsCOMPtr<nsMsgCompFields>
   +0x01c mTempFileSpec    : 0x05255fe8
   +0x020 mOutputFile      : 0x05240400
   +0x024 mMessageWarningSize : 0
   +0x028 m_dont_deliver_p : 1
   +0x02c m_deliver_mode   : 4
   +0x030 mMsgToReplace    : nsCOMPtr<nsIMsgDBHdr>
   +0x034 mParentWindow    : nsCOMPtr<nsIDOMWindowInternal>
   +0x038 mSendProgress    : nsCOMPtr<nsIMsgProgress>
   +0x03c mListener        : nsCOMPtr<nsIMsgSendListener>
   +0x040 mStatusFeedback  : nsCOMPtr<nsIMsgStatusFeedback>
   +0x044 mRunningRequest  : nsCOMPtr<nsIRequest>
   +0x048 mSendMailAlso    : 0
   +0x04c mReturnFileSpec  : 0x052305c0
   +0x050 mHTMLFileSpec    : (null) 
   +0x054 m_folderName     : nsCString
   +0x064 mCopyFileSpec    : (null) 
   +0x068 mCopyFileSpec2   : (null) 
   +0x06c mCopyObj         : (null) 
   +0x070 mNeedToPerformSecondFCC : 0
   +0x074 mEditor          : (null) 
   +0x078 m_attachment1_type : 0x04fe3c00 "text/plain"
   +0x07c m_attachment1_encoding : (null) 
   +0x080 m_attachment1_encoder_data : (null) 
   +0x084 m_attachment1_body : (null) 
   +0x088 m_attachment1_body_length : 0x3c1
   +0x08c mOriginalHTMLBody : (null) 
   +0x090 m_plaintext      : (null) 
   +0x094 m_related_part   : (null) 
   +0x098 m_related_body_part : (null) 
   +0x09c m_attachment_count : 1
   +0x0a0 m_attachment_pending_count : 0
   +0x0a4 m_attachments    : 0x0514f03c
   +0x0a8 m_status         : 0
   +0x0ac mPreloadedAttachmentCount : 1
   +0x0b0 mRemoteAttachmentCount : 0
   +0x0b4 mMultipartRelatedAttachmentCount : 0
   +0x0b8 mCompFieldLocalAttachments : 0
   +0x0bc mCompFieldRemoteAttachments : 0
   +0x0c0 m_attachments_only_p : 0
   +0x0c4 m_pre_snarfed_attachments_p : 1
   +0x0c8 m_digest_p       : 0
   +0x0cc m_be_synchronous_p : 0
   +0x0d0 mGUINotificationEnabled : 1
   +0x0d4 mLastErrorReported : 0
   +0x0d8 mAbortInProcess  : 0
   +0x0dc m_crypto_closure : nsCOMPtr<nsIMsgComposeSecure>
   +0x0e0 mComposeBundle   : nsCOMPtr<nsIMsgStringService>
   +0x0e4 mSendReport      : nsCOMPtr<nsIMsgSendReport>
   +0x0e8 mSmtpPassword    : nsCString

I think that this essentually boils down the body_is_us_ascii test not checking 
for m_attachment1_body being non-NULL.  However, that probably only causes 
problems on "odd" character encodings such as "ISO-2022-JP", which was the 
charset of the particular document that was being imported.  Looking at the 
temp document left in my temp folder, it was indeed using ECS-$ and ESC-(, 
which is either UEC or Shift-JIS encoding - can't recall which.

However, I also spotted two other things in passing which I believe should be 
fixed:

1. SnarfAndCopyBody does not call EnsureLineBreaks if the passed in 
attachment1 [details] [diff] [review]_body in NULL (which it was) which results in both 
m_attachment1_body and m_attachment1_body_length being left with whatever 
values they had last time.

2. nsMsgComposeAndSend::Clear() frees off m_attachment1_body but doesn't set 
m_attachment1_body_length back to NULL.

I'll attach a -C5 patch for the fixes I applied.

Comment 7

[Derek N Slone]

Attachment: cvs diff -C5 of my proposed fix

Comment 8

[Scott MacGregor]

Derek, your code patch is in mailnews compose when we send a message. It looked
to me like this bug was reporting a crash during the import process which
wouldn't have anything to do with sending. Do we think you are experiencing
another crash instead of the one reported here? 

Comment 9

[Derek N Slone]

Attachment: Dlg screen shot + info from WinDbg

The bug does occure during the import from Outlook.  I to was initially
surprised when I found this on the call-stack!	I then started to hypothesise
that perhaps the import facility design was to create a new mail item, file it,

and NOT send it.  That's my best guess.

However, I can definitely repro this, on import, with no sending taking place. 

Even if it was some background thing that had decided to tap into Mozilla to
send something, I should not have been able to reproduce the problem with the
regulatiry that I could.

I've attached my Bug file, which includs the stack trace from WinDbg.  Although
I feel it doesn't shed much light on the problem :-(  (Open it in WordPad with
word-wrap off for best display (certainly NOT Word!))

Comment 10

[Scott MacGregor]

That is weird. I'm going to send this over to Cavin who has been working on
eudora import issues. He may also know why we appear to be going through the
send code.

About your patch, we tend to frown on goto statements. Any chance you can change
your patch instead of:

+   if ( !m_attachment1_body )
+       goto Assume7Bit;
     do non assume 7 bit stuff

to
if (m_attachment1_body)
{
  do non assume 7 bit stuff
}

Comment 11

[Scott MacGregor]

adding crash keyword while I'm here.

Comment 12

[Cavin Song]

Yes, the import code dose invoke compose code (CreateAndSendMessage()) to create 
a msg with headers and body (and attachments) in a temp file.

Derek, you probably have tested your patch (against your Outlook folders) to 
make sure it fixes the import crash problem. But is it possible for you to email 
me a sample Outlook .pst which can reproduce the problem? That way, when I check 
in the fix for you later I can be sure the problem is fixed. Thanks.

Other than the issue pointed out by mscott, your patch should fix the problems 
you listed in comment #6.

Ccing ducarroz since he's the owner of the compose module.

Comment 13

[Jean-Francois Ducarroz]

same problem than bug 144508. THe crash occurs because the message doesn't have
a body!

*** This bug has been marked as a duplicate of 144508 ***

Comment 14

[Derek N Slone]

Caveat its an empty body with multi-byte encoding.

So much for my efforts.  I had a fix for bug 144508 2-weeks before it was opened
- but the patch that's attached to 144508 appears does appear to address the
same issue that I found.

I did rather hijack this bug though; we should probably confirm that Victor's
and Vincent's problems have also been resolved by the same fix - they could
easily have had a different underlying cause.

Comment 15

[Cavin Song]

> we should probably confirm that Victor's and Vincent's problems have also been
> resolved by the same fix
>
I agree. So Victor and Vincent, can you try the latest build and see if your
problem goes away by this fix? Thanks.

Comment 16

[Vincent]

Sorry, I cannot reproduce. ;-(

Comment 17

[Ninoschka Baca]

Verified duplicate.