156998 - TOP command may not be supported by POP3 server

Status: RESOLVED FIXED

(MailNews Core :: Networking: POP, defect)

Description

[Charles Durst]

My employer runs his own homebrewed POP3 server which implements the absolute,
bare minimum allowed by the POP3 RFC spec.

Mozilla [1.0 RC3 for Windows, Build ID: 2002052306] is able to connect to it and
log in, but then it attempts a command ("TOP", I believe) that is unimplemented
in this POP3 server.

The server reports the error as it is supposed to (with "ERR- unrecognized
command", or something like that).  Mozilla then drops the connection and acts,
from the user's point of view, as if it succeeded and found no mail -- even
though the POP3 server had reported 5 waiting messages to it before the
connection was dropped.

At the very least, Mozilla should report that an error occurred when downloading
the messages.

Better would be for it to have a fallback method that does not use the "TOP"
command for those few (or one) POP3 servers out there that don't support it.

I might be able to arrange to set up a POP account on the server in question for
testing.

Comment 1

[Matthias Versen [:Matti]]

Mozilla doesn't use "Top" on my pop3 server.
+0200 Connected
+OK server600 MERAK 3.00.120 POP3 Fri, 12 Jul 2002 00:19:55 +0200
<20020712001955@server600>
USER matti
+OK matti
PASS *********
 +OK 1 messages 813 octets
 STAT
+OK 1 813
LIST
+OK 1 messages 813 octets
UIDL
+OK
RETR 1
+OK 813 octets
DELE 1
+OK Message deleted
QUIT
+OK server600 closing connection

Reporter: Can you attach a pop3 log ?
(http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html)

Comment 2

[Matthias Versen [:Matti]]

Mozilla use the OPTIONAL "UIDL" which could be the problem.

Comment 3

[Charles Durst]

Attachment: POP3 Log

Here's a sample log.
One minor correction to my initial bug report: the error reply from the server
starts with '-ERR' rather than 'ERR-'.	That's what I get from writing a bug
from memory...

Comment 4

[Charles Durst]

Just to add a little motivation to fix this problem ... I'd like to point out
that MSFT Outlook and Outlook Express both work fine with this POP3 server, and
from what I've heard, Netscape doesn't.

Comment 5

[(not reading, please use [e-mail] instead)]

mass re-assign.

Comment 6

[Christian Eyrich]

The problem is, that want to find you if we know a message on the server and
only download it if not.
For this we try UIDL first, if it fails XTND XLST and if this fails TOP.
Actually you should get an error message, true. We should change the existing
UpdateStatusWithString(statusString) in nsPop3Protocol::GetFakeUidlTop() to 
Error(statusString).

The other thing is to get it work even without TOP. I don't know how Outlook
does it. At the moment I see two ways we can go:
1. Just download all the messages on the server completely and don't care if it
already is in the local mailbox.
2. Download any message on the server completely but look at the Message-ID:
line (as we currently do it with the TOP result) before writing it in the local
mailbox. If the message id is known, drop the message.

Comment 7

[David :Bienvenu]

if there's no TOP command, we just can't implement the feature of not
downloading messages > XX KB. And if UIDL isn't supported, we can't do leave on
server. But we can still download the messages and delete them from the server.

Comment 8

[Christian Eyrich]

David, for the "do not download messages > XX KB" Feature I agree.
But UDIL isn't essential for leave on server as we have the fallback to XTND
XLST and FakeUidlTop.
And we *could* implement leave on server also with RETR, if that's worth the
effort and trouble is another thing.

Anyway, we need an error message here. And at the moment getting mail isn't
possible even without leave messages on server.

Comment 9

[Christian Eyrich]

Attachment: proposed patch

This patch enables mail retrieval also if neither UIDL/XTND XLST nor TOP is
supported by the server at least if "leave messages on server" is disabled.

In the other case an error is displayed - this alert is the cause for the most
of the code. If anyone knows how to display it with less, I'm happy with it.

Comment 10

[David :Bienvenu]

Another way of handling this would be to create a new error code that gets
handled in nsMsgProtocol::OnStopRequest, which already knows how to put up
alerts. That would entail putting the error text in the base properties file,
however.

Another possibility is creating a helper routine in nsMsgProtocol.cpp, that
takes a string bundle or url, a string id, and a url (and whatever else is
needed), and does all this. That code could be shared. See also
FormatStringWithHostNameByID, which has similar functionality.

Comment 11

[Christian Eyrich]

Using FormatStringWithHostNameByID() together with GetPromptDialogFromUrl()
would be small and clear. This could be also used for nsPop3Protocol::Error().
But that also requires the alert to be in the base properties file. :-( No
workaround for this?

Comment 12

[timeless]

*** Bug 300529 has been marked as a duplicate of this bug. ***

Comment 13

[Juergen Ebert]

Today I made a little POP3 server myself and I am also stuck with this problem. Here is the log (> means received, < means sent):

<+OK POP3 server ready
>CAPA
<+OK Capability list follows
<USER
<.
>USER username
<+OK username is a valid mailbox
>PASS password
<+OK 14 messages (464709 octets)
>STAT
<+OK 14 464709
>LIST
<+OK 14 messages (464709 octets)
<1 2158
<2 4114
<3 428575
<4 1786
<5 1588
<6 2526
<7 2673
<8 4277
<9 2831
<10 3316
<11 2789
<12 3139
<13 2578
<14 2359
<.
>TOP 14 1
<-ERR Command not supported

As you can see the mail server states that is does not support the TOP command (CAPA). I did not mark "To save disk space, do not download: ..." in Thunderbird (1.5.0.5). TOP is optional and even only because of this Thunderbird must work without it.

Comment 14

[Yulia Shabunio]

I have same problem with Thunderbird 2.0.0.0 (20070326) with our corporative server. Options "Leave messages on server" and "Maximum Message Size" are disabled, but it does not work anyway.

Any other mail programs (Outlook, Outlook Express, The Bat!) work fine.

Slice of pop3 log:

0[274740]: Entering NET_ProcessPop3 19
0[274740]: POP3: Entering state: 1
0[274740]: POP3: Entering state: 2
0[274740]: POP3: Entering state: 4
0[274740]: RECV: +OK ewpop3d ready
0[274740]: POP3: Entering state: 29
0[274740]: SEND: AUTH

0[274740]: Entering NET_ProcessPop3 38
0[274740]: POP3: Entering state: 3
0[274740]: RECV: +OK List of available mech:
0[274740]: POP3: Entering state: 30
0[274740]: RECV: NTLM
0[274740]: POP3: Entering state: 30
0[274740]: RECV: .
0[274740]: POP3: Entering state: 31
0[274740]: SEND: CAPA

0[274740]: Entering NET_ProcessPop3 22
0[274740]: POP3: Entering state: 3
0[274740]: RECV: -ERR invalid command
0[274740]: POP3: Entering state: 32
0[274740]: POP3: Entering state: 33
0[274740]: POP3: Entering state: 5
0[274740]: SEND: USER ysinitsyna

0[274740]: Entering NET_ProcessPop3 37
0[274740]: POP3: Entering state: 3
0[274740]: RECV: +OK user selected, password please.
0[274740]: POP3: Entering state: 34
0[274740]: POP3: Entering state: 6
0[274740]: Logging suppressed for this command (it probably contained authentication information)
0[274740]: Entering NET_ProcessPop3 32
0[274740]: POP3: Entering state: 3
0[274740]: RECV: +OK 8 messages (164728 octets)
0[274740]: POP3: Entering state: 34
0[274740]: POP3: Entering state: 7
0[274740]: SEND: STAT

0[274740]: Entering NET_ProcessPop3 14
0[274740]: POP3: Entering state: 3
0[274740]: RECV: +OK 8 164728
0[274740]: POP3: Entering state: 8
0[274740]: POP3: Entering state: 9
0[274740]: SEND: LIST

0[274740]: Entering NET_ProcessPop3 128
0[274740]: POP3: Entering state: 3
0[274740]: RECV: +OK sending list ending with a '.' on a line by itself
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 1 15318
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 2 8451
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 3 13289
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 4 3183
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 5 6302
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 6 27357
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 7 48684
0[274740]: POP3: Entering state: 10
0[274740]: RECV: 8 42144
0[274740]: POP3: Entering state: 10
0[274740]: RECV: .
0[274740]: POP3: Entering state: 11
0[274740]: SEND: UIDL

0[274740]: Entering NET_ProcessPop3 22
0[274740]: POP3: Entering state: 3
0[274740]: RECV: -ERR invalid command
0[274740]: POP3: Entering state: 12
0[274740]: POP3: Entering state: 13
0[274740]: SEND: XTND XLST Message-Id

0[274740]: Entering NET_ProcessPop3 22
0[274740]: POP3: Entering state: 3
0[274740]: RECV: -ERR invalid command
0[274740]: POP3: Entering state: 14
0[274740]: POP3: Entering state: 26
0[274740]: SEND: TOP 8 1

0[274740]: Entering NET_ProcessPop3 22
0[274740]: POP3: Entering state: 3
0[274740]: RECV: -ERR invalid command
0[274740]: POP3: Entering state: 28
0[274740]: POP3: Entering state: 24
0[274740]: POP3: Entering state: 25

Comment 15

[Christian Eyrich]

New try.
Currently we're stopping retrieval if neither of UIDL, XTND XLST and TOP is available on the server regardless whether or not the user makes use of 'Keep Mail on Server', 'Maximum Message Size' or 'Headers only'. That's unnecessary restrictive and a bug.

What are theese commands needed for? At one hand one of those is needed to make each message on the server identifiable over sessions. This isn't needed to leave messages on the server or download them partially. But is needed in order to retrieve the rest of one particular message or to  not download over and over again the messges left on server.
Additionally the TOP command itself is needed to only partially download a message if a message is to big or 'Headers only' is active.

So if none of those features is needed we still can retrieve message without restrictions and don't have to issue a warning. I'm about to do that.
Questions I have are:
*If 'Maximum Message Size' is activated, shall we instantly throw an error upon noticing the lack of at least TOP. Or shall we download every not to big message and leave every to big message on the server without even downloading its header? How to make the user aware of this?

*If the server lacks every of the needed commands, the user should be notified. Upon every retrieval, once for each server, once in every session for each server?

Comment 16

[David :Bienvenu]

Thx for tackling this, Christian.


>If 'Maximum Message Size' is activated, shall we instantly throw an error upon
>noticing the lack of at least TOP

Yes - optionally we could ask the user if they want us to ignore and/or clear the max size limit, and if they say yes, download the messages.

>*If the server lacks every of the needed commands, the user should be notified.
>Upon every retrieval once for each server, once in every session for each
>server?
Not to complicate things, but we might want to treat errors on automatic background checks differently from explicit get new mail or get new mail on startup - I think maybe we should just notify in the latter cases. We're not going to make it so get new mails that used to work don't work anymore, right?

Comment 17

[Christian Eyrich]

Attachment: proposal 2

Ok, this patch is a mix of the former one in this bug and dropping support of TOP for advanced features I mentioned in bug 383460, comment 3.

The code to bring up the alert is still the old one and thus still uggly.

Comment 18

[David :Bienvenu]

Comment on attachment 267741 [details] [diff] [review]
proposal 2

this looks good in general, Christian, thx very much. It's nice to remove all that code.

I'd probably remove the negative comments about "some users" though :-)

Comment 19

[Christian Eyrich]

Attachment: alert text to be changed?

Along with my last patch, should the alert text be modified?
This proposal specifies in more detail what is needed to implement what.
And should we also give a hint to contact the server admin to get the required commands?

On the other hand my proposal is quite long now and would become even longer then.

Comment 20

[David :Bienvenu]

Comment on attachment 268077 [details] [diff] [review]
alert text to be changed?

It's long, but as long as it displays correctly, I have no problem with it. It should be rare, and it's a pretty serious error, so the more detail we can give, the better.

While you're there, you could correct the part about "Mail Server panel of Preferences". There's no such thing anymore - it should say refer to Server Settings in the Account Settings window. (there must be other places where we point users at that UI - you could copy what they say)

Comment 21

[Christian Eyrich]

Attachment: proposed patch v4

In localMsgs.properties there was different wording used. There and in other places it mostly simply speaks of "the Account Settings".
I've now rephrased in at least this file according to your example.

In the nsPop3Protocol.cpp I also did a little cleanup and added a few checks.

Comment 22

[Christian Eyrich]

Attachment: proposed patch v5

So this should be it. Changes to v4 were only a little cleanup and removal of unnecessary code in CommitState() and state POP3_INTERRUPTED.

Comment 23

[David :Bienvenu]

Comment on attachment 268802 [details] [diff] [review]
proposed patch v5

thx for the patch, Christian. One question - I see you removed the case for POP3_INTERRUPTED - can the #define be removed as well? I need to apply the patch and run with it a bit.

I can't quite tell what the function NoUidListAvailable does from its name - is there a more descriptive name you could use? And it should be UidlList...

Comment 24

[Christian Eyrich]

Oh sorry, I must have diffed an old version of the header file. Yes, the define can be removed as well.

NoUidListAvailable() handles the case if neither UIDL nor XTND XLST is available and decides what to do. I put it into a function because it can be encountered at two points. Mybe HandleNoUidListAvailable()?
Regarding "UidlList", UIDL stands for "unique-id listing". One element is a UID, a list of it is a uidl or UidList. Currently we're using "uidl" everywhere without differentiation.

Comment 25

[David :Bienvenu]

Comment on attachment 268802 [details] [diff] [review]
proposed patch v5

thx, Christian - looks good. I'll try to run with this for a bit

Oh, this comment needs to be fixed:

@@ -129,7 +129,7 @@
 # Do not translate "POP3"
 # Do not translate "%s". Place %s in your translation where the name of the server should appear.
 # Do not translate "TOP"
-4011=The POP3 mail server (%s) does not support the TOP command. Without server support for this, we cannot implement the ``Maximum Message Size'' preference.  This option has been disabled, and messages will be downloaded regardless of their size.
+4011=The POP3 mail server (%S) does not support the TOP command. Without server support for this, we cannot implement the ``Maximum Message Size'' or ``Fetch Headers Only'' preference.  This option has been disabled, and messages will be downloaded regardless of their size.

comment should refer to %S, not %s. 

Scott, I wonder if we need to generate a new string id for this string, since we've changed its semantics a bit.

Comment 26

[Christian Eyrich]

Attachment: patch v6

The last patch was developed in an old environment. This one's better.
I also added a check in put_hash() to prevent unoccupied slots in msg_info or empty uid strings from generating null hashes and producing popstate lines like
b  1182529772
Getting no uid should be very very rare but that cheap insurance can't be wrong.
And since having no hash entry makes it impossible to retrieve the rest if a message is only downloaded partially or delete it some time later, such a message is also retrieved normally. Regardless of fetch headers only or disk space settings.

Comment 27

[Christian Eyrich]

Attachment: patch v7

Update to current trunk since two patches yesterday changed same code.

Comment 28

[David :Bienvenu]

Comment on attachment 269866 [details] [diff] [review]
patch v7

looks good, thx, Christian.

Comment 29

[Christian Eyrich]

Attachment: patch v7 -u for checkin

Thanks David. In case it gets sr, here's the -u version of the patch for checkin.

Comment 30

[David :Bienvenu]

fixed on trunk, thx, Christian. I had to hand-apply the nsLocalStrings.h changes because of some line ending issues in your patch - I think I got it right.

Comment 31

[Marek Stępień [:marcoos, inactive]]

+# LOCALIZATION NOTE(4010): The following sentence should be translated in this +4040=The POP3 mail server (%S) does not support UIDL or XTND XLST, which are 

+# LOCALIZATION NOTE(4011): The following sentence should be translated in this way:
+4041=The POP3 mail server (%S) does not support the TOP command. Without server support for this, we cannot implement the ``Maximum Message Size'' or ``Fetch 

You may want to fix the LOCALIZATION NOTE numbers as well (s/4010/4040/ and s/4011/4041/).

Comment 32

[Christian Eyrich]

Thanks David. Don't know what happened to the line endings, but they look right now.

Sorry Marek, I missed to change these numbers. I made a note to hopefully fix this in some future patch.