-
Notifications
You must be signed in to change notification settings - Fork 45
option to preserve the permissions #42
Comments
With the proposed option turned on, when creating new files I think it would also be good to obey the system's umask instead of creating files with restricted permissions. Until there is an option for this, I'm using this code as a workaround: class AtomicWriterBetterPermissions(AtomicWriter):
def commit(self, f):
try:
mode = stat(self._path).st_mode
except FileNotFoundError:
# Creating a new file, emulate what os.open() does
umask = os.umask(0)
umask(umask)
mode = 0o777 & ~umask
os.chmod(f.name, mode)
super().commit(f) |
isn't there a possibility of a race here too? it seems to me the chmod should happen on a file descriptor, not a path. |
Good point. I think I should also move the permission change time to
before the temporary file is written to, since that could take a while
and the original file could disappear or change modes in the meantime.
```python
class AtomicWriterPerms(AtomicWriter):
def get_fileobject(self, **kwargs):
f = super().get_fileobject(**kwargs)
try:
mode = os.stat(self._path).st_mode
except FileNotFoundError:
# Creating a new file, emulate what os.open() does
umask = os.umask(0)
os.umask(umask)
mode = 0o777 & ~umask
fd = f.fileno()
os.fchmod(fd, mode)
return f
```
…--
bye,
pabs
https://wiki.debian.org/PaulWise
|
If you figure out a way to do this without any races lmk. I personally have no usecase for this and would think that people who care about this just call chmod with a fixed mask after writing the file to force it to a hardcoded value. |
This is my current workaround. There will always be a race since the
original file could change permissions between when the original
file permissions are read and when the new file gets the permissions
changed to the original permissions or it could even change permissions
while the new file is being written to.
```
from os import stat, umask, fchmod
from atomicwrites import AtomicWriter, atomic_write
class AtomicWriterPerms(AtomicWriter):
def get_fileobject(self, **kwargs):
f = super().get_fileobject(**kwargs)
try:
mode = stat(self._path).st_mode
except FileNotFoundError:
# Creating a new file, emulate what os.open() does
mask = umask(0)
umask(mask)
mode = 0o777 & ~mask
fd = f.fileno()
fchmod(fd, mode)
return f
```
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
+1 on this. Changing a file's permissions when overwriting is not expected behavior. |
Since the umask is per-process, not per-thread, changing it is a potential security problem in the presence of multiple threads. (See https://bugs.python.org/issue21082.) That’s also why it’s hard for callers to “just call chmod with a fixed mask after writing the file”—it’s difficult to compute the correct mask in a threadsafe way. My approach for the corresponding bug in Click avoids this problem by replacing |
My recommendation was to set it to a value that is hardcoded in your software. I recognize that this is not always possible but it's certainly the least likely to cause races. |
When I overwrite a file, the permissions of the file are changed:
The normal non-atomic method of overwriting a file does not change the mode:
It would be nice to have an option to preserve the mode of the original file.
The text was updated successfully, but these errors were encountered: