Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.12] gh-117467: Add preserving of mailbox owner on flush (GH-117510) #117537

Merged
merged 1 commit into from Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions Lib/mailbox.py
Expand Up @@ -698,9 +698,13 @@ def flush(self):
_sync_close(new_file)
# self._file is about to get replaced, so no need to sync.
self._file.close()
# Make sure the new file's mode is the same as the old file's
mode = os.stat(self._path).st_mode
os.chmod(new_file.name, mode)
# Make sure the new file's mode and owner are the same as the old file's
info = os.stat(self._path)
os.chmod(new_file.name, info.st_mode)
try:
os.chown(new_file.name, info.st_uid, info.st_gid)
except (AttributeError, OSError):
pass
try:
os.rename(new_file.name, self._path)
except FileExistsError:
Expand Down
42 changes: 42 additions & 0 deletions Lib/test/test_mailbox.py
Expand Up @@ -9,6 +9,7 @@
import io
import tempfile
from test import support
from test.support import import_helper
from test.support import os_helper
from test.support import socket_helper
import unittest
Expand Down Expand Up @@ -981,6 +982,47 @@ def test_permissions_after_flush(self):

self.assertEqual(os.stat(self._path).st_mode, mode)

@unittest.skipUnless(hasattr(os, 'chown'), 'requires os.chown')
def test_ownership_after_flush(self):
# See issue gh-117467

pwd = import_helper.import_module('pwd')
grp = import_helper.import_module('grp')
st = os.stat(self._path)

for e in pwd.getpwall():
if e.pw_uid != st.st_uid:
other_uid = e.pw_uid
break
else:
self.skipTest("test needs more than one user")

for e in grp.getgrall():
if e.gr_gid != st.st_gid:
other_gid = e.gr_gid
break
else:
self.skipTest("test needs more than one group")

try:
os.chown(self._path, other_uid, other_gid)
except OSError:
self.skipTest('test needs root privilege')
# Change permissions as in test_permissions_after_flush.
mode = st.st_mode | 0o666
os.chmod(self._path, mode)

self._box.add(self._template % 0)
i = self._box.add(self._template % 1)
# Need to remove one message to make flush() create a new file
self._box.remove(i)
self._box.flush()

st = os.stat(self._path)
self.assertEqual(st.st_uid, other_uid)
self.assertEqual(st.st_gid, other_gid)
self.assertEqual(st.st_mode, mode)


class _TestMboxMMDF(_TestSingleFile):

Expand Down
@@ -0,0 +1,2 @@
Preserve mailbox ownership when rewriting in :func:`mailbox.mbox.flush`.
Patch by Tony Mountifield.