484684 - ParseFTPList can't handle regular unix ls -l listing of filenames starting with spaces.

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[andreas+mozilla]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.7) Gecko/2009030810 Iceweasel/3.0.7 (Debian-3.0.7-1)
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.7) Gecko/2009030810 Iceweasel/3.0.7 (Debian-3.0.7-1)

ParseFTPList strips of all leading spaces..... a filename that starts with spaces gets incorrectly listed.
ie. if you have both '   foo' and 'foo', you get a directory listing with 2x'foo'.


Reproducible: Always

Steps to Reproduce:
1. create files 'foo' and '   foo' on your ftp server.
2. connect with firefox
3. see 'foo' listed twice.
Actual Results:  
'foo' listed twice.

Expected Results:  
'   foo' and 'foo' listed.

Originally reported here (gvfsd-ftp reuses ParseFTPList):
http://bugzilla.gnome.org/show_bug.cgi?id=576229

See line 107 and 1165:
http://hg.mozilla.org/mozilla-central/file/956071116564/netwerk/streamconv/converters/ParseFTPList.cpp#l107
http://hg.mozilla.org/mozilla-central/file/956071116564/netwerk/streamconv/converters/ParseFTPList.cpp#l1165

Comment 1

[andreas+mozilla]

Attachment: workaround

This patch fixes the regular unix ls -l case, but might break the Hellsoft (old version) parsing which has an example at:
http://hg.mozilla.org/mozilla-central/file/956071116564/netwerk/streamconv/converters/ParseFTPList.cpp#l977

Comment 2

[Michal Novotny [:michal]]

Andreas, you need to ask someone for review if you want to get the patch in.

Comment 3

[andreas+mozilla]

(In reply to comment #2)
> Andreas, you need to ask someone for review if you want to get the patch in.

Who? Where? How?

Do you have anyone you can poke about looking at this? As I said before, my patch is a tradeoff that probably breaks some old obscure list format that I've never seen used to fix the common unix list format.... Would be nice if you have anyone available who cares about ParseFTPList for discussing the issue.

Comment 4

[Michal Novotny [:michal]]

Attachment: patch [Checkin: Comment 7]

This patch fixes space at the beginning of the filename for unix and DOS listing. It also contains a unittest. I'm not sure if old Hellsoft ftp really puts more spaces between date and filename. There is only 1 space in file netwerk/streamconv/converters/parse-ftp/U-HellSoft.in. But lets assume that some version with more spaces exists...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 371958 [details] [diff] [review]
patch
[Checkin: Comment 7]

Looks ok, but doesn't look like a change that needs sr to start with.  Does look like a change where a diff -w would be good.

Comment 6

[Jason Duell]

Comment on attachment 371958 [details] [diff] [review]
patch
[Checkin: Comment 7]

This looks great.   Nice test coverage.

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 371958 [details] [diff] [review]
patch
[Checkin: Comment 7]


http://hg.mozilla.org/mozilla-central/rev/179c6f22d6f2

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 371958 [details] [diff] [review]
patch
[Checkin: Comment 7]

This bug must have been around forever, we'd rather not take it on the stable branches and incur the additional QA requirement of testing the changed code.