Closed Bug 148704 Opened 22 years ago Closed 20 years ago

nsLocalFileWin::IsWritable() and directories.

Categories

(Core :: XPCOM, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jonsmirl, Assigned: bmo)

Details

Attachments

(1 file)

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.
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.
confirming (the code is still unchanged)
Status: UNCONFIRMED → NEW
Ever confirmed: true
We should really be checking NTFS permissions in addition to readonly bits.
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?
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. 
Attached patch patch 1Splinter Review
This returns TRUE if the target is a directory. Otherwise it uses the previous
code for a file.
Assignee: dougt → brofield
Status: NEW → ASSIGNED
Attachment #153958 - Flags: superreview?(dougt)
Attachment #153958 - Flags: review?(darin)
Comment on attachment 153958 [details] [diff] [review]
patch 1

this seams reasonable.
Attachment #153958 - Flags: superreview?(dougt) → superreview+
Comment on attachment 153958 [details] [diff] [review]
patch 1

r=darin

thanks again for the great work, brodie!
Attachment #153958 - Flags: review?(darin) → review+
Checking in xpcom/io/nsLocalFileWin.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v  <--  nsLocalFileWin.cpp
new revision: 1.129; previous revision: 1.128
done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: