From d97dbcea85274baef1ac0572a77a85dc9d9b2b57 Mon Sep 17 00:00:00 2001 From: lipengyu Date: Mon, 24 Aug 2026 19:44:19 +0800 Subject: [PATCH] gh-156312: Prevent mailbox.MH replacement failures from corrupting messages Make `mailbox.MH.__setitem__()` failure-atomic by writing replacements to a temporary file before replacing the original. Failed serialization now leaves the existing message unchanged. --- Lib/mailbox.py | 31 +++++++++-- Lib/test/test_mailbox.py | 52 +++++++++++++++++++ ...-08-24-19-37-56.gh-issue-156312.tF9p9m.rst | 3 ++ 3 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-08-24-19-37-56.gh-issue-156312.tF9p9m.rst diff --git a/Lib/mailbox.py b/Lib/mailbox.py index 99426220154360..bda86c531fb9ba 100644 --- a/Lib/mailbox.py +++ b/Lib/mailbox.py @@ -1069,19 +1069,44 @@ def __setitem__(self, key, message): raise KeyError('No message with key: %s' % key) else: raise + file_closed = False try: if self._locked: _lock_file(f) try: - os.close(os.open(path, os.O_WRONLY | os.O_TRUNC)) - self._dump_message(message, f) + new_file = _create_temporary(path) + try: + self._dump_message(message, new_file) + _sync_close(new_file) + info = os.fstat(f.fileno()) + try: + os.chown(new_file.name, info.st_uid, info.st_gid) + except (AttributeError, OSError): + pass + os.chmod(new_file.name, info.st_mode) + if os.name == 'nt': + # Windows cannot replace an open file. + f.close() + file_closed = True + os.replace(new_file.name, path) + except BaseException: + try: + new_file.close() + except OSError: + pass + try: + os.remove(new_file.name) + except OSError: + pass + raise if isinstance(message, MHMessage): self._dump_sequences(message, key) finally: if self._locked: _unlock_file(f) finally: - _sync_close(f) + if not file_closed: + f.close() def get_message(self, key): """Return a Message representation or raise a KeyError.""" diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py index 019c699bff55c4..340c725a9e03d8 100644 --- a/Lib/test/test_mailbox.py +++ b/Lib/test/test_mailbox.py @@ -1,4 +1,5 @@ import os +import stat import sys import time import socket @@ -1338,6 +1339,57 @@ class TestMH(TestMailbox, unittest.TestCase): def assertMailboxEmpty(self): self.assertEqual(os.listdir(self._path), ['.mh_sequences']) + def test_set_item_nonascii_string_raises_without_modifying_message(self): + key = self._box.add(self._template % 'original') + original = self._box.get_bytes(key) + with self.assertRaisesRegex(ValueError, "ASCII-only"): + self._box[key] = self._nonascii_msg + self.assertEqual(self._box.get_bytes(key), original) + self._box.close() + self._box = self._factory(self._path) + self.assertEqual(self._box.get_bytes(key), original) + + def test_set_item_read_error_does_not_modify_message(self): + class CustomError(Exception): + pass + + class FaultyMessage: + def __init__(self): + self.first_read = True + + def read(self): + raise AssertionError + + def readline(self): + if self.first_read: + self.first_read = False + return b'Subject: replacement\n' + raise CustomError + + key = self._box.add(self._template % 'original') + original = self._box.get_bytes(key) + original_files = set(os.listdir(self._path)) + with self.assertRaises(CustomError): + self._box[key] = FaultyMessage() + self.assertEqual(self._box.get_bytes(key), original) + self.assertEqual(set(os.listdir(self._path)), original_files) + self._box.close() + self._box = self._factory(self._path) + self.assertEqual(self._box.get_bytes(key), original) + + @unittest.skipUnless(hasattr(os, 'chown'), 'requires os.chown') + def test_set_item_preserves_mode(self): + key = self._box.add(self._template % 'original') + path = os.path.join(self._path, str(key)) + mode = os.stat(path).st_mode | stat.S_ISUID + os.chmod(path, mode) + if os.stat(path).st_mode != mode: + self.skipTest('filesystem does not support set-user-ID mode') + + self._box[key] = self._template % 'replacement' + + self.assertEqual(os.stat(path).st_mode, mode) + def test_list_folders(self): # List folders self._box.add_folder('one') diff --git a/Misc/NEWS.d/next/Library/2026-08-24-19-37-56.gh-issue-156312.tF9p9m.rst b/Misc/NEWS.d/next/Library/2026-08-24-19-37-56.gh-issue-156312.tF9p9m.rst new file mode 100644 index 00000000000000..9fcfc2c6a0a8d6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-08-24-19-37-56.gh-issue-156312.tF9p9m.rst @@ -0,0 +1,3 @@ +Prevent :class:`mailbox.MH` message replacement from truncating or partially +overwriting the original message when serializing the replacement raises an +exception.