Closed Bug 207973 Opened 17 years ago Closed 11 years ago

Directory listing errors when a bad soft link (symlink) is present

Categories

(Core :: Networking: File, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: benc, Assigned: jag+mozilla)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

Mozilla 1.4b - Mac OS X 10.2.6

STEPS:

ln -s bad badlink

View the directory w/ a file URL.

OBSERVED:

Index of file:///Users/benc/testing/linktest
Up to higher level directory

(no listing)

EXPECTED:
Directory would be displayed somehow.

The file picker in Mac OS has the bad links as greyed.

NOTE: I think that dir->HTML is doing some per-file poking around, because the
directory listing checks links to see if they point to a file or a directory,
and then displays the correct icon.
Component: Networking → Networking: File
MacOSX 10.2.8/Mozilla nightly 2004040305.
I ran into this too, except it wouldn't pull up a blank listing, but gives this
dialog:

The file /Users/krishna/blah/blah cannot be found.  Please check the location
and try again.

Editing a file (blah.txt) in that directory in xemacs where outstanding edits
haven't yet been saved produces a .#blah.txt ->
krishna@Krishnas-computer.local..1314 dangling soft link.  I believe xemacs uses
this as a locking/modification control identifier.  Saving the file removed the
link, and I could immediately view the directory listing.
CONFRIMED: Mozilla 1.7b, Mac OS X 10.3.
Doesn't happen in Linux, 17branch build.
Summary: Directory listing is blank when a bad soft link is present → Directory listing errors when a bad soft link is present
*** Bug 255735 has been marked as a duplicate of this bug. ***
-> default owner
Assignee: darin → nobody
QA Contact: benc → networking.file
This looks to be the same as bug 397363
Also added in the leak fixes from bug 397363.

Alternatively, we embrace returning NS_FILE_NOT_FOUND from IsFile() and IsDirectory() for dangling symlinks, and fix nsDirectoryIndexStream.cpp to not bail if the returned error is NS_FILE_NOT_FOUND.
Assignee: nobody → jag
Hardware: PowerPC → All
Duplicate of this bug: 397363
Blocks: 345835
Duplicate of this bug: 479277
Comment on attachment 363873 [details] [diff] [review]
Behave like nsLocalFileUnix for dangling symlinks

Based on the wording of https://developer.mozilla.org/en/nsIFile (nsIFile.idl is rather lacking in spec for isFile and isDirectory), and the principle of least surprise, I'd say let's make this work like nsLocalFileUnix.
Attachment #363873 - Flags: superreview?(doug.turner)
Attachment #363873 - Flags: review?(joshmoz)
Summary: Directory listing errors when a bad soft link is present → Directory listing errors when a bad soft link (symlink) is present
Hrm, we might also want to add something to Reveal(), CopyInternal(), {Get,Set}Permissions(), etc. etc.

I'm starting to think my solution in bug 397363 was better, or at least it brings us closer to what nsLocalFileUnix does (it falls back on lstat if stat fails). Let me see if I can unbitrot that patch.
Construct an FSRef the hard way since the easy way fails on dangling symlinks.
Attachment #363873 - Attachment is obsolete: true
Attachment #363887 - Flags: superreview?(doug.turner)
Attachment #363887 - Flags: review?(joshmoz)
Attachment #363873 - Flags: superreview?(doug.turner)
Attachment #363873 - Flags: review?(joshmoz)
Attachment #363887 - Flags: review?(joshmoz) → review+
Attachment #363887 - Flags: superreview?(doug.turner) → superreview?(benjamin)
See related bug 481369: how does this patch interact?
There's no interaction, from what I can tell. The leak fixes shouldn't affect bug 481369 at all, nor should the change I'm making for this bug. None of the code in HasMoreElements() uses GetFSRefInternal().
Attachment #363887 - Flags: superreview?(benjamin) → superreview+
Comment on attachment 363887 [details] [diff] [review]
Behave like nsLocalFileUnix for dangling symlinks v2

This is ok, but I'd be a lot happier if it used nsRefPtr<nsLocalFile> instead of manually deleting!
Attachment #363887 - Attachment is obsolete: true
Attachment #365483 - Flags: superreview?(benjamin)
Attachment #365483 - Flags: review+
I was trying to avoid the extra addref/release, but it probably doesn't matter here.
Attachment #365483 - Flags: superreview?(benjamin) → superreview+
Pushed changeset 0288a48bd3ab to mozilla-central.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #365483 - Flags: approval1.9.1?
Can we put a test directory with symlinks in Mercurial, or should these be created and cleared as part of the test?

If we can have a dangling symlink somewhere to test, then it should be as simple as iterating over that directory's contents (or just directly going to the file) and making sure isFile, isDirectory and isSymlink don't throw an exception.

Bonus points if we can also add resolvable symlinks to a file, a directory, and a symlink.

I can do the legwork, but I could use some help with whether we can simply put the symlinks in hg, or if we can create them (and where, safely, and clean up afterwards) from the test.
I don't think we should try to put symlinks in hg.

I suppose that we don't have an easy way from within XPCOM to create symlinks? You could of course use nsIProcess and `ln -s` but that might get annoying.
Looks like comment 19 has all I need. I didn't realize these files would be created in _tests, but that takes care of my remaining worry. I'll try to create a good test for this.
Blocks: symlink
I just wanted to test this bug, but I've exposed (and filed and worked around) a few others, so I'm thinking this could be the start of a symlink unit test.
Attachment #368635 - Flags: review?(ted.mielczarek)
Attachment #368635 - Flags: review?(ted.mielczarek) → review?(benjamin)
Attachment #368635 - Flags: review?(benjamin) → review+
Updated with lots of testing help from Neil on Linux.
Attachment #368635 - Attachment is obsolete: true
Attachment #369486 - Flags: review?(benjamin)
Attachment #369486 - Attachment is obsolete: true
Attachment #370308 - Flags: review?(benjamin)
Attachment #369486 - Flags: review?(benjamin)
Comment on attachment 370308 [details]
Unit test for symlinks on Mac and other Unixen v1.1

I got r=ted on irc if I replace |__LOCATION__.parent| with |do_get_cwd()| and make sure the lack of tests won't make this fail on Windows. Try sez I'm good to go.
Attachment #370308 - Flags: review?(benjamin) → review+
Comment on attachment 370308 [details]
Unit test for symlinks on Mac and other Unixen v1.1

Test checked in: http://hg.mozilla.org/mozilla-central/rev/fce0de0377e5
Comment on attachment 365483 [details] [diff] [review]
Behave like nsLocalFileUnix for dangling symlinks v2b

Not sure that we need to change this at this time, as it changes the behaviour of the API pretty late in the development cycle for a non-essential fix.
Attachment #365483 - Flags: approval1.9.1? → approval1.9.1-
You need to log in before you can comment on or make changes to this bug.