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

Status: RESOLVED FIXED

(Core :: Networking: File, defect)

Description

[benc]

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.

Comment 1

[Krishna Sethuraman]

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 ->
[e-mail]..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.

Comment 2

[benc]

CONFRIMED: Mozilla 1.7b, Mac OS X 10.3.
Doesn't happen in Linux, 17branch build.

Comment 3

[benc]

*** Bug 255735 has been marked as a duplicate of this bug. ***

Comment 4

[Darin Fisher]

-> default owner

Comment 5

[Dave Townsend [:mossop]]

This looks to be the same as bug 397363

Comment 6

[jag (Peter Annema)]

Attachment: Behave like nsLocalFileUnix for dangling symlinks

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.

Comment 9

[jag (Peter Annema)]

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.

Comment 10

[jag (Peter Annema)]

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.

Comment 11

[jag (Peter Annema)]

Attachment: Behave like nsLocalFileUnix for dangling symlinks v2

Construct an FSRef the hard way since the easy way fails on dangling symlinks.

Comment 12

[Benjamin Smedberg]

See related bug 481369: how does this patch interact?

Comment 13

[jag (Peter Annema)]

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().

Comment 14

[Benjamin Smedberg]

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!

Comment 15

[jag (Peter Annema)]

Attachment: Behave like nsLocalFileUnix for dangling symlinks v2b

Comment 16

[jag (Peter Annema)]

I was trying to avoid the extra addref/release, but it probably doesn't matter here.

Comment 17

[jag (Peter Annema)]

Pushed changeset 0288a48bd3ab to mozilla-central.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this testable?

Comment 19

[(not currently active) Ted Mielczarek]

Should be, see:
http://mxr.mozilla.org/mozilla-central/source/xpcom/tests/unit/test_bug476919.js

Which tests nsIFile::isSymlink

Comment 20

[jag (Peter Annema)]

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.

Comment 21

[Benjamin Smedberg]

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.

Comment 22

[jag (Peter Annema)]

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.

Comment 23

[jag (Peter Annema)]

Attachment: Unit test for symlinks on Mac and other Unixen

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.

Comment 24

[jag (Peter Annema)]

Attachment: Unit test for symlinks on Mac and other Unixen

Updated with lots of testing help from Neil on Linux.

Comment 25

[jag (Peter Annema)]

Attachment: Unit test for symlinks on Mac and other Unixen v1.1

Comment 26

[jag (Peter Annema)]

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.

Comment 27

[jag (Peter Annema)]

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 28

[Mike Beltzner [:beltzner, not reading bugmail]]

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.