191220 - FTP: not entering directory, instead message box : "Alert: 250 CWD command successful"

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect, P1)

Description

[Hermann Schwab]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030129
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030129

I was in ftp://ftp.mozilla.org/pub/mozilla/nightly
and wanted to look in some folders.
I could look in some folders with mozilla, but the two I was interested in I
couldn´t enter, got an Alert-Box instead.
While beeing on this site, i tried multiple times and folders,
I couldnt enter ftp://ftp.mozilla.org/pub/mozilla/nightly/2003-01-28-12-trunk
and ftp://ftp.mozilla.org/pub/mozilla/nightly/2003-01-28-08-trunk

Got a box: Alert 250 CWD command successful.

tried with Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030129 Build
ID 2003012908, was a talkbackless zipfile from komodo just unzipped over
yesterdays talkback enabled nightly.
working with phoenix: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3a)
Gecko/20030127 Phoenix/0.5

Then I deleted this mozilla, and unzipped this talkbackenabled Mozilla creating
a new, fresh folder.
At first glance, it seemed to work better, but while I´m writing this bug I get
same error message when trieng to access that folders.
I also retried Phoenix, and Phoenix shows same errror, but much less often.
Once I got PASV, maybe bug 130299

The effect is not limited to these 2 folders, sometimes they open, mostly not.
Others opened mostly, alerted rarely.

I didn´t use ftp, so I can´t tell if downloading via ftp is affected.

Reproducible: Sometimes

Steps to Reproduce:
1.ftp://ftp.mozilla.org/pub/mozilla/nightly
2.try to open folders, esp. 28-08-trunk or 28-12-trunk
3.

Actual Results:  
some folders opened, others gave alerts.
Alert: 250 CWD command successful didn´t open folder, did nothing at all.

Expected Results:  
descend into folder, display contents.

tried in Mozilla with pipelining enabled and disabled, no big difference visible.

There maybe additional info/discussion in
http://www.mozillazine.org/forums/viewtopic.php?p=38069

Comment 1

[Ian Neal]

Confirming on BuildID 2003012908 as well as 2003012808 on WinXP SP1. Get other
similar messages (like the Passive mode one) to do with ftp as well. Problem
doesn't happen in BuildID 2003011508 as far as I've noticed.

Comment 2

[Philip K. Warren]

Changing Hardware/OS to All/All - seeing this on Windows 2000 and AIX builds as
well.

Comment 3

[Darin Fisher]

-> me (i think i know exactly what is wrong)

Comment 4

[Darin Fisher]

ok, the problem is that FTP uses a single segment pipe with a blocking input end
for writing to the control connection.  this is a regression from my patch for
bug 189689.  i forgot that FTP needs the pipe to roll-back its write cursor,
else the single segment pipe can essentially become full and a blocking write
call will only be able to write part of the data.  FTP's write code assumes that
a blocking write will either write all or fail with some error, which is
required by nsIOutputStream.  the solution is to fix the pipe to rewind its
cursors inside GetWriteSegment if it happens that mWriteCursor == mReadCursor. 
i had tried to do this before, but i mistakenly did the rollback inside
AdvanceReadCursor, which was the cause for bug 189689.  quick patch coming up...

Comment 5

[Philip K. Warren]

Just FYI, I am getting the following assertion in a debug build when I 
experience this problem:

###!!! ASSERTION: Read buffer doesn't include response code: 'line.Length() > 4 
&& startNum', 
file /home/pkw/builds/trunk/mozilla/netwerk/protocol/ftp/src/nsFtpConnectionThre
ad.cpp, line 494

After closing the alert dialog, I get the following:

Error loading URL ftp://ftp.mozilla.org/pub/mozilla/nightly/2003-01-12-08-
trunk : 804b0016

Comment 6

[Darin Fisher]

>ok, the problem is that FTP uses a single segment pipe with a blocking input end
 ... of course, i meant... with a blocking _output_ end ;)

Comment 7

[Darin Fisher]

Attachment: v1 patch

here's the patch.

Comment 8

[Hermann Schwab]

wanted to retest today, got another result:

At first load folders between 2003-01-21-21-trunk and 2003-01-29-12-trunk were
not displayed, after reload missing folders were shown.
Tested with 
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3a) Gecko/20030130 Phoenix/0.5
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030130 Build ID 2003013008 
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030131 Build ID 2003013104

Didn´t have the time to test the original subject

Comment 9

[Philip K. Warren]

Attachment: NSPR FTP log

The first patch didn't seem to fix the problem for me. Attached is a NSPR log,
using the following settings suggested by darin:

NSPR_LOG_MODULES=nsFTPProtocol:5,nsSocketTransport:5,nsPipe:5

Comment 10

[Darin Fisher]

OK, so my original suspicious are off (though i still think the first patch is
good to have)... however, from the log file it appears the problem results from
the following sequence:

  nsFtpState::ODA(response + CR)
  nsFtpState::ODA(LF)

the code doesn't seem to behave well when the CRLF is split into two ODA events.
 it seems that the async-io landing has somehow made this bug more visible.

the second ODA event with only a LF causes mResponseCode to be set to a value of
0 and mResponseMsg to hold the LF.  as a result, the next ODA event will not set
mResponseCode.  in the case of nsFtpState::R_cwd, a response code of 0 is
treated as a failure, hence the alert with the "250 CWD ..." nonesense.

Comment 11

[Darin Fisher]

Attachment: v2 patch

patch attempting to fix split-CRLF problem.

Comment 12

[Philip K. Warren]

I have tested the second patch extensively and it seems to fix the problem. I
have tried it both with and without the first patch, and the first patch doesn't
seem to have any effect.

I connected to ftp://ftp.mozilla.org/pub/mozilla/nightly and changed directories
30+ times with no problems. There are no longer any assertions on the console,
and I have been unable to reproduce the behavior described in this bug. Looks
good from here.

Comment 13

[Doug Turner (:dougt)]

Comment on attachment 113227 [details] [diff] [review]
v2 patch

the logic is fine.

how about combinding the tests into one if? maybe move the strlen() up or
something?

Comment 14

[Darin Fisher]

dougt: sure thing...

Comment 15

[Darin Fisher]

Attachment: v2.1 patch (revised per dougt's comments)

Comment 16

[Bradley Baetz (:bbaetz)]

Comment on attachment 113248 [details] [diff] [review]
v2.1 patch (revised per dougt's comments)

I think that this bug may be a dupe
r=bbaetz

Comment 17

[Darin Fisher]

Comment on attachment 113248 [details] [diff] [review]
v2.1 patch (revised per dougt's comments)

seeking drivers approval for 1.3b... low-risk fix for a sometimes annoying FTP
bug.

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 113248 [details] [diff] [review]
v2.1 patch (revised per dougt's comments)

a=asa (on behalf of drivers) for checkin to 1.3beta.

Comment 19

[Darin Fisher]

fixed-on-trunk

Comment 20

[Philip K. Warren]

Verified on 2003020308 Windows build.