Closed
Bug 1061898
Opened 10 years ago
Closed 9 years ago
Firefox fails to parse directory listing of Microsoft FTP Server with FtpDirBrowseShowLongDate set
Categories
(Core Graveyard :: Networking: FTP, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla36
People
(Reporter: u473386, Assigned: Gijs)
References
Details
Attachments
(1 file)
1.61 KB,
patch
|
jduell.mcbugs
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Updated•10 years ago
|
Component: Untriaged → Networking: FTP
Product: Firefox → Core
Assignee | ||
Comment 2•10 years ago
|
||
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
Comment 3•10 years ago
|
||
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.
Comment 4•10 years ago
|
||
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
Assignee | ||
Comment 5•10 years ago
|
||
(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...
Comment 6•10 years ago
|
||
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.
Assignee | ||
Comment 9•10 years ago
|
||
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...
Assignee | ||
Comment 10•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 11•9 years ago
|
||
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+
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e10a2ca070e6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•1 month ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•