Small fixes to setting Windows file permissions #31892
Conversation
On Windows files can have a simple read-only permission flag, and the more complex access rights and access control lists. The read-only flag is one bit of the file attributes. To make sure only the read-only flag is toggled and not the other attributes, read the files attributes and write them back with only this bit changed. By reading and setting the attributes from the handle the file does not have to be opened twice. Also directories can not be read-only, here the flag has a different meaning. So we should no longer report directories as read-only, and also not allow making them read-only.
|
One more reason we might want this: currently |
| } else { | ||
| self.set_attributes(attr | c::FILE_ATTRIBUTE_READONLY) | ||
| }, | ||
| (false, true) => self.set_attributes(attr & !c::FILE_ATTRIBUTE_READONLY), |
There was a problem hiding this comment.
Shouldn't this branch also make sure that the file isn't a directory? Or actually, why not just always return an error if it is a directory?
There was a problem hiding this comment.
Oops, yes it should. Thank you!
|
I chose to return I still feel a little sad that you can't simply do |
|
No high-five today? |
|
I messed up git :(. this pr is now included in #31944. So closing this one |
On Windows files can have a simple read-only permission flag, and the more complex access rights
and access control lists. The read-only flag is one bit of the file attributes.
To make sure only the read-only flag is toggled and not the other attributes, read the files
attributes and write them back with only this bit changed. By reading and setting the attributes
from the handle the file does not have to be opened twice.
Also directories can not be read-only, here the flag has a different meaning. So we should no
longer report directories as read-only, and also not allow making them read-only.
Note: This pr is split out from the one I am making for #29497. For that I need to be able to
remove the read-only flags from an already open handle. And the read-only detection needed to be
more robust.