[Apply length-checking.diff from bug #1599254]

Add length checking to single-file mailbox formats: before doing a
flush() on a mailbox, seek to the end and verify its length is
unchanged, raising ExternalClashError if the file's length has
changed.

This fix avoids potential data loss if some other process appends to
the mailbox file after the table of contents has been generated;
instead of overwriting the modified file, you'll get the exception.

I also noticed that the self._lookup() call in self.flush() wasn't
necessary (everything that sets self._pending to True also calls
self.lookup()), and replaced it by an assertion.

2.5 backport candidate.

1 files changed, 23 insertions, 2 deletions

diff --git a/Lib/mailbox.py b/Lib/mailbox.py
index f9af494..7bdd347 100755
--- a/Lib/mailbox.py
+++ b/Lib/mailbox.py
@@ -513,6 +513,7 @@ class _singlefileMailbox(Mailbox):
         self._next_key = 0
         self._pending = False   # No changes require rewriting the file.
         self._locked = False
+        self._file_length = None        # Used to record mailbox size
 
     def add(self, message):
         """Add message and return assigned key."""
@@ -566,7 +567,21 @@ class _singlefileMailbox(Mailbox):
         """Write any pending changes to disk."""
         if not self._pending:
             return
-        self._lookup()
+
+        # In order to be writing anything out at all, self._toc must
+        # already have been generated (and presumably has been modified
+        # by adding or deleting an item).
+        assert self._toc is not None
+        
+        # Check length of self._file; if it's changed, some other process
+        # has modified the mailbox since we scanned it.
+        self._file.seek(0, 2)
+        cur_len = self._file.tell()
+        if cur_len != self._file_length:
+            raise ExternalClashError('Size of mailbox file changed '
+                                     '(expected %i, found %i)' %
+                                     (self._file_length, cur_len))
+        
         new_file = _create_temporary(self._path)
         try:
             new_toc = {}
@@ -642,6 +657,7 @@ class _singlefileMailbox(Mailbox):
         offsets = self._install_message(message)
         self._post_message_hook(self._file)
         self._file.flush()
+        self._file_length = self._file.tell()  # Record current length of mailbox
         return offsets
 
 
@@ -733,6 +749,7 @@ class mbox(_mboxMMDF):
                 break
         self._toc = dict(enumerate(zip(starts, stops)))
         self._next_key = len(self._toc)
+        self._file_length = self._file.tell()
 
 
 class MMDF(_mboxMMDF):
@@ -776,6 +793,8 @@ class MMDF(_mboxMMDF):
                 break
         self._toc = dict(enumerate(zip(starts, stops)))
         self._next_key = len(self._toc)
+        self._file.seek(0, 2)
+        self._file_length = self._file.tell()
 
 
 class MH(Mailbox):
@@ -1201,7 +1220,9 @@ class Babyl(_singlefileMailbox):
         self._toc = dict(enumerate(zip(starts, stops)))
         self._labels = dict(enumerate(label_lists))
         self._next_key = len(self._toc)
-
+        self._file.seek(0, 2)
+        self._file_length = self._file.tell()
+        
     def _pre_mailbox_hook(self, f):
         """Called before writing the mailbox to file f."""
         f.write('BABYL OPTIONS:%sVersion: 5%sLabels:%s%s\037' %