386167 - firefox doesn't show login/password form when connecting to ftp server with anonymous accounts disabled

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Arkadiusz Miśkiewicz]

User-Agent:       Opera/9.21 (X11; Linux i686; U; en)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.4) Gecko/20070531 2.0.0.4

Example session with pureftpd ftp server with anonymous users disabled:

x.x.0x.099.00021-192.168.001.157.04968: 220-Welcome to Pure-FTPd.
220-You are user number 6 of 100 allowed.
220-This is a private system - No anonymous login
220-IPv6 connections are also welcome on this server.
220 You will be disconnected after 15 minutes of inactivity.

192.168.001.157.04968-x.x.x.099.00021: USER anonymous

x.x.x.099.00021-192.168.001.157.04968: 530 This is a private system - No anonymous login

Note that firefox isn't able to deal with that if 530 message is sent directly after USER xyz.

If USER is accepted and ftpd sends 530 after PASSWORD command then firefox deals with that correctly.

Reproducible: Always

Steps to Reproduce:
1. See Details
2.
3.


Expected Results:  
Firefox should display login dialog asking for username and password.

Comment 1

[Michal Novotny [:michal]]

It isn't a big problem to handle 530 response after USER command in the same way as after PASS command. The problem is that pureftpd closes the connection after it sends 530 error message. And even worse is that f.e. Fedora version of pureftpd closes the connection without sending any error at all. This could be probably solved by reconnecting in nsFtpState::OnControlError() under given conditions. But the code isn't reentrant so any second try after anonymous login attempt must be done with new connection. Assume following scenario:

  1) client tries anonymous login first and sends
     "USER anonymous"
  2) server responds with
     "530 This is private system ..."
  3) client sets mAnonymous = PR_FALSE and enters FTP_S_USER state again
  4) modal prompt window is shown in the nsFtpState::S_user() and next events are processed
  5) server closes the connection and nsFtpState::OnControlError() is called
  6) now we shouldn't manipulate with nsFtpState class because nsFtpState::S_user() hasn't finished yet

Easiest way is IMHO to drop control connection at point 3 and create the new one. Any opinions?

Comment 2

[Michal Novotny [:michal]]

Attachment: Fixes the bug as described in comment #1

Comment 3

[Michal Novotny [:michal]]

Attachment: testing ftp servers

There are 3 testing servers in the archive. They all fail to authorize user but they differ in handling anonymous user.

testserv-ftp-1.py - closes connection without any response after "USER anonymous" is received 

testserv-ftp-2.py - responds "530 ..." and closes connection after "USER anonymous" is received

testserv-ftp-3.py - doesn't close connection, responds "530 Access denied" after "PASS ..." is received

Firefox works correctly only with testserv-ftp-3.py.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 323935 [details] [diff] [review]
Fixes the bug as described in comment #1

Please put the new member next to the other packed bools (and reorder the initializer list accordingly)  r+sr=bzbarsky with that.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, and can we add tests?

Comment 6

[Michal Novotny [:michal]]

Attachment: fixed issue addressed in comment #4

Comment 7

[Michal Novotny [:michal]]

(In reply to comment #5)
> Oh, and can we add tests?

This seems quite complicated to me. I would need to write the ftp server in JS for this. Is there some example in xpc-shell tests that I could use? And I would need to install my own auth prompt where I would mark the test as passed. Is this possible?

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 339823 [details] [diff] [review]
fixed issue addressed in comment #4

Could you attach a Hg diff, please ?

Comment 9

[Michal Novotny [:michal]]

Attachment: patch against hg trunk [Checkin: Comment 10]

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 363131 [details] [diff] [review]
patch against hg trunk
[Checkin: Comment 10]


http://hg.mozilla.org/mozilla-central/rev/4fb9b2baeed4