397363 - nsLocalFileOSX doesn't play nice with dangling symlinks

Status: RESOLVED DUPLICATE of bug 207973

(Core :: XPCOM, defect)

Description

[jag (Peter Annema)]

Say you do ln -s doesNotExist aSymLink in /test and then you try to view it with file:///test/, on Mac I get "Page Load Error" with "File Not Found". It looks as thought this is due to nsLocalFile::GetFSRefInternal() failing because ::CFURLGetFSRef(...) will automatically follow symlinks, and fail because the target file doesn't exist.

Comment 1

[jag (Peter Annema)]

So the callers of GetFSRefInternal() are things like GetFileSize() and GetLastModifiedTime(). Maybe whatever iterates over the files to generate the directory view could handle an error returned from any of these a little more gracefully too.

Checking Google for how to deal with this, the suggestion seems to be to get the parent directory and leaf name from the CFURL and create an FSRef from those two:
http://lists.apple.com/archives/cocoa-dev/2004/Jan/msg01572.html

I was thinking we could put that into GetFSRefInternal() for the else case of CFURLGetFSRef(). Maybe do some check to make sure that |whichURLRef| is indeed a symlink.

Per UpdateTargetRef(), if mFollowLinks and mBaseRef is a dangling symlink, then mTargetLink == mBaseRef, so you could just check mBaseRef above. I wonder though what happens if mBaseRef is an alias to a symlink, will FSResolveAliasFile() make fsRef point at the linked file, or at the link? If the latter, then mTargetLink != mBaseRef and GetFSRefInternal() with mFollowLinks would fail, and you do want to use |whichURLRef|.

Comment 2

[jag (Peter Annema)]

Attachment: Sample implementation and some leak-on-error fixes

So, this works, and is most likely wrong, seeing how I'm not very familiar with this stuff.

Comment 3

[jag (Peter Annema)]

Attachment: Sample implementation and some leak-on-error fixes

Ignore the other stuff in the previous attachment.

Comment 4

[jag (Peter Annema)]

While this makes things work, is this even the right approach? Should we stop using CFURLRefs?

Comment 5

[jag (Peter Annema)]

Hrm. Maybe we only want to do this when !mFollowLinks? If the target file doesn't exist, should we report that back if followLinks is set to true? I guess I'll look at what nsLocalFileUnix does, though since the dangling symlink test worked on Linux, I suspect they give the info for the link if the target is missing.

If we decide to change that behaviour, we'll have to fix nsDirectoryIndexStream.cpp and whatever else expects the old behaviour.

Comment 6

[jag (Peter Annema)]

Comment on attachment 282143 [details] [diff] [review]
Sample implementation and some leak-on-error fixes

Could you review this, Josh? I don't think this is the final patch, but I'd at least like your input on the approach. Or punt review to someone else you think is more qualified.

Comment 7

[Josh Aas]

Comment on attachment 282143 [details] [diff] [review]
Sample implementation and some leak-on-error fixes

I'd like to help, but right now I can't really spend time on things that aren't blocking1.9+, or highly requested wanted1.9+. Perhaps Mark can help you, he's probably better with this type of code than me anyway.