92582 - Some MS ftp servers (ftp.microsoft.com) never display lists - PASV invalid after error response

Status: RESOLVED WONTFIX

(Core Graveyard :: Networking: FTP, defect)

Description

[tenthumbs]

With an 07-26 nightly, but probably any build, mozilla hangs on 
<ftp://ftp.microsoft.com/>. The throbber throbs, the barber pole spins 
but nothing ever happens. The proximal cause is that the server sends a 
"425 Can't open data connection" response to the LIST command.

The real cause is more interesting. Mozilla sends these commands:
    USER ...
    PASS ...
    TYPE I
    PASV
    [mozilla parses the response and opens a socket to the host:port]
    SIZE / (fails)
    RETR / (fails)
    CWD / (succeeds)
    LIST (fails with the above message)

All this is easily reproducible with two telnet clients.

I believe that what is happening is that the server fails the RETR 
command and closes its socket to the data port. The client is still 
attached so the server cannot bind to the port again. The server, in 
fact, may be unwilling to open any data channel at this point. If I 
close the client data connection and just reissue a LIST, the server 
returns the same 425 response and the data port is no longer available 
to a client. The only thing that works is to reissue a PASV and then a 
LIST.

Since this doesn't seem like completely unreasonable server behavior, I
believe mozilla will have to close the a data channel on any failure and 
reissue a PASV for the next stage. Something like:
    SIZE /
    PASV
    [open data channel]
    RETR /
    [close data channel on failure]
    CWD /
    PASV 
    LIST

Similar things should happen for any RETR, LIST, or NLST.

This probably affects all platforms but I can only test Linux.

FWIW, I was investigating whether mozilla will properly parse the list 
output from this server. My reading of the code suggests mozilla will 
fail but this bug got in the way.

Comment 1

[tenthumbs]

Make summary more informative.

Comment 2

[Doug Turner (:dougt)]

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

Comment 3

[tenthumbs]

One issue is that a server that does not close a socket on error 
effectively sees two PASV commands in succession. What happens? ProFTPD 
and wu-ftpd don't care. They just close the previous connection and 
create a new one. I suppose there are some servers out there that might 
get confused. It probably needs testing.

Comment 4

[Doug Turner (:dougt)]

the ftp server is either full or it does not support passive transfers.  Marking
invalid.

Maybe we should reexamine the need for PORT - active transfers?

Comment 5

[tenthumbs]

I think the analysis is invalid. Using various command line clients, as 
well as telnetting, demonstrates that the server is clearly able to use 
passive connections as long as no error response occurs between the time 
the server issues the PASV response and the server tries to use it.

It is not clear to me that this is a protocol violation by the server.

This behavior is also not limited to the www.microsoft.com server; 
ftp.asus.com appears to use the same type of server and it has exactly 
the same problems.

Reopening, and changing summary to (hopefully) something more 
informative.

Comment 6

[lchiang]

Doug, is this a stop ship or is it resolved?  per PDT

Comment 7

[Doug Turner (:dougt)]

thanks tenthumbs for reopening.

this probably should be fixed soon.  QA- please check to see if this is a new
regression from 0.9.2. 

Something is wierd going on in the sockettransport.  I have some local diffs
that push out the same responses as 4x and we still have this no-connect problem.

Comment 8

[Marek Z. Jeziorek]

nsbranch+ per pdt triage

Comment 9

[Darin Fisher]

if you remove the 'RETR /' command, the site appears to load correctly.  any
ideas why this could be the case?

Comment 10

[Asa Dotzler [:asa]]

0.9.4 is out the door

Comment 11

[Ashley Bischoff (blog at handcoding.com)]

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

Comment 12

[tenthumbs]

The fundamental problem is that mozilla assumes that a 5xx response can 
be treated simply as a status message. These servers show that that 
assumption is untrue amd that a permanent error means just that. 
Emitting the RETR / shows what can happen.

Now the visible response is a 425 error which is "temporary" but it's 
the result of a previous 550 response. Mozilla is going to have to learn 
how to unwind a transaction and restart it if necessary. Since a data 
channel is open when these errors occur, mozilla must close the data 
channel and go back to a state before it issued a PASV. If mozilla 
actually did that before the 550 response to the RETR then it would 
never see the 425 response from the LIST. Even if mozilla did get an 
error from LIST, it could unwind and retry, which it probably should 
do for a 4xx response.

FWIW, my squid proxy also has troubles with these servers. It decides 
the servers don't support PASV and switches to a PORT transaction. 
That's not something mozilla can always do.

Comment 13

[Doug Turner (:dougt)]

Attachment: Double PASV support

Comment 14

[Doug Turner (:dougt)]

[e-mail], can you check out this patch?  It works for me.  
gagan, can you review?
darin, can you review?

Comment 15

[Darin Fisher]

Comment on attachment 49698 [details] [diff] [review]
Double PASV support

looks fine to me (sr=darin)

Comment 16

[Gagan]

Comment on attachment 49698 [details] [diff] [review]
Double PASV support

r=gagan

Comment 17

[Doug Turner (:dougt)]

Give it a try in tomorrows daily.

Checking in nsFtpConnectionThread.cpp;
/cvsroot/mozilla/netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp,v  <-- 
nsFtpConnectionThread.cpp
new revision: 1.194; previous revision: 1.193
done
Checking in nsFtpConnectionThread.h;
/cvsroot/mozilla/netwerk/protocol/ftp/src/nsFtpConnectionThread.h,v  <-- 
nsFtpConnectionThread.h
new revision: 1.86; previous revision: 1.85
done

Comment 18

[lchiang]

Doug - this is in the trunk?  Do we want this on the branch also? 

Comment 19

[Darin Fisher]

i'd like to see this land on the trunk... i'm sure it effects any site running a
MS ftp server.

Comment 20

[Darin Fisher]

s/trunk/branch/ ;-)

Comment 21

[lchiang]

For any possibility to land on the 094 branch, I'd suggest reopening this bug
and adding [FIXED ON TRUNK] in the status whiteboard.  This way, the bug will
get on PDT radar (and you can bring to PDT mtg) to get this in.  Tom or
TenThumbs - can you try out this fix on the trunk to see if ok and no major
regressions?  This will help in the data to get on the branch.  Thank you.

Comment 22

[Doug Turner (:dougt)]

reopening because PDT wants this fix on the 0.9.4 branch.

Comment 23

[Jaime Rodriguez, Jr.]

Check it in - PDT+. Let's get serious with the testing different server types.
Run it through its paces.

Comment 24

[tenthumbs]

I tried a 09-18-21 nightly. Both ftp.microsoft.com and ftp.asuscom.de 
work although mozilla throws some error messages to the console. 
ftp.asus.com has other problems.

Comment 25

[Tom Everingham]

verified on trunk using ngrep

winNT v4 2001091903
Linux rh6 2001091908

the ftp.asus.com problem with RETR is described in bug 99233

Comment 26

[Doug Turner (:dougt)]

reassigning to [e-mail].

Comment 27

[Bradley Baetz (:bbaetz)]

->dougt if he wants this on the branch

Comment 28

[Darin Fisher]

fixed-on-branch

Comment 29

[Tom Everingham]

verified on branch:

Win NT4 2001-09-24-05-0.9.4
Linux rh6 2001-09-24-05-0.9.4

Comment 30

[Tom Everingham]

verified on branch:

Win NT4 2001-09-24-05-0.9.4
Linux rh6 2001-09-24-05-0.9.4

Comment 31

[Mike Kaply [:mkaply]]

This patch broke FTPing to an OS/2 server.

We get the message:

pasv : Can't bind the socket.: error 10.

The mResponseCode we get is 550.

Comment 32

[Bradley Baetz (:bbaetz)]

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

Comment 33

[Darin Fisher]

so, getting a listing off a wu-ftpd server on a redhat 8.0 linux box requires
two DATA connections because of this patch.  the first DATA connection is
destroyed as a result of receiving a 550 response to the RETR command.  i'm not
sure how much this matters, but if it is breaking FTP on OS/2 then perhaps we
need to look for another solution.

Comment 34

[Doug Turner (:dougt)]

the solution does suck.

Comment 35

[Darin Fisher]

dougt: if you're going to hack on this, let me know because i have "async
stream" changes (bug 176919) effecting this code.  adding as a dependency bug fwiw.

Comment 36

[tenthumbs]

Since we've learned the hard way that ftp servers are unreasonable how 
about this?

The ftp URL syntax is rather restrictive so why not use it to determine 
the proper action. For example, <ftp://localhost/foo/bar/> is distinct 
from <ftp://localhost/foo/bar>. In the first case, you transform it into
   CWD foo/bar
   LIST

In the second case, you do
   CWD foo
   RETR BAR

For well-formed URL's this should work quite nicely because you never 
try a command on the wrong file type. This is essentially what the squid 
proxy does.

Of course the internet is filled with mal-formed URL's so the tricky 
part is error handling. I see that RFC1738 suggests that the safest 
policy is to abandon the entire connection. Presumably that includes the 
control channel. Mozilla could do that and retry with the knowledge it's 
learned. BTW, since there are a lot of unix servers you can't assume 
that only directories and regular files exist, you have to consider the 
7 POSIX types as well.

A major rewrite of the state machine, unfortunately.

Comment 37

[Doug Turner (:dougt)]

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

Comment 38

[Vladimir]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007

Ran several FTP servers, i.e.:
1. TYPSoft FTP Server ( http://www.webattack.com/get/typftpserver.html )
2. Windows Ftp Server (
http://www.softpicks.net/software/Windows-Ftp-Server-6108.htm )

Mozilla either could not connect or produces wrong directory listing.

MSIE worked fine and (several) FTP clients worked fine.

All tested on the same PC.

Comment 39

[Matthias Versen [:Matti]]

Try www.guildftpd.com

Comment 40

[Doug Turner (:dougt)]

mass reassigning to nobody.