177964 - FTP: can't download file with # in its name

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Jesse Ruderman]

Steps to reproduce:
1. Log into Jeff's ftp with Phoenix 10/26 or with Mozilla.
2. Go to /upload/Hack_Sign/.
3. Look for episode 17 and click the link.

Result: "No such file or directory".

Filename:  /upload/Hack_Sign/[#Anime-Kraze]_Hack_17[SBC_HQ_DIVX].avi
Error: 550 /upload/Hack_Sign/[: No such file or directory.

The steps above work correctly in Internet Explorer and Netscape 4, but they
give an error message in Mozilla and Opera.  If I copy the URL, paste it into
the address bar, replace the # with %23, and press enter, I am able to download
the file with Mozilla.

Old, related bugs:
bug 27741 [fixed] can't visit directorys with ftp that include # or :
bug 77937 [fixed] '#''s in ftp url's dont work [as hashes pointing to anchors]

The bug seems to be in in the code that generates the HTML directory listing. 
It should convert # to %23 in the link URLs (like Netscape 4 does) so that when
I click to download a file with # in its name, the link works correctly.

Comment 1

[benc]

Mozilla 1.2a - I've confirmed this.

The directory displays "#", but does not provide the correct link.

I thought we had covered the handling of "#", but I had forgotten to test for
the directory display's mapping of special characters to URLs for the filenames
themselves.

Comment 2

[Andreas Otte]

Yes, obviously the links are not escaped correctly (esc_FileBaseName-Mask)
before handed over to ftp.
 

Comment 3

[neeti]

me

Comment 4

[neeti]

reporter: how do I log into Jeff's ftp?

Comment 5

[R.K.Aa.]

similar: bug 180161

Comment 6

[neeti]

Attachment: proposed patch

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 108082 [details] [diff] [review]
proposed patch

the call to NS_EscapeURL leaks.

Comment 8

[Doug Turner (:dougt)]

nhotta, could you look at this patch (ignoring the leak).  i believe this
problem to be regression caused by our fix for 165585.  Does this patch cause
the same problem that we had with 165585?

Comment 9

[nhottanscp]

I think non ASCII characters would be all escaped. Is it possible to escape only
characters which really need to be escaped?
Also ToNewCString(unEscapeSpec) cannot be used for Unicode which may contain
characters greater than 255.

Comment 10

[neeti]

Attachment: revised patch

Comment 11

[nhottanscp]

Comment on attachment 108253 [details] [diff] [review]
revised patch

+    NS_ConvertUCS2toUTF8 ucs2UnEscapeSpec(unEscapeSpec);

please change the variable name to something like utf8UnEscapeSpec because the
string is UTF-8.

Comment 12

[neeti]

Attachment: revised patch, changed variable name

Comment 13

[Darin Fisher]

Comment on attachment 108253 [details] [diff] [review]
revised patch

>Index: nsIndexedToHTML.cpp

>+    NS_ConvertUCS2toUTF8 ucs2UnEscapeSpec(unEscapeSpec);

nit: ucs2UnEscapeSpec is not UCS2, but UTF8.  might be better
to use a name like utf8UnEscapeSpec.

sr=darin (no need to submit another patch)

Comment 14

[Darin Fisher]

Comment on attachment 108256 [details] [diff] [review]
revised patch, changed variable name

sr=darin (whoops!! i hadn't noticed that you already fixed this)

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 108256 [details] [diff] [review]
revised patch, changed variable name

a=asa for checkin to 1.3a

Comment 16

[nhottanscp]

cc to ylong for i18n verification once checked in

Comment 17

[neeti]

fix checked in.

Comment 18

[Yuying Long]

> cc to ylong for i18n verification once checked in

verified download works with:
"#" + double byte characters
"#" + single byte and double byte characters
named file on an internal ftp site.

Comment 19

[benc]

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

Comment 20

[benc]

+regression.

I was poking around w/ the duplicate, and noticed that this works in Netscape
7.0.1 (Mozilla 1.0.2), and Chimera 0.6 (branch?).

Is this something that regressed recently?

Comment 21

[benc]

VERIFIED: Mozilla 1.3f, Win98

Comment 22

[benc]

VERIFIED: mozilla 1.4b, Linux+Mach-O