148704 - nsLocalFileWin::IsWritable() and directories.

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jon Smirl]

I'm porting some code from Unix to Windows and the Unix version calls 
nsLocalFile::IsWritable() with a directory to make sure the directory is 
writable. On Windows nsLocalFileWin::IsWritable() calls ::GetFileAttributes() 
with the path. The return is checked against FILE_ATTRIBUTE_READONLY. The doc 
for ::GetFileAttributes(), 
http://msdn.microsoft.com/library/default.asp?url=/library/en-
us/fileio/filesio_9pgz.asp?frame=true , says:
"FILE_ATTRIBUTE_READONLY The file or directory is read-only. Applications can 
read the file but cannot write to it or delete it. In the case of a directory, 
applications cannot delete it."  This doesn't say anything about file create 
permission in a directory. I'm returning FALSE from the call even though I 
believe I have write permission to the directory (it's on a FAT32 drive).

I can work around this by removing the check in my code for write permissions 
and letting the code fail later when it tries to create the file.

Comment 1

[Jon Smirl]

I'm not really sure what Windows is trying to do here. IsWritable() is 
returning false because the directory is a system directory and it has the 
ReadOnly bit set; but I can still create files in the directory even though it 
is set ReadOnly. Readonly on directories may only mean that they can't be 
deleted.

Best solution might be to check for FILE_ATTRIBUTE_DIRECTORY and always return 
true for a directory.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

confirming (the code is still unchanged)

Comment 3

[Benjamin Smedberg]

We should really be checking NTFS permissions in addition to readonly bits.

Comment 4

[Brodie]

I can create a patch so that we only return read-only for files. Directories 
are always writable, at least on FAT. I've not looked into NTFS permissions 
before, Benjamin, can you provide some information on how to do so?

Comment 5

[Brodie]

Adding checks for NTFS permissions can be a new RFE. It's not as simple as FAT
read/write permissions. It seems to involve getting the current user SID, and
the object DACL using GetNamedSecurityInfo then running through the list seeing
if the current user SID has write/modify permissions. 

Comment 6

[Brodie]

Attachment: patch 1

This returns TRUE if the target is a directory. Otherwise it uses the previous
code for a file.

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 153958 [details] [diff] [review]
patch 1

this seams reasonable.

Comment 8

[Darin Fisher]

Comment on attachment 153958 [details] [diff] [review]
patch 1

r=darin

thanks again for the great work, brodie!

Comment 9

[Brodie]

Checking in xpcom/io/nsLocalFileWin.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v  <--  nsLocalFileWin.cpp
new revision: 1.129; previous revision: 1.128
done