238087 - Message ID not recognized on folded MESSAGE-ID: lines in TOP response

Status: RESOLVED FIXED

(MailNews Core :: Networking: POP, defect)

Description

[Christian Eyrich]

As fallback for UIDL and XTND XLST we use the TOP response to find the
MESSAGE-ID: header line and parse it. But we fail to find the message ID on a
folded line like
Message-ID: 
<[e-mail]>

The function should be changed to parse the line correctly. That's point one.

Point two is, that an empty message_id_token stops us from look for further
(resp. earlier as we go backwards trough the list) messages. This should be
changed as well, e.g. if no Message-ID: line is in the header or it's empty.
We could treat it as unknown anyway, or ignore it. Whatever, the user won't like
it, but what else can we do?

Comment 1

[Christian Eyrich]

Attachment: proposed patch

This patch enables parsing of multi-line Message-Id: headers. Mails without
Message-Id is treated as unknown.
To this the number of really_new_messages displayed in the mailnews status line
is now correct in the FakeUidlTop case.
The changes are commented in detail.

Comment 2

[David :Bienvenu]

seems good, thx for doing this! - can you change m_seenMidBefore to
m_parsingMultiLineMessageId or something like that? It's not obvious that Mid
means message-id...

>+      // msg_info[i].uidl when coming from GetFakeUidlTop() -
>+      // cause that would result in incorrect really_new_messages
cause -> because

>+// This function not really fakes UIDL but scans message headers
>+// starting with the last message backwards until all done or
>+// reaching a known one.

This comment needs work. It's difficult because this function is very confusing
:-) I'd suggest something like:

// This function is used when we have a server that doesn't support UIDL, and we
// send the TOP command to use the MESSAGE-ID header as a replacement for 
// the UIDL. It parses the the data from the TOP command, looking
// for the MESSAGE-ID: header, and then determines the next state to go to 
// in the state machine.
// 

r=bienvenu with those changes, if they seem OK.

Comment 3

[Christian Eyrich]

Remarks are accepted. What I just noticed, becauseo of the change in the first
line of SendFakeUidlTop() this patch needs to be checked in together with the
one from bug 226669. Is this ok, or should I move this change to that other patch?

Comment 4

[David :Bienvenu]

yes, I noticed that and meant to mention it. You'll just have to attach a
cumulative patch (or e-mail me one) when it comes time to checkin.

Comment 5

[Christian Eyrich]

Attachment: patch v2

Ok, I'll mail you then anyway to give you the -u patch.

So this patch addresses the few notes.
It also reactivates eatMessageIdToken in GetXtndXlstMsgid(), see my comment
from bug 226669#c14.

Comment 6

[David :Bienvenu]

Comment on attachment 145283 [details] [diff] [review]
patch v2

great, thx.

Comment 7

[David :Bienvenu]

fix checked in, thx, Christian!

Comment 8

[Daniel Veditz [:dveditz]]

If this is fixed in Aviary should it be landed on the 1.7 branch?

Comment 9

[Asa Dotzler [:asa]]

1.7.5 has shipped. Moving request to 1.7.6.

Comment 10

[chris hofmann]

closing down 1.7.6 to try and get it shipped.  -minus

Comment 11

[Daniel Veditz [:dveditz]]

a- for 1.7.6, not in the scope of a security release. But if this was checked-in
in April and 1.0 branched from 1.7 in May isn't this already on the 1.7 branch?