239583 - mail lost when getting fetchmailed messages

Status: RESOLVED FIXED

(MailNews Core :: Movemail, defect)

Description

[kirill]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040312 Debian/1.6-3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040312 Debian/1.6-3

I am using X11; U; Linux i686; en-US; rv:1.6) Gecko/20040312 Debian/1.6-3 and I
saw today Mozilla losing email message. 
I am using fetchmail to deliver the mail from 2 POP3 accounts into the single
localhost account. I've got 2 emails which were logged in exim delivery logs.
When I checked the account in Mozilla mail it showed only one new message. The
message shown was in Cyrillic Win1251 encoding (HTML body and subject line). The
lost message was (probably) in Russian KOI-8R: subject line in Cyrillic with
base64 plain text body text. To add insult to injury the delivered message
happen to be a junk message.

The egrep of sender address in ~/.mozilla did not return any matches.
I am using fetchmail 5.9.11+NTLM+SDPS+NLS and Exim version 3.36 on 2.6.4 kernel.

Reproducible: Couldn't Reproduce
Steps to Reproduce:
1.
2.
3.

Comment 1

[kirill]

Attachment: Spool file with 5 test messages

Comment 2

[kirill]

The problem is reproducable with 
- 1.7b;
- nightly (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040405)

With nightly the ~/.mozilla was moved out of the way before test.

To reproduce the problem using "Spool file ..." attached:
- create new Movemail account in Mozilla using all defaults;
- decompress the file and place it into /var/spool/mail under corresponding user
name;
- click "GetMsgs" in Mozilla

Mozilla here shows single new message in Inbox.

mailx shows 5 messages: 4 messages with Subject lines in Win1251 encoding and
last message with Unicode subject line (the one which is actually shown by Mozilla)

Comment 3

[Philip K. Warren]

I can confirm that this is occurring on a trunk build from today.

Comment 4

[Philip K. Warren]

I noticed that there are embedded null characters contained within the sample
spool file - these have caused problems with Mozilla/Movemail in the past. I'm
trying to track down the bug where this problem was supposedly fixed.

Comment 5

[Philip K. Warren]

bienvenu: Didn't you work on a bug dealing with embedded null characters before?

Comment 6

[David :Bienvenu]

yes, I did, but for pop3 and imap, not for movemail. The movemail code probably
needs a similar kind of fix.

Comment 7

[Philip K. Warren]

I believe this may be a duplicate of Bug 115179.

Comment 8

[Philip K. Warren]

I have a patch I'm working on which uses dougt's suggestion from Bug 115179
(http://bugzilla.mozilla.org/show_bug.cgi?id=115179#c11). In early testing it
appears to fix the issues with null characters.

Comment 9

[Philip K. Warren]

Attachment: Patch v1 (cvs diff -uw8)

This patch does a lot of cleanup for the Movemail code, including switching it
to use the nsILineInputStream interface for reading the file, which fixes the
errors with reading null characters in the mail spool files.

Summary of changes:
- Where possible, replaces usage of nsFileSpec with nsILocalFile.
- Splits off the finding of the mail spool file into a separate function for
readability.
- Switches from using nsInputFileStream for parsing of the file to
nsIInputStream/nsILineInputStream.
- Removes allocation of the buffer used in prior reading of file, uses nsRefPtr
for automatic deletion of newMailParser, and moves the locking of the mail
spool until right before it is read. This allows the removal of the
"freebuff_and_unlock" goto.
- Uses nsIFile's SetFileSize instead of nsFileSpec's Truncate function for
truncating the mail spool.

I have tested this code thoroughly on the provided mail spool and movemail is
now working properly with this fix.

Comment 10

[Philip K. Warren]

Comment on attachment 145717 [details] [diff] [review]
Patch v1 (cvs diff -uw8)

Seth - can you please look over these changes?

Comment 11

[Scott MacGregor]

Comment on attachment 145717 [details] [diff] [review]
Patch v1 (cvs diff -uw8)

Seth's busy doing non moz stuff these days. Taking over and sr'ing the patch.

I'm happy to have an active module owner for movemail. Go for it Philip.

I'm trusting you to of course test these changes. I don't have the ability to.

Comment 12

[Philip K. Warren]

Should this change wait for 1.8a or should we try and get this in for 1.7? I am 
not sure how often mail spools have null characters embedded in them, but this 
is crucial in those situations to prevent the loss of mail.

Comment 13

[David :Bienvenu]

Comment on attachment 145717 [details] [diff] [review]
Patch v1 (cvs diff -uw8)

I think we should try for 1.7 since it does cause data loss.

Comment 14

[chris hofmann]

Comment on attachment 145717 [details] [diff] [review]
Patch v1 (cvs diff -uw8)

a=chofmann for 1.7

Comment 15

[Philip K. Warren]

Fixed.

Checking in nsMovemailService.cpp;
/cvsroot/mozilla/mailnews/local/src/nsMovemailService.cpp,v  <-- 
nsMovemailService.cpp
new revision: 1.42; previous revision: 1.41
done

Comment 16

[Mike Cowperthwaite]

Comment on attachment 145717 [details] [diff] [review]
Patch v1 (cvs diff -uw8)

Bug 260488 appears to be a TB filing of the same bug -- I'm assuming this
hasn't made it into aviary yet...?

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 145717 [details] [diff] [review]
Patch v1 (cvs diff -uw8)

a=asa for aviary checkin.

Comment 18

[David :Bienvenu]

I don't believe this is related to bug 260488 - this fix went in long before the
aviary branch was made, and is a regression between .7.3 and .8