Last Comment Bug 429484 - Win32 nsILocalFile.exists() returns false for an existing system file (such as hiberfile.sys or pagefile.sys)
: Win32 nsILocalFile.exists() returns false for an existing system file (such a...
Status: VERIFIED FIXED
[bugday-20110401]
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: unspecified
: x86 Windows XP
: P4 minor with 1 vote (vote)
: mozilla9
Assigned To: Brian R. Bondy [:bbondy]
:
: Nathan Froyd [:froydnj]
Mentors:
Depends on:
Blocks: 300692 682571
  Show dependency treegraph
 
Reported: 2008-04-17 05:41 PDT by Zbynek Winkler
Modified: 2011-11-11 08:13 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (693 bytes, application/x-javascript)
2011-03-26 10:29 PDT, Atte Kemppilä [:atte]
no flags Details
Patch for better error handling and exists fix (2.95 KB, patch)
2011-08-26 21:33 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Patch for better error handling and exists fix v2 (3.28 KB, patch)
2011-08-27 06:58 PDT, Brian R. Bondy [:bbondy]
benjamin: review+
Details | Diff | Splinter Review

Description Zbynek Winkler 2008-04-17 05:41:00 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/20080311 Firefox/2.0.0.13
Build Identifier: 2.0.0.13 and 3.0b5

var file = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile);
file.initWithPath("c:\\hiberfil.sys"); 
alert(file.exists());

The code above shows false even though "dir /a c:\hiberfil.sys" shows it exists.
c:\>dir /a c:\hiberfil.sys
 Volume in drive C has no label.
 Volume Serial Number is 3C7A-79DB

 Directory of c:\

14.04.2008  08:33     1 072 091 136 hiberfil.sys
               1 File(s)  1 072 091 136 bytes
               0 Dir(s)   4 735 647 744 bytes free

Also iterating over nsILocalFile.directoryEntries does not show the files either.

Reproducible: Always
Comment 1 Nickolay_Ponomarev 2009-06-12 14:07:00 PDT
I see this on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090611 Shiretoko/3.5pre, no duplicates, confirming.
Comment 2 ondra zara 2011-02-19 01:12:31 PST
Confirming the same on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b11) Gecko/20100101 Firefox/4.0b11.
Comment 3 Atte Kemppilä [:atte] 2011-03-26 10:29:17 PDT
Created attachment 522118 [details]
testcase

If I read the comment 0 correctly, hiberfil.sys and pagefile.sys files are NOT included in directoryEntries of "c:". I don't have XP, so I can't test.

In my win7 + ff36 (Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.16) Gecko/20110319 Firefox/3.6.16) hiberfil.sys and pagefile.sys are NOT included in directoryEntries of "c:". The attached test case opens 2 alerts for me with exists() being false.

In my win7 + ff4 (Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0) hiberfil.sys and pagefile.sys ARE included in directoryEntries of "c:". The attached test case opens 4 alerts for me with exists() being false.

The mentioned files actually exists on my disk.

It doesn't make much sense that directoryEntries return files that don't "exist".

An user (win xp sp3 + ff4) of my extension encountered this [1]. In my code I loop directoryEntries and do isDirectory() on the entry, which of course fails because the mentioned files don't "exist".

[1] http://code.google.com/p/savefileto/issues/detail?id=5
Comment 4 Brian R. Bondy [:bbondy] 2011-08-26 21:33:38 PDT
Created attachment 556213 [details] [diff] [review]
Patch for better error handling and exists fix

If a file exists, and is also either i) exclusively opened, or ii) locked, then we should be returning true from exists. 

If this patch gets r+ I will check the rest of the code base to see if I can spot any problems from changing this behaviour to the proper handling.  And I'll submit a second patch if necessary for any changes.
Comment 5 Brian R. Bondy [:bbondy] 2011-08-27 06:58:33 PDT
Created attachment 556257 [details] [diff] [review]
Patch for better error handling and exists fix v2

Fixed Minor problem from last patch
Comment 6 Brian R. Bondy [:bbondy] 2011-09-20 13:48:29 PDT
Pushed to try:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=7372a2522155
Comment 7 Brian R. Bondy [:bbondy] 2011-09-21 09:51:27 PDT
Pushed to inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/519c7eed899a
Comment 8 Ed Morley [:emorley] 2011-09-21 18:14:54 PDT
https://hg.mozilla.org/mozilla-central/rev/519c7eed899a
Comment 9 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-21 21:25:46 PDT
Should this be set to status-firefox9:fixed? 

Also, how do I utilize the attached testcase? It's not completely clear to me how to use it to verify the fix.
Comment 10 Brian R. Bondy [:bbondy] 2011-09-21 21:32:08 PDT
> Should this be set to status-firefox9:fixed? 

I'm not sure but it affects Core (all products) so I think it is correctly marked.

> how do I utilize the attached testcase

The easiest way is probably to use the Developer Assistant add-on:
https://addons.mozilla.org/en-US/firefox/addon/extension-developer/

Once installed right click somewhere near your URL bar and customize, and add the "Javascript Environment" button.  

Once added click it to open and paste the code from the test case. 

Please let me know if you want me to document the code more in the test case to explain it.
Comment 11 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-22 11:46:54 PDT
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110922 Firefox/9.0a1

Tested using the following steps:
1) Install https://addons.mozilla.org/en-US/firefox/addon/extension-developer/
2) Add "Javascript Environment" button to the main toolbar
3) Create a temp file called hiberfil.sys in C:\ (was necessary due to lack of file)
4) Copy the following code:

var file = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile);
file.initWithPath("c:\\hiberfil.sys"); 
alert(file.exists());

5) Click the "Javascript ENV" button on the toolbar
6) Paste the copied code
7) Click the Execute button

Result:
Alert dialog displaying true.
Repeating after removing the file displays false.

Setting to VERIFIED FIXED.
Comment 12 ondra zara 2011-09-22 12:07:03 PDT
This was not much of a test; the followed steps would work even before this fix was applied. 

This patch solves an issue which is encountered *exclusively* with system-locked files (such as the *real* hiberfil.sys), not with any other arbitrary (user-created) files.

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #11)
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110922 Firefox/9.0a1
> 
> Tested using the following steps:
> 1) Install
> https://addons.mozilla.org/en-US/firefox/addon/extension-developer/
> 2) Add "Javascript Environment" button to the main toolbar
> 3) Create a temp file called hiberfil.sys in C:\ (was necessary due to lack
> of file)
> 4) Copy the following code:
> 
> var file =
> Components.classes['@mozilla.org/file/local;1'].createInstance(Components.
> interfaces.nsILocalFile);
> file.initWithPath("c:\\hiberfil.sys"); 
> alert(file.exists());
> 
> 5) Click the "Javascript ENV" button on the toolbar
> 6) Paste the copied code
> 7) Click the Execute button
> 
> Result:
> Alert dialog displaying true.
> Repeating after removing the file displays false.
> 
> Setting to VERIFIED FIXED.
Comment 13 Brian R. Bondy [:bbondy] 2011-09-22 12:14:58 PDT
> the followed steps would work even before this fix was applied. 

It would return false before this patch.  After this patch it would return true.

Additional things that can be tried to verify no regressions were introduced (these were both tried already by me but you can verify as well):
- Try an exclusively opened file like: C:\Users\bbondy\NTUSER.dat should still return true.
- Try a file which is not open and make sure it still returns true as it did before.


ondra zara if you have extra ideas please let us know. Thanks!
Comment 14 ondra zara 2011-09-22 12:30:06 PDT
(In reply to Brian R. Bondy [:bbondy] from comment #13)

> 
> ondra zara if you have extra ideas please let us know. Thanks!

I have no other ideas and I strongly believe that the patch is correct :)

> > the followed steps would work even before this fix was applied. 
> 
> It would return false before this patch.  After this patch it would return
> true.
> 

It just came to me that when the "c:\hiberfil.sys" file was *not* present in the system prior to verifying, it must have been created (by the verifier) as an ordinary file, thus not being one of those system-locked files - which are the main subject of this issue. But no big deal here...
Comment 15 Atte Kemppilä [:atte] 2011-11-11 08:13:08 PST
I assume that this was fixed for ff9. At least, exists() seems to work in ff9 but other isXxx() methods seem to fail. I filed bug 701721.

Note You need to log in before you can comment on or make changes to this bug.