Closed Bug 682571 Opened 8 years ago Closed 8 years ago

nsLocalFile::isWritable behaves wrongly on Windows

Categories

(Core :: XPCOM, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: bbondy, Assigned: bbondy)

References

Details

Attachments

(1 file, 2 obsolete files)

nsLocalFile::isWritable behaves wrongly on Windows.

1) For read only file shares, we return true:
var file = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile); file.initWithPath("\\\\rhesus\\readonlyshare\\1.txt"); file.isWritable()

> true

This should not return true, it should return false.


2) For files that are exclusively opened or locked:
var file = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile); file.initWithPath("C:\\hiberfil.sys"); file.isWritable()
NS_ERROR_FILE_NOT_FOUND on line 3: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsILocalFile.isWritable]

This should not throw an error, it should return false.

3) For files that have no permission (security):

var file = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile); 
file.initWithPath("C:\\restricted.txt"); 
file.isWritable();
> true

This should not return true, it should return false
Setting you as the reviewer since there are 2 similar bugs I already requested from you.
Attachment #556553 - Flags: review?(benjamin)
I don't think that #2 is correct: if a file would normally be writeable but is currently locked, we should still return `true`.
Attachment #556553 - Flags: review?(benjamin) → review-
OK I'll change that. The documentation is vague on if it means currently writeable or writeable in general regarding permissions. They are the same on other platforms more or less (other than if you mount your filesystem with mandatory locks), but differ quite a bit on Windows.  

I'll clarify on this point at https://developer.mozilla.org/en/nsIFile
Works the same as before except it will return true for #2. 
I also tested other cases and they work as expected.

Summary:
- Normal file: IsWritable -> true
- Read only file share's file: IsWritable -> false
- Exclusively opened or locked but normally writeable: IsWritable -> true
- Exclusively opened or locked and read only flag: IsWritable -> false
- Exclusively opened or locked and no permissions: IsWritable -> false
- No write permissions on local file: IsWritable -> false
- Read only flag set on file: IsWritable -> false
Attachment #556553 - Attachment is obsolete: true
Attachment #558896 - Flags: review?(benjamin)
Comment on attachment 558896 [details] [diff] [review]
Patch for the 3 problems mentioned for nsLocalFile::IsWritable. v2.

+    if(*aIsWritable) {

unsnuggle the if (twice in this patch)
Attachment #558896 - Flags: review?(benjamin) → review+
Old habits die hard.  Fixed the snuggled parentheses and a couple others in the same file not related to my changes.
Attachment #558896 - Attachment is obsolete: true
Attachment #558904 - Flags: review+
I put a reminder in my calendar to update the documentation regarding the meaning of isWritable once this reaches release.  If you'd prefer I update it now just let me know.
Do it now, with release-specific notes as necessary. Sheppy may add it to the "for developers" page too, although this is minor enough that it's probably not worth it.
Keywords: dev-doc-needed
Removing dev-doc-needed keyword since it's already updated as per Comment 9.
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/225001455808
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.