134901 - Time left is negative (has "-") and size is 0K for ftp downloads

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Martin Rasp]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.9+) Gecko/20020401
BuildID:    2002040108

When downloading something the value behind "Time Left" contains one or more "-"
characters.

Reproducible: Always
Steps to Reproduce:
1. Download file
2.
3.

Actual Results:  00:0-45
0-1:0-5

Expected Results:  00:00:45
00:01:05

Comment 1

[Adam Hauner]

Same with 2002040103/Win2K. BTW saving hasn't info about whole size of
downloaded file, it shows status: 8013KB of 0KB (at 50.6KB/sec).

Comment 2

[Boris Zbarsky [:bzbarsky]]

This may well be a networking:ftp issue.  It seems to be a problem in the
regular download dialog too, not just in download manager.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

I've seen this too today, without using download manager.
to networking:ftp

Comment 4

[Bradley Baetz (:bbaetz)]

Oops. I readded the response code to mResponseMsg, but didn't change the code
which parsed this.

Patch coming up.

Comment 5

[Bradley Baetz (:bbaetz)]

Attachment: patch

Cut the response code out of the message before trying to parse it.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 77370 [details] [diff] [review]
patch

r=dougt

Comment 8

[Marc Boullet]

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

Comment 9

[Darin Fisher]

Comment on attachment 77370 [details] [diff] [review]
patch

Index: nsFtpConnectionThread.cpp

>     if (mResponseCode/100 == 2) {
>-        PRInt32 conversionError;
>-        PRInt32 length = mResponseMsg.ToInteger(&conversionError);
>-        if (NS_FAILED(mChannel->SetContentLength(length))) return FTP_ERROR;
>+        mFileSize = atoi(mResponseMsg.get()+4);
>+        if (NS_FAILED(mChannel->SetContentLength(mFileSize))) return FTP_ERROR;
>     }
> 
>-    if (mResponseCode == 213)
>-        mFileSize = atoi(mResponseMsg.get());

so, this bug is about an error w/ the way the file size is parsed, but
previously we were only assigning mFileSize on a 213... now we do it for
any 2xx message.  is that ok?

sr=darin

Comment 10

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 11

[Bradley Baetz (:bbaetz)]

We were actually parsing teh file size twice, before, and looking at the first
digit only is correct, since that indicates what sort of response it was.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 77370 [details] [diff] [review]
patch

This makes us look bad, very visible.
a=[e-mail]

Comment 13

[Bradley Baetz (:bbaetz)]

Checked in.

Comment 14

[[not reading bugmail]]

<QA note> this happens on ftp://ftp links not http://ftp links.. see my bug
128927 comment #35.

Comment 15

[Adam Hauner]

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

Comment 16

[benc]

VERIFIED: rc1,w98.
should be covered by file handler tests anyhow.