491688 - nsIFile::Exists returns true for directories that don't have an FSRef, breaks FireFTP

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Markus Stange [:mstange]]

Steps to reproduce:
1. Turn of extension compatibility checking (set extensions.checkCompatibility
   to false in about:config).
2. Visit https://addons.mozilla.org/en-US/firefox/addons/versions/684 and
   install FireFTP version 1.0.4.
3. Restart Firefox.
4. Start FireFTP (Tools -> FireFTP)

Actual results:
FireFTP brings up an error dialog with the text "You do not have the appropriate permissions or directory does not exist." Moreover, the directory list in the left pane is incomplete.

Expected results:
The error message shouldn't occur.

I've backed bug 490870 out locally and verified that it fixes the problem.
I haven't done any further investigation. This might not even be a valid bug, so I'm filing it as unconfirmed.

Comment 1

[Markus Stange [:mstange]]

Requesting blocking for investigation.

Comment 2

[Mime Čuvalo]

Attachment: Testcase exhibiting problem

Alright, I finally got some time to sit down at a friend's Mac and hack this out.  Attached is a testcase.  This is how to reproduce:
 1.) Save testcase to local directory
 2.) Input '/'  (without quotes) into textfield
 3.) Click Test Directory
 4.) The directory '/dev' fails on the isHidden() test

Comment 3

[Mime Čuvalo]

Apologies, btw, for taking so long to look into this.  :-/

Comment 4

[Markus Stange [:mstange]]

Thanks, Mime!

So the problem here is that CFURLGetFSRef fails for '/dev'. But nsIFile::Exists returns true. Before bug 490870, nsIFile::Exists used GetFSRefInternal, so it returned false, and thus /dev wasn't listed as a subdirectory of '/'.

Josh, what's the right behavior here? Why is /dev special?

Comment 5

[Josh Aas]

/dev may not be a normal filesystem entry, I don't know how the Mac OS X kernel exposes it. FSRef objects are basically direct disk inode references and the /dev directory is probably not an inode of the type it knows how to reference. For example, /dev might be inserted by the kernel at a higher level than the disk itself and while the kernel takes it into account for certain system calls (like "stat" in this case) you wouldn't really be able to get an inode reference to it.

I used to know this stuff about the Linux kernel but that was a while ago and I never looked into it on Mac OS X so I could be wrong. In any case, we should probably keep killing off FSRef usage to fix this bug.

Comment 6

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

Need to fix, either by fixing or by backing out the change which caused the regression.

Comment 7

[Josh Aas]

Attachment: fix v1.0

I'm out of here for the night (go Phillies!) but this is basically what we want. It works, I just don't have time to verify that this is really how we want to do it. Back on it tomorrow...

Comment 8

[Josh Aas]

Attachment: fix v1.1

Comment 9

[Benjamin Smedberg]

Tests?

Comment 10

[Josh Aas]

Attachment: basic test v1.0

I don't think we can meaningfully test the real bug here, which has to do with the difference between files on disk and virtual components in the filesystem. That really has nothing to do with hidden files. We could use some general hidden file coverage though. This is a start but I can't test it right now because of bug 513747.

Comment 11

[Josh Aas]

Attachment: basic test v1.1

Comment 12

[Josh Aas]

pushed fix and test to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/3410e420f7bf

Comment 13

[Josh Aas]

pushed to mozilla-1.9.2

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b5d85a30e144

Comment 14

[Marcia Knous [:marcia]]

Verified fixed on the 1.9.2 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2b2pre) Gecko/20091104 Namoroka/3.6b2pre. I verified using both the testcase and the STR in Comment 0. Adding keyword.