383460 - Fetch headers only with TOP only is all broken

Status: RESOLVED FIXED

(MailNews Core :: Networking: POP, defect)

Description

[Christian Eyrich]

If UIDL or XTND XLST aren't supported we fall back to using TOP to fake UIDL - but this really doesn't work.

Have some headers only messages in your inbox. Then put one more message on the server and get messages again.
Result is that only the new messages header will be in your inbox afterwards. The popstate.dat only contains one line for that message.

Reason is that TOP traverses backwards up to the first known UID and stops. Upon preparing download in nsPop3Protocol::GetMsg() only that UID will be put into the new hash table.

Slightly different is the result in the following scenario:
Have some headers only messages in your inbox. Then delete one (this gets marked d in the popstate.dat). Then put one more message on the server and get messages again.
Result is that all messages headers except the first one will be in your inbox afterwards. The popstate.dat only contains two lines, one for the new message and one with only a timestamp but without UID.

Reason is that TOP traverses backwards up to and including the first UID. But only the UID of the new message will be filled into msg_info. Preparing download in nsPop3Protocol::GetMsg() starts with message no 2 and every message gets downloaded but all the empty UID of the know messages get hashed into the same entry of the new hash table. This results in msg 1 gets removed from the inbox and popstate will hold one line without UID.

We could get rid of all that problems if always TOPing all messages and filling their UID in the array. That's no problem for 1, 5 or 10 messages but will generate havy traffic for more messages, especially with big headers.

There might be another solution but I don't see it right now. And ideas?

Comment 1

[Christian Eyrich]

Hm, didn't look into full body messages in conjunction with "leave on server" before but now I see it's also affected. Already downloaded messages get redownloaded (except the first one) if one is marked as to be deleted on the next fetch.

And like with headers only messages don't get deleted when it's marked as such though we do the whole back-traverse-up-to-msg-1 exactly for that reason (judging from a variable named delete_server_message_during_top_traversal).

Comment 2

[David :Bienvenu]

Christian, I'm not sure I follow, but the normal UIDL code is confusing enough...is this problem similar to the problem I recently was trying to fix when a new msg retrieval errors out and we were writing out the new msg hash table to popstate.dat even though it only had some of the messages from the original popstate.dat hash table? In which case, perhaps the solution is to make sure the entries from the old hash table are added to the new msg hash table somehow...

Comment 3

[Christian Eyrich]

Uh glad glad I'm not the only one confused by the UIDL stuff. :)

I guess you mean bug 352998? I just found that by chance and saw you said you cc'ed me although you seemingly forgot to.
Yes, I guess it boils down to that the headers currently in the inbox aren't in the new hash (since we don't have their UID). Maybe you're right, copying the old hash to the new one before modifying it could help against trashing current headers and redownloading.

That wouldn't solve the inability to delete a message on the server when it (or its header) is deleted in the client.
In order to do that we've to note down the UID's while TOPing all messages in order to build a connection between the hash entry and the msg no on the server.
Question is if we want that traffic-wise.

I tinker with the idea of dropping support for any advanced feature without UIDL or XTND XLST. We have reports about problems for servers without all three commands but AFAIK not about the massive problems I described here. That make me suspect that servers either miss any optional commands or have at least UIDL and TOP but not one without the other.

Comment 4

[David :Bienvenu]

I'm ok with dropping support for the advanced features on servers that have limited capabilities.

I'm not really sure why we have two hash tables - if the new hash table is just for new messages, maybe it would have been simpler just to have one hash table and a flag on messages in the hash table saying whether they were new or not...

Comment 5

[Christian Eyrich]

By duplicating old to new hash before transaction I was able to fix the missbehaviour in my testcases. But since deleting and also detecting/replicating deletion of mails on the server (also if you deleted it via webmail or so) doesn't work without getting all UIDs I'll try dropping advanced feature support next.

Sorting out if one hash table wouldn't be sufficient would be interesting after that.

Comment 6

[Christian Eyrich]

Ok, support for TOP to fake UIDL was dropped by fix for bug 156998. So I count this bug as done and closing.