If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in mozilla1.3beta

Status

()

Core
Networking: FTP
P1
major
VERIFIED FIXED
15 years ago
15 years ago

People

(Reporter: Hermann Schwab, Assigned: Darin Fisher)

Tracking

Trunk
mozilla1.3beta
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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

15 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

15 years ago
Changing Hardware/OS to All/All - seeing this on Windows 2000 and AIX builds as
well.
OS: Windows 98 → All
Hardware: PC → All
(Assignee)

Comment 3

15 years ago
-> me (i think i know exactly what is wrong)
Assignee: dougt → darin
(Assignee)

Comment 4

15 years ago
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...
Severity: normal → major
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.3beta

Comment 5

15 years ago
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
(Assignee)

Comment 6

15 years ago
>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 ;)
(Assignee)

Comment 7

15 years ago
Created attachment 113215 [details] [diff] [review]
v1 patch

here's the patch.
(Reporter)

Comment 8

15 years ago
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

15 years ago
Created attachment 113218 [details]
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
(Assignee)

Comment 10

15 years ago
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.
(Assignee)

Comment 11

15 years ago
Created attachment 113227 [details] [diff] [review]
v2 patch

patch attempting to fix split-CRLF problem.

Comment 12

15 years ago
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.
(Assignee)

Updated

15 years ago
Attachment #113227 - Flags: superreview?(dougt)
Attachment #113227 - Flags: review?(bbaetz)

Comment 13

15 years ago
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?
(Assignee)

Comment 14

15 years ago
dougt: sure thing...
(Assignee)

Comment 15

15 years ago
Created attachment 113248 [details] [diff] [review]
v2.1 patch (revised per dougt's comments)
Attachment #113227 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #113248 - Flags: superreview?(dougt)
Attachment #113248 - Flags: review?(bbaetz)

Updated

15 years ago
Attachment #113248 - Flags: superreview?(dougt) → superreview+
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
Attachment #113248 - Flags: review?(bbaetz) → review+
(Assignee)

Comment 17

15 years ago
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.
Attachment #113248 - Flags: approval1.3b?

Comment 18

15 years ago
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.
Attachment #113248 - Flags: approval1.3b? → approval1.3b+
(Assignee)

Comment 19

15 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 20

15 years ago
Verified on 2003020308 Windows build.
Status: RESOLVED → VERIFIED

Updated

15 years ago
Attachment #113227 - Flags: superreview?(dougt)
Attachment #113227 - Flags: review?(bbaetz)
You need to log in before you can comment on or make changes to this bug.