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

RESOLVED FIXED in mozilla1.8alpha2

Status

MailNews Core
Networking: IMAP
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: Lorenzo Colitti, Assigned: Lorenzo Colitti)

Tracking

Trunk
mozilla1.8alpha2
x86
Windows XP
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-aviary1.0)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

14 years ago
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
(Assignee)

Comment 1

14 years ago
Created attachment 149333 [details]
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.
(Assignee)

Comment 2

14 years ago
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.
(Assignee)

Comment 3

14 years ago
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

14 years ago
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

(Assignee)

Comment 5

14 years ago
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.
(Assignee)

Comment 6

14 years ago
Taking bug. Patch coming up.
Assignee: bienvenu → lorenzo
(Assignee)

Comment 7

14 years ago
Created attachment 149627 [details] [diff] [review]
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...
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 8

14 years ago
Comment on attachment 149627 [details] [diff] [review]
patch v1

mscott, can you take a quick look at this? Only 2 lines of code. :)
Attachment #149627 - Flags: review?(mscott)

Comment 9

14 years ago
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.
Attachment #149627 - Flags: review?(mscott) → review+

Comment 10

14 years ago
Let's see how this fix does in the real world.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Whiteboard: fixed-aviary1.0
Target Milestone: --- → mozilla1.8alpha2

Comment 11

14 years ago
not so well, i think
Blocks: 245233

Comment 12

14 years ago
re-opening. I backed this out since it introduced a crash.
Status: RESOLVED → REOPENED
Keywords: regression
Resolution: FIXED → ---
Whiteboard: fixed-aviary1.0
(Assignee)

Comment 13

14 years ago
Created attachment 149844 [details] [diff] [review]
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.
Attachment #149627 - Attachment is obsolete: true

Comment 14

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

I'll check this in
Attachment #149844 - Flags: superreview+

Comment 15

14 years ago
let's try again.
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago14 years ago
Resolution: --- → FIXED
Whiteboard: fixed-aviary1.0
(Assignee)

Updated

14 years ago
Blocks: 235125
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.