365133 - The FTP folder won't open!

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Mehrdad Afshari]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20060601 Firefox/2.0 (Ubuntu-edgy)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20060601 Firefox/2.0 (Ubuntu-edgy)

The problem is that if the folder in FTP contains an space at the end, when you click the link in firefox, the url will not contain the trailing space, for example it works if manually enter "ftp://194.225.61/pub/Microsoft/Microsoft Office 2003 " (note the trailing space before quote!).

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Actual Results:  
You can not click the link to open a FTP folder containing a trailing space in its name

Expected Results:  
You should be able to navigate easily in FTP, even if a folder name contains a trailing space! 

It also appears in Konqueror

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

This doesn't even work for me in Internet Explorer 7.0 on Windows XP, so I'm not sure this is a Mozilla-only bug, or even _a_ bug, at that.  Did this ever work?  Do you have another test URL where this works in some other browser (Opera?)

Comment 2

[Mehrdad Afshari]

It IS a bug. If it doesn't work in IE, it doesn't mean it's not a bug! It can mean IE has the bug too! I had this problem today on a local FTP site in which a fool admin put a space at the end of name by mistake. It was unacceptable for me that the folder doesn't open, so I tried to open it in command line FTP. It didn't work either. Suddenly, I thought that the folder may end with space, so I put "\ " at the end of the "cd" command and it did work! After that, I tried to open it in FireFox by putting "%20" at the end of the URL. It worked too. The only problem is in rendering FTP folder as HTML, in link URLs, FireFox should replace " " with "%20". I tried to fix the bug myself -since it's easy to fix- but I couldn't find any documentation on FF source code. If anyone tell me in which file, FTP folder is rendered as HTML, I can probably fix it.

Comment 3

[Michal Novotny [:michal]]

Attachment: fix

Patch fixes:

- space and tab at the end of filename in Unix and DOS style listings
- date in DOS listing, short variants of year 00-80 were badly converted to 100-180, years 81+ weren't changed at all
- enhanced logic when searching for symlink name in Unix style listing
- regular files with sequence " -> " in name aren't now incorrectly detected as symlinks (DOS and Unix style listing only)

All data for Unix style listing testcases are from real ftp server (vsftpd), but all data for DOS listing testcases were made manually. I couldn't find any free windows ftp server that sends listing in DOS format.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 327510 [details] [diff] [review]
fix

>Index: netwerk/test/unit/test_bug365133.js
>+  return scinp.read(scinp.available());

Probably better to do this in a loop, no, just in case there is more than one packet?

>+                0 /* nsICache::STORE_ANYWHERE */,

Can you use Components.interfaces.nsICache.STORE_ANYWHERE here?  If so, make |const nICache = Components.interfaces.nsICache| and use that approach?

>Index: netwerk/streamconv/converters/ParseFTPList.cpp
>@@ -1150,37 +1152,59 @@ int ParseFTPList(const char *line, struc
>+              result->fe_fname[result->fe_fnlen - fe_size - 4] == ' ' &&
>+              result->fe_fname[result->fe_fnlen - fe_size - 3] == '-' &&
>+              result->fe_fname[result->fe_fnlen - fe_size - 2] == '>' &&
>+              result->fe_fname[result->fe_fnlen - fe_size - 1] == ' ')

This is crying out for an strncmp (or the NSPR version thereof, probably) call, I think.

>+              if (*p == ' ' && p[1] == '-' && p[2] == '>' && p[3] == ' ')

Probably same here.

The rest looks ok; r=bzbarsky with the above changes.  Doug, do you still remotely recall this code and want to give this a once-over?

Comment 5

[Michal Novotny [:michal]]

Attachment: patch v2, addressed review comments

Comment 6

[Doug Turner (:dougt)]

Comment on attachment 339815 [details] [diff] [review]
patch v2, addressed review comments

cool unit tests for FTP.  that is a first.

it has been a while, and I do not recall why we wanted to exclude the space.  sorry.

Could you add a comment around the date calculation.  I understand what you are doing, only after looking at the test case and recalling the incoming format.

Comment 7

[Michal Novotny [:michal]]

Attachment: patch v3, added comment about date calculation

Comment 8

[Reed Loden [:reed]]

http://hg.mozilla.org/mozilla-central/rev/b79bc71c6832