Closed Bug 1061898 Opened 5 years ago Closed 5 years ago

Firefox fails to parse directory listing of Microsoft FTP Server with FtpDirBrowseShowLongDate set

Categories

(Core :: Networking: FTP, defect)

31 Branch
x86
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla36

People

(Reporter: pdknsk, Assigned: Gijs)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:31.0) Gecko/20100101 Firefox/31.0
Build ID: 20140715214335

Steps to reproduce:

It appears that most Microsoft FTP Server return the following directory listing in response to LIST.

10-10-14  10:10AM       <DIR>        FTP

I have encountered a server which returns the following format.

10-10-2014  10:10AM       <DIR>        FTP

Firefox cannot parse this apparently, and renders no directory listing.

Works fine in Chrome and other FTP clients.

PS. The IIS FTP option to configure this is described at this URL.

http://msdn.microsoft.com/windows/desktop/ms524462
Summary: Firefox fails to parse date directory listing of Microsoft FTP Server with FtpDirBrowseShowLongDate set → Firefox fails to parse directory listing of Microsoft FTP Server with FtpDirBrowseShowLongDate set
Component: Untriaged → Networking: FTP
Product: Firefox → Core
This shouldn't be too hard to fix, although it seems the parser has hardcoded integer offsets into that dirlisting specified... which makes me nervous. There's also a test for it, but that's disabled because it relies on caching, which is broken now that we have cache2. That's bug 913827, but the idea there is to just remove caching, which means the test needs a different idea. :-(
Depends on: 913827
FTP is on life support at this point.  If someone gives us a patch, we'll consider it, but we have no automated testing, and no list of canonical FTP servers to test against, so really I'm reluctant to change the code unless someone commits to widespread manual testing.
Re bug 913807 comment 61:

> Is there a bug on file on fixing and re-enabling the FTP tests that were disabled 

Honza didn't answer clearly: there are no FTP automated tests.  Whether FTP continues using the HTTP cache to store directory info does not block anything in this bug.
No longer depends on: 913827
(In reply to Jason Duell (:jduell) from comment #4)
> Re bug 913807 comment 61:
> 
> > Is there a bug on file on fixing and re-enabling the FTP tests that were disabled 
> 
> Honza didn't answer clearly: there are no FTP automated tests.  Whether FTP
> continues using the HTTP cache to store directory info does not block
> anything in this bug.

I mean, the parsing of the directory lists actually has (disabled) tests:

http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_bug365133.js
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_bug484684.js
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_bug515583.js

that rely on the cache behaviour, but otherwise seem reasonably effective to test the directory list part...
Ah--I didn't know that.  Good to know.
It turns out that IIS can switch to UNIX directory listings, but of course there is no way to tell Firefox to try it.

ftp> site dirstyle
200 MSDOS-like directory output is off.
ftp> site dirstyle
200 MSDOS-like directory output is on.
Duplicate of this bug: 1094167
The dupe has a test URL:

ftp://ftp.topsolid.com/public/topsolid/dvd/

which allowed me to test a 1-line patch, which actually seems to work fine...
Not sure what the risk balance is here when FTP is already on life support. Unfortunately I don't have time to rework the automated tests right now, but on the other hand, the patch is so simple that I figured I might as well ask for review anyway. :-)

Note also that there is code further down: http://hg.mozilla.org/mozilla-central/annotate/62990ec7ad78/netwerk/streamconv/converters/ParseFTPList.cpp#l817 that deals with the longer year format, so it seems that to some degree this was meant to work already?
Attachment #8517417 - Flags: review?(jduell.mcbugs)
Assignee: nobody → gijskruitbosch+bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8517417 [details] [diff] [review]
fix ftp dir listing for windows long dates,

Review of attachment 8517417 [details] [diff] [review]:
-----------------------------------------------------------------

OK this looks simple enough and you've tested it enough for me to +r. 

Thanks for the patch!
Attachment #8517417 - Flags: review?(jduell.mcbugs) → review+
https://hg.mozilla.org/mozilla-central/rev/e10a2ca070e6
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Works in FF36. Thanks.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.