126468 - crash when displaying FTP directory listing

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Mark Smith [:mcs]]

BuildID:    20020218 (my own build actually)

The code that parses FTP directory listings assumes that it will receive data in
a specific format. Unfortunately, if that is not the case, Mozilla may crash.

Reproducible: Always

Steps to Reproduce:
1. Find an FTP server that will return "something weird" in a directory listing.
Unfortunately the example I have is only accessible by authenticated users
inside the Netscape firewall (ftp://<id>@maize.mcom/com/%2Fu/mcs/tmp). There is
a file in /u/mcs/tmp that causes ls -l (and FTP dir listings) to return an error
message.

2. Enter the URL to the bad FTP location in the URL bar and hit return.

3. Crash inside PR_FormatTimeUSEnglish(), called from
nsFTPDirListingConv::DigestBufferLines() because the time structure is not
initialized.

Comment 1

[Mark Smith [:mcs]]

Attachment: log from command line FTP client

Comment 2

[Mark Smith [:mcs]]

Attachment: patch that prevents the crash (needs more work)

Comment 3

[Rod Cater]

Attachment: crash on Linux with ftp session log

Comment 4

[benc]

+helpwanted, crash - problem is well described.

Ron, I'm assuming you have the same problem on a different URL. If it is public,
can you post it here?

Comment 5

[benc]

-> ftp

Comment 6

[Bradley Baetz (:bbaetz)]

I can't repro this, but the patch looks fine - we do it everywere else to
prevent this crash. See bug 116170, too.

What about the patch "needs more work" ?

Comment 7

[Mark Smith [:mcs]]

The output produced is somewhat unusual, which is why I said the patch needs
more work (perhaps lines in the LIST results that are not parsable should be
ignored? I am not sure).

But given the rarity of FTP reachable servers and directories that have this
kind of problem, the patch is probably good enough. Much better than crashing.

Comment 8

[Bradley Baetz (:bbaetz)]

Comment on attachment 70362 [details] [diff] [review]
patch that prevents the crash (needs more work)

r=bbaetz. While I can't reproduce this, not initing the value here was just an
oversight.

Comment 9

[Bradley Baetz (:bbaetz)]

-> patch author.

Mark, you should mail [e-mail], and cc [e-mail], aksing
for superreview for this bug. I can check it in after that + approval, if you
don't have a cvs account.

This should probably be done soon so that it gets in for 1.0.

Comment 10

[Mark Smith [:mcs]]

brade said she would talk to Darin about this. Reassigning.

Comment 11

[Darin Fisher]

Comment on attachment 70362 [details] [diff] [review]
patch that prevents the crash (needs more work)

sr=darin (nice catch)

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 70362 [details] [diff] [review]
patch that prevents the crash (needs more work)

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 13

[Kathleen :Brade]

fix checked in

Comment 14

[benc]

mcs: if this works now, can you mark VERIFIED?

Comment 15

[Mark Smith [:mcs]]

Verified with Mozilla 1.0rc2 on Solaris (as far as I know, 1.0 final builds are
not available yet for Solaris).