227267 - Upload file doesn't work well. It just upload file with zero size. [ftp only?]

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Pete Zha]

I can't use necko to upload file on trunk. Files upload on server are zero size.
This worked fine on 1.4 branch but trunk has problem. I will upload a testcase soon.

Comment 1

[Pete Zha]

Attachment: testcase for ftp upload

Please modify server information to test

Comment 2

[Pete Zha]

Comment on attachment 136654 [details]
testcase for ftp upload

And please download this file to local and open it.

Comment 3

[Pete Zha]

Actually this also breaks the Webpage publish feature. The transfer process will
not stop when I upload file to ftp server using web publish.
Darin,
Do you have idea on this problem? Seems network changed a lot between 1.4 and 1.6.  

Comment 4

[Ben Bucksch (:BenB)]

Is this only ftp upload or also http upload? If the former, this wouldn't
prevent the review and check in of the roaming code.

Comment 5

[Pete Zha]

http also have problem, file size will be zero on server.

yes, It's only block the function of roaming. I will make patch and go on the
review process of roaming. But I think this should be fix before we check in
roaming feature. Otherwise it will make confuse.

Comment 6

[Matthias Versen [:Matti]]

Http Upload works in my win2k trunk build. Tested uploading a gif to bug 224303

Comment 7

[Ben Bucksch (:BenB)]

Matti, this is interesting, but does Editor's Publish function work for you,
too? This is much closer to the roaming code.

Comment 8

[Pete Zha]

Matti,
Maybe it's different between upload file on webpage and upload file using http
channel. Did you try the testcase?

Comment 9

[Matthias Versen [:Matti]]

Uploading via publishing doesn't work for me...

Comment 10

[Kathleen :Brade]

This seems like a very bad regression.  Is it too late to block 1.6?  Do we know
approximately when this regression first appeared?  Can we pin it on a
particular checkin?

Comment 11

[Kathleen :Brade]

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

Comment 12

[WD]

It works with 1.5, but not 1.6a
(from duped bug)

Comment 13

[Frank Wein [:mcsmurf]]

If this should really be a regression in netwerk, here's a bonsai link
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fnetwerk&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=10%2F01%2F2003+05%3A00%3A00&maxdate=10%2F18%2F2003+05%3A00%3A00&cvsroot=%2Fcvsroot
I tracked down this regression to somwhere between 2003-10-01-05 and
2003-10-18-05 on trunk (i know that's not very exact but nightlies are missing
for that timeframe on my pc).

Comment 14

[Kathleen :Brade]

According to Frank Wein (comment 13) and what I see in bonsai, the regression
may be caused by the checkins for
 - bug 210125 (need to be able to AsyncWait for closure only) on October 5
 - bug 192284 (support nsIChannel::open for all protocols) on October 7

Comment 15

[Boris Zbarsky [:bzbarsky]]

Having a smaller regression window (or a test server that could be used by
someone else to narrow the regression window) would be nice.

Comment 16

[Frank Wein [:mcsmurf]]

Tracked down somewhat more: This regressed between 2003-10-05-05 and 2003-10-11-05

Comment 17

[Boris Zbarsky [:bzbarsky]]

Right, but per comment 14 what we need to know is whether builds from the 6th
are broken....

Comment 18

[Frank Wein [:mcsmurf]]

yes, builds from 2003-10-06-05 are already broken

Comment 19

[Darin Fisher]

this is likely due to bug 210125... investigating.

Comment 20

[Darin Fisher]

i'm able to reproduce this using wu-ftpd on a linux box and TestUpload as the
client.

$ ./TestUpload ftp://user:pass@host/file.ext /path/to/file.ext

the result is a zero length file on the server named file.ext, and TestUpload
never seems to complete.  i'm not sure if the fact that it never seems to
complete is related to this bug or not.

investigating...

Comment 21

[Darin Fisher]

this bug is FTP specific.  i'm not sure it can be fixed with a trivial patch :(

Comment 22

[Kim Scarborough]

Reporter says it's happening on HTTP as well. That's how he found it, I believe.

Comment 23

[Darin Fisher]

then the HTTP problem is unrelated to this bug.

Comment 24

[Ben Bucksch (:BenB)]

Filed bug 228784 about HTTP.

Comment 25

[Darin Fisher]

Attachment: v1 patch

ok, this works with my wu-ftpd testcase.  if anyone else can test this with
other servers that would be most appreciated!

Comment 26

[Darin Fisher]

Comment on attachment 137582 [details] [diff] [review]
v1 patch

ok, here's the deal with this patch... instead of creating a input stream pump
in the STOR case and then tearing it down in R_stor, this patch makes it so
that we setup a async stream copier from the get go.  the result, is that we
don't have to worry about tearing down the input stream pump and so we don't
have to deal with the wanky NS_ERROR_IGNORE_NOTIFICATION fubar.

note: nsIAsyncStreamCopier implements nsIRequest just like nsIInputStreamPump,
so this patch works without much need to change anything up-stream ;-)

Comment 27

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

the patch also works for me with ProFTPD 1.2.8

Comment 28

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

I applied the patch to Nvu and it fixes the bug.
Thanks for the good work, Darin!

Comment 29

[WD]

The patch fixes the problem for me, both proftpd-1.2.9_rc2 and the FTP server
that ftp.tripod.com uses.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 137582 [details] [diff] [review]
v1 patch

I won't be able to review this until probably Tuesday.

Comment 31

[Daniel Veditz [:dveditz]]

Comment on attachment 137582 [details] [diff] [review]
v1 patch

sr=dveditz

Comment 32

[Doug Turner (:dougt)]

Comment on attachment 137582 [details] [diff] [review]
v1 patch

yes, this should work fine.  Great work Darin.

Comment 33

[chris hofmann]

Comment on attachment 137582 [details] [diff] [review]
v1 patch

a=chofmann for 1.6.  thanks to all for the help in knocking this one out
quickly

Comment 34

[Bradley Baetz (:bbaetz)]

Comment on attachment 137582 [details] [diff] [review]
v1 patch

Have you tested with OS2?

Comment 35

[Darin Fisher]

bbaetz: nope, i haven't tested this patch against an OS/2 FTP server.  are you
concerned about something in particular?

Comment 36

[Darin Fisher]

fixed-on-trunk for 1.6 final

Comment 37

[Pete Zha]

This patch works fine for me on windows. Thanks darin!

-Pete

Comment 38

[WD]

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

Comment 39

[Matthias Versen [:Matti]]

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

Comment 40

[Asa Dotzler [:asa]]

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