Closed Bug 238087 Opened 20 years ago Closed 20 years ago

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


(MailNews Core :: Networking: POP, defect)

Not set


(Not tracked)



(Reporter: ch.ey, Assigned: ch.ey)


(Whiteboard: fixed-aviary1.0)


(1 file, 1 obsolete file)

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

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?
Attached patch proposed patch (obsolete) — Splinter Review
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.
Assignee: sspitzer → ch.ey
Attachment #144464 - Flags: review?(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.
Attachment #144464 - Attachment is obsolete: true
Attachment #144464 - Flags: review?(bienvenu)
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?
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.
Attached patch patch v2Splinter Review
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 on attachment 145283 [details] [diff] [review]
patch v2

great, thx.
Attachment #145283 - Flags: superreview+
fix checked in, thx, Christian!
Closed: 20 years ago
Resolution: --- → FIXED
Whiteboard: fixed-aviary1.0
If this is fixed in Aviary should it be landed on the 1.7 branch?
Flags: blocking1.7.3?
Product: MailNews → Core
1.7.5 has shipped. Moving request to 1.7.6.
Flags: blocking1.7.5? → blocking1.7.6?
closing down 1.7.6 to try and get it shipped.  -minus
Flags: blocking1.7.6? → blocking1.7.6-
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?
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.