[Zodb-checkins] SVN: ZODB/branches/3.3/ Forward port from ZODB 3.2.

[Tim Peters]

Log message for revision 28466:
  Forward port from ZODB 3.2.
  
  Collector 1581:  fspack can blow up when .fs is corrupted
  
  Repaired three places where fspack referenced an undefined global while
  *trying* to raise CorruptedError.  Added new checkCorruptionInPack()
  test to verify the correct exception gets raised.
  

Changed:
  U   ZODB/branches/3.3/NEWS.txt
  U   ZODB/branches/3.3/src/ZODB/FileStorage/fspack.py
  U   ZODB/branches/3.3/src/ZODB/tests/testFileStorage.py

-=-
Modified: ZODB/branches/3.3/NEWS.txt
===================================================================
--- ZODB/branches/3.3/NEWS.txt	2004-11-16 21:46:32 UTC (rev 28465)
+++ ZODB/branches/3.3/NEWS.txt	2004-11-16 21:51:58 UTC (rev 28466)
@@ -28,6 +28,15 @@
 has been changed to obtain the class path from the object's pickle, without
 trying to import application modules or classes.
 
+FileStorage
+-----------
+
+Collector 1581:  When an attempt to pack a corrupted ``Data.fs`` file was
+made, it was possible for the pack routine to die with a reference to an
+undefined global while it was trying to raise ``CorruptedError``.  It
+raises ``CorruptedError``, as it always intended, in these cases now.
+
+
 Install
 -------
 

Modified: ZODB/branches/3.3/src/ZODB/FileStorage/fspack.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/FileStorage/fspack.py	2004-11-16 21:46:32 UTC (rev 28465)
+++ ZODB/branches/3.3/src/ZODB/FileStorage/fspack.py	2004-11-16 21:51:58 UTC (rev 28466)
@@ -264,7 +264,7 @@
             if tlen != th.tlen:
                 self.fail(pos, "redundant transaction length does not "
                           "match initial transaction length: %d != %d",
-                          u64(s), th.tlen)
+                          tlen, th.tlen)
             pos += 8
 
         self.packpos = pos
@@ -359,7 +359,7 @@
             if tlen != th.tlen:
                 self.fail(pos, "redundant transaction length does not "
                           "match initial transaction length: %d != %d",
-                          u64(s), th.tlen)
+                          tlen, th.tlen)
             pos += 8
 
         for pos in extra_roots:
@@ -553,7 +553,7 @@
             if tlen != th.tlen:
                 self.fail(pos, "redundant transaction length does not "
                           "match initial transaction length: %d != %d",
-                          u64(s), th.tlen)
+                          tlen, th.tlen)
             pos += 8
 
         return pos, new_pos

Modified: ZODB/branches/3.3/src/ZODB/tests/testFileStorage.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/tests/testFileStorage.py	2004-11-16 21:46:32 UTC (rev 28465)
+++ ZODB/branches/3.3/src/ZODB/tests/testFileStorage.py	2004-11-16 21:51:58 UTC (rev 28466)
@@ -167,7 +167,56 @@
 
         self.failUnless(self._storage._records_before_save > 20)
 
+    def checkCorruptionInPack(self):
+        # This sets up a corrupt .fs file, with a redundant transaction
+        # length mismatch.  The implementation of pack in many releases of
+        # ZODB blew up if the .fs file had such damage:  it detected the
+        # damage, but the code to raise CorruptedError referenced an undefined
+        # global.
+        import time
 
+        from ZODB.DB import DB
+        from ZODB.utils import U64, p64
+        from ZODB.FileStorage.format import CorruptedError
+
+        db = DB(self._storage)
+        conn = db.open()
+        conn.root()['xyz'] = 1
+        get_transaction().commit()
+
+        # Ensure it's all on disk.
+        db.close()
+        self._storage.close()
+
+        # Reopen before damaging.
+        self.open()
+
+        # Open .fs directly, and damage content.
+        f = open('FileStorageTests.fs', 'r+b')
+        f.seek(0, 2)
+        pos2 = f.tell() - 8
+        f.seek(pos2)
+        tlen2 = U64(f.read(8))  # length-8 of the last transaction
+        pos1 = pos2 - tlen2 + 8 # skip over the tid at the start
+        f.seek(pos1)
+        tlen1 = U64(f.read(8))  # should be redundant length-8
+        self.assertEqual(tlen1, tlen2)  # verify that it is redundant
+
+        # Now damage the second copy.
+        f.seek(pos2)
+        f.write(p64(tlen2 - 1))
+        f.close()
+
+        # Try to pack.  This used to yield
+        #     NameError: global name 's' is not defined
+        try:
+            self._storage.pack(time.time(), None)
+        except CorruptedError, detail:
+            self.assert_("redundant transaction length does not match "
+                         "initial transaction length" in str(detail))
+        else:
+            self.fail("expected CorruptedError")
+
 class FileStorageRecoveryTest(
     StorageTestBase.StorageTestBase,
     RecoveryStorage.RecoveryStorage,