244722 - Message truncated in headers stops mail from automatically moving all junk messages

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Lorenzo Colitti]

Often my IMAP server contains weird broken emails that do not have a new line
after the message headers (see attached eml file). I get about two or three of
these per day.

I have junk filtering set to automatically move messages to the junk folder.

When I open my inbox and one of these messages is lurking inside, spam messages
are classified as junk but are not moved into the junk folder automatically.

This used to work before the junk mail rewrite in 0.5.

To reproduce this:
0. Set the junk mail filter to automatically move messages to the junk folder
1. Drop the .eml file into your IMAP server (or send it to yourself)
2. Open your inbox
3. Wait for some spam to arrive
4. Observe that the spam messages are not sent to the junk folder

Comment 1

[Lorenzo Colitti]

Attachment: testcase malformed email

This email has been doctored to remove my address and provider, but other than
that it's an actual message I got from my IMAP server. Note that there is no
empty line after the headers.

Comment 2

[Lorenzo Colitti]

This is what is happens when the bad message arrives:

1. nsImapMailFolder::SpamFilterClassifyMessages() is called
2. nsImapMailFolder::m_numFilterClassifyRequests is incremented
3. The weird message causes something to fail and mimeEmitterEndHeader(), which
   would call the junk mail filter, is never called
4. The junk mail filter never sees the classification request. That is,
   TokenStreamListener::ProcessHeaders (in nsBayesianFilter.cpp) is never
   called

Then, when a spam message arrives:

1. nsImapMailFolder::SpamFilterClassifyMessages() is called
2. nsImapMailFolder::m_numFilterClassifyRequests is incremented
3. The junk mail filter gets the classification request and classifies the
   message as junk.
3. nsImapMailFolder::OnMessageClassified() is called with aClassification = JUNK
4. Since the previous classification request (for the bad message) never
   completed, m_numFilterClassifyRequests = 2
5. Since --m_numFilterClassifyRequests != 0, the messages are not moved

No further spam will be moved until the app is restarted.

Comment 3

[Lorenzo Colitti]

I realize this is not a major concern since it probably only happens to me. I'm
willing to fix it, since it hits me hard, but I don't know where to act on this.
What is failing here is the MIME code, but probably touching that code is not a
good idea since a lot of stuff depends on it.

Ideas?

CCing mscott since he did the "turn junk mail plugins into nsIMsgHeaderSinks" work.

Comment 4

[Scott MacGregor]

If you are already walking through the debugger then what would really help
would be to identify the part of libmime that fails. 

Start stepping from here (if it is called):

http://lxr.mozilla.org/mozilla/source/mailnews/mime/src/mimemsg.cpp#668

Comment 5

[Lorenzo Colitti]

With one of these malformed messages, MimeMessage_write_headers_html is never
called. This is what I see from an (unfortunately) non-debug build:

#0  0x612092f9 in MimeHeaders_parse_line(char const*, int, MimeHeaders*) ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/mail.dll
#1  0x6120ff69 in MimeMessage_parse_line(char*, int, MimeObject*) ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/mail.dll
#2  0x61219d12 in convert_and_send_buffer(char*, int, int, int (*)(char*,
unsigned, void*), void*) ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/mail.dll
#3  0x61219e69 in mime_LineBuffer ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/mail.dll
#4  0x61212c01 in MimeObject_parse_buffer(char const*, int, MimeObject*) ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/mail.dll
#5  0x6121b843 in mime_display_stream_write ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/mail.dll
#6  0x61224921 in
_ZN17nsStreamConverter15OnDataAvailableEP10nsIRequestP11nsISupportsP14nsIInputStreamjj@24
()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/mail.dll
#7  0x01010431 in
_ZN19nsStreamListenerTee15OnDataAvailableEP10nsIRequestP11nsISupportsP14nsIInputStreamjj@24
()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/necko.dll
#8  0x00ff68cd in _ZN23nsOnDataAvailableEvent011HandleEventEv@4 ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/necko.dll
#9  0x00ff6274 in nsStreamListenerEvent0::HandlePLEvent(PLEvent*) ()
   from /cygdrive/e/Mozilla/tb/mozilla/dist/bin/components/necko.dll
#10 0x677fc778 in PL_HandleEvent ()

I'll start moving up the stack to see what I can find.

Comment 6

[Lorenzo Colitti]

Taking bug. Patch coming up.

Comment 7

[Lorenzo Colitti]

Attachment: patch v1

Change MimeMessage_parse_eof so that if the headers have not been parsed by the
end of the message it will hack a newline on to the end of the message and try
again.

This is justified by the fact that a message without any empty lines is broken
anyway...

Comment 8

[Lorenzo Colitti]

Comment on attachment 149627 [details] [diff] [review]
patch v1

mscott, can you take a quick look at this? Only 2 lines of code. :)

Comment 9

[Scott MacGregor]

Comment on attachment 149627 [details] [diff] [review]
patch v1

To be honest, I don't know what the ramifications of this change will be :).
I'll check it into 1.8a2 and we can see what happens.

This has the potential to be a *great* fix functionality wise (we'll find out
about the correctness) because I know this junk mail failure case is quite
common and I've never been able to track down why some messages aren't moved.

Comment 10

[Scott MacGregor]

Let's see how this fix does in the real world.

Comment 11

[timeless]

not so well, i think

Comment 12

[Scott MacGregor]

re-opening. I backed this out since it introduced a crash.

Comment 13

[Lorenzo Colitti]

Attachment: patch that fixes the crash in bug 245233

Same patch as above, except with an extra null check to fix the crash in bug
245233.

Comment 14

[Scott MacGregor]

Comment on attachment 149844 [details] [diff] [review]
patch that fixes the crash in bug 245233

I'll check this in

Comment 15

[Scott MacGregor]

let's try again.