361351 - nsIFile.exists() should throw NOT_INITIALIZED if nsIFile is not initialized

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[paulie4]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

nsIFile.exists() causes an error if nsIFile is not initialized, even though it just returns false in Windows.  Returning false seems like the most reasonable thing to do.  This makes programming cross platform extensions more tedious.

Reproducible: Always

Steps to Reproduce:
1.var file = Components.classes["@mozilla.org/file/local;1"].createInstance(Components.interfaces.nsIFile);
2.file.exists();

Actual Results:  
NS_ERROR_NOT_INITIALIZED on line 1: Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIFile.exists]

(just false on Windows)

Expected Results:  
false

Comment 1

[Andrew Schultz]

I would assert that the behavior on linux is entirely correct.  But I do agree that the same behavior should exist on all platforms.

Comment 2

[Benjamin Smedberg]

Expected behavior should be to throw NOT_INITIALIZED on all platforms.

Comment 3

[paulie4]

How does one check for initialization?
(nsIFile.nativePath != "") OR (nsIFile.path != "")?

Comment 4

[Andrew Schultz]

OK, this bug exists on at least Win and OSX.  Unix checks for initialization here:

http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileUnix.cpp#109

Comment 5

[Ryan Jones-Ward [:sciguyryan]]

(In reply to comment #4)
> OK, this bug exists on at least Win and OSX.  Unix checks for initialization
> here:
> 
> http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileUnix.cpp#109
> 

The same thing (or similar) can be done on Windows and Mac too then? I'll try and look at this tomorrow after school.

Comment 6

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Like this? (nsLocalFileWin v1)

* Attempt v1. Only patch for nsLocalFileWin.cpp - if this looks correct I'll make a similar patch for Mac and OS2 if its required.

Comment 7

[Darin Fisher]

Comment on attachment 246490 [details] [diff] [review]
Like this? (nsLocalFileWin v1)

Do all of the other platform ports behave the same?  Is this something you could create a PASS/FAIL unit test for in mozilla/xpcom/tests?  

Maybe extend nsIFileTest.cpp or create a new test that mimics how TestTArray.cpp and TestStrings.cpp are written.

Comment 8

[Ryan Jones-Ward [:sciguyryan]]

It looks like Mac already does this:

http://lxr.mozilla.org/mozilla/source/xpcom/io/nsLocalFileOSX.cpp#417

For example. OS2 does appear to be affected and I will submit a patch shortly.

Comment 9

[Ryan Jones-Ward [:sciguyryan]]

Attachment: nsLocalFileOS2 v1

* Patch v1 for nsLocalFileOS2.

Comment 10

[Ryan Jones-Ward [:sciguyryan]]

I've checked Mac and Unix and they seem to be covered in this case.(In reply to comment #7)
> (From update of attachment 246490 [details] [diff] [review] [edit])
> Do all of the other platform ports behave the same?  Is this something you
> could create a PASS/FAIL unit test for in mozilla/xpcom/tests?  
> 
> Maybe extend nsIFileTest.cpp or create a new test that mimics how
> TestTArray.cpp and TestStrings.cpp are written.
> 

Would a test be needed in this case? If so I'll try and create one.

Comment 11

[Andrew Schultz]

> It looks like Mac already does this:

not for exists():

http://lxr.mozilla.org/mozilla/source/xpcom/io/nsLocalFileOSX.cpp#915

Comment 12

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Merged patch v1

* Merged patch v1.

This is a merged patch containing the changes too nsLocalFileOSX, nsLocalFileWin and nsLocalFileOS2.

If there are any other functions that require initialization checking let me know and I will make a new patch as required.

Comment 13

[Ryan Jones-Ward [:sciguyryan]]

I'll get working on a test for this as soon as I get a little free time (hopefully this week).

Comment 14

[:Gavin Sharp [email: [e-mail]]]

mozilla/xpcom/io/nsLocalFileOS2.cpp 	1.93
mozilla/xpcom/io/nsLocalFileOSX.cpp 	1.45
mozilla/xpcom/io/nsLocalFileWin.cpp 	1.168

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Attachment: additional Mac bustage fix (checked in)