| msg68896 - (view) |
Author: Piotr Lewandowski (pl) |
Date: 2008-06-28 19:23 |
| #v+ $ umask 0077 $ stat /tmp/foobar stat: cannot stat `/tmp/foobar': No such file or directory $ python -c "from mailbox import mbox; m=mbox('/tmp/foobar', create=True); m.add(''); m.close()" $ stat -c '%A' /tmp/foobar -rwx------ #v- Bug is probably present in _create_carefully() function in mailbox.py. os.open() takes mode argument (which defaults to 0777) but it's not supplied there. #v+ $ grep -A2 'def _create_carefully' /usr/lib/python2.5/mailbox.py def _create_carefully(path): """Create a file if it doesn't exist and open for reading and writing.""" fd = os.open(path, os.O_CREAT | os.O_EXCL |
os.O_RDWR) #v- |
|
| msg70591 - (view) |
Author: Jack Diederich (jackdied) *  |
Date: 2008-08-01 20:28 |
| mailbox.py uses os.open instead of the builtin open() because it wants to pass the exclusive flag (O_EXCL). As a result your 0077 umask gets combined with the default of 0777 like this: 0777 & ~0077 == 0700 == '-rwx------' So you probably want to change your default umask when creating mailboxes. Or submit a patch to mailbox.py to allow a different default mode ;) |
|
|
| msg70624 - (view) |
Author: Niels Gustäbel (ngustaebel) |
Date: 2008-08-02 10:01 |
| The problem is not limited to mboxes but also applies to maildirs. Any message file created inside a maildir has execute permissions set as well because the same function _create_carefully() is used. Changing the umask is not an option because it also affects any folders that are created. Setting the umask to something like 0177 would render the base folder and the {new,cur,tmp} folders of a newly created Maildir unusable. To me the only solution is changing the _create_carefully() function, so that it provides a mode argument to os.open() as done in the attached patch. The patch also fixes another instance of this problem: the empty file "maildirfolder" which is created inside a maildir's base folder. It should not be executable as well. |
|
|
| msg70628 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-08-02 12:41 |
| It would be nice if the patch came with a test. |
|
|
| msg70652 - (view) |
Author: Niels Gustäbel (ngustaebel) |
Date: 2008-08-03 10:13 |
| I've added tests for all mailbox formats and the maildirfolder marker, so I think all aspects of the fix are covered. The permissions of temporary files and lock files are not checked, but they are basically all using the _create_carefully() function and they are not persistent anyway. |
|
|
| msg70653 - (view) |
Author: Jakub Wilk (jwilk) |
Date: 2008-08-03 10:40 |
| For consistency with other methods, test_folder_file_permissions() should set umask to 0. |
|
|
| msg70654 - (view) |
Author: Niels Gustäbel (ngustaebel) |
Date: 2008-08-03 10:54 |
| Of course you are right... |
|
|
| msg70675 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2008-08-04 01:44 |
| Committed to trunk in rev. 65472, along with two corresponding tests. |
|
|
| msg70677 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2008-08-04 01:57 |
| There's another use of os.open in the module in the MH class's __setitem__ that doesn't provide a mode, but I think this occurrence is harmless; it's truncating a file that always exists, so the existing mode is preserved. Here's the little test I tried: #!./python.exe import os os.umask(0077) from mailbox import MH m = MH('/tmp/foobar') m.add('123') ; m.add('456') m[1] = 'abc' ; m[2] = 'def' m.close() The files /tmp/foobar/{1,2} both end up with 0600 permissions. |
|
|
| msg70688 - (view) |
Author: Niels Gustäbel (ngustaebel) |
Date: 2008-08-04 08:03 |
| I agree regarding the os.open call in MH.__setitem__. It does not include the O_CREAT flag, so the mode would never be used. |
|
|
| msg70726 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2008-08-05 01:07 |
| I took one test and an idea from Niels' patch -- checking for the existence of os.stat as well as os.umask -- and applied it as rev. 65536. Closing this issue now. Thanks, everyone! |
|
|