83634 - Can't get messages from POP server

Status: VERIFIED FIXED

(MailNews Core :: Networking: POP, defect, P1)

Description

[Koike Kazuhiko]

A Japanese user reported he often failed to get new messages from POP server.
POP3 log is the following:

0[344048]: Entering NET_ProcessPop3 21
0[344048]: POP3: Entering state: 1
0[344048]: POP3: Entering state: 2
0[344048]: POP3: Entering state: 4
0[344048]: RECV: (null)
0[344048]: POP3: Entering state: 29
0[344048]: POP3: Entering state: 24
0[344048]: POP3: Entering state: 25

"RECV: (null)" is strange. But if he connect the POP server by telnet command,
he gets "+OK <mail server> POP3 server ready".

http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=987

Comment 1

[Navin Gupta]

Attachment: proposed fix.

Comment 2

[Navin Gupta]

Could you try this patch or ask that japanese user to try this patch, if 
possible. I think command_succeeded should be set to TRUE even when we receive
null so that the subsequent commands are sent. 

nominating 

Comment 3

[Koike Kazuhiko]

Isn't it neccesary to receive "+OK <mail server> POP3 server ready"?
The original reporter said he received it if he connected the server by
telnet command.

I can't reproduce this bug, but I modified nsPop3Protocol.cpp as follows:
*********************************
if(line && *line == '+')
    {
        if(PL_strlen(line) > 4)
                        m_commandResponse = line+4;
        else
                        m_commandResponse = line;
    }
m_pop3ConData->command_succeeded = PR_FALSE;
*********************************

And I got the same log as the original reporter's.
*********************************
1024[805faf8]: Entering NET_ProcessPop3 57
1024[805faf8]: POP3: Entering state: 1
1024[805faf8]: POP3: Entering state: 2
1024[805faf8]: POP3: Entering state: 4
1024[805faf8]: RECV: +OK <mail server> POP3 server ready (ESIZE/jetstar)
1024[805faf8]: POP3: Entering state: 29
1024[805faf8]: POP3: Entering state: 24
1024[805faf8]: POP3: Entering state: 25
*********************************

I think command_succeeded should be set to FALSE if "+" isn't returned.

Comment 4

[Koike Kazuhiko]

The problem is why Mozilla can't get "+OK <mail server> POP3 server ready".

Comment 5

[Navin Gupta]

Try my patch, it is the correct thing to do here. 

Comment 6

[Koike Kazuhiko]

OK, please check in the patch. The original reporter doesn't have a build
environment.

Comment 7

[Navin Gupta]

Could I get a test account on that server to verify this patch, thanks!

Comment 8

[Koike Kazuhiko]

No, the server is within his company's LAN and not accessible from outside.

Comment 9

[scottputterman]

Navin,

If we don't receive +OK at this point, what happens next with the code that you
changed? Does it keep waiting until it gets it?  If so, at what point does it
stop waiting?

Comment 10

[Navin Gupta]

Attachment: better & safer fix.

Comment 11

[Navin Gupta]

We want to waitforStartofconnectionResponse only when we start a new connection
That is the only time the server would send us +Ok..... If the user types a
wrong password, then we are basically using the same connection so set the 
next state to after FINISH_CONNECT. 

cc bienvenu for review. 

I want to get this checked in for 0.9.1

Comment 12

[David :Bienvenu]

I really don't know this code and I'm a little afraid of changing this so late.
Is this a recent regression? Can you get mscott to look at it?

Comment 13

[Koike Kazuhiko]

0.8 works fine, but 0.9 has this bug.

Comment 14

[Navin Gupta]

I had changed some code to make the password dialog to come up again and again 
and it caused this problem. But I am pretty sure this is correct thing to do 
here while still allowing the pwd dialog to come up again and again. 

When we make a connection we get +Ok.. from the server. So we need to wait 
for the response if we receive null. But we don't need to repeat this step 
(waitforStartofconnectionResponse) if we are resending the password on the same 
connection because the server will only send null and nothing else. 

mscott is away on vacation but i can wait for him to come back. 

Comment 15

[Scott MacGregor]

sr=mscott

Comment 16

[David :Bienvenu]

r=bienvenu, but make sure you've tested the various permutations of pop get new
mail (cancelling password prompt, invalid password, etc).

Comment 17

[scottputterman]

adding pdt+

Comment 18

[Navin Gupta]

I have tested all combinations of Getting New mail, Cancel password and
invalid password. 

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

a=dbaron for trunk checkin (on behalf of drivers)

I was a little worried about this one (as I said in email yesterday), which is
why I didn't approve it yesterday.  It appeared to me like a nontrivial change
in core-level code, although the bug it's fixing also seems quite serious so
it's certainly worthy of approval even if somewhat risky, assuming it's well
tested.  In an approval request (or, better, in comments on the bug cited in the
approval request), it's often nice to read a little (say, a sentence or two)
about the risk involved  in a patch when it's not obvious.  The drivers
certainly don't have detailed knowledge about all the code in Mozilla, so it's
helpful to give a knowledgable opinion about what the risk is (for example, what
possible regressions it could cause and how well the code has been tested). 
Anyway, please watch for any possible regressions...

FWIW, this comment seems backwards:
+        m_pop3ConData->pause_for_read = PR_TRUE; /* don't pause */

Comment 20

[Navin Gupta]

fix checked in. [e-mail], please verify this bug with tomorrow's
trunk build. 
                                                                      

Comment 21

[Sheela Ravindran]

reporter,
Can you please verify this bug since this problem is seen on one of your 
internal pop servers. Thanks.

Comment 22

[Koike Kazuhiko]

The original reporter said he could get messages from POP server now.
Verifying.