storage/mutable: raise a specific error upon seeing bad magic, instead of using assert

diff --git a/src/allmydata/storage/common.py b/src/allmydata/storage/common.py
index a442dbf4ae654170ecc1089a8d2d2c5ffba1474a..70f8fed06af926efe7a89f5602a90b1cf811e8b7 100644 (file)
--- a/src/allmydata/storage/common.py
+++ b/src/allmydata/storage/common.py
@@ -4,6 +4,8 @@ from allmydata.util import base32
 
 class DataTooLargeError(Exception):
     pass
+class UnknownMutableContainerVersionError(Exception):
+    pass
 
 
 def si_b2a(storageindex):

diff --git a/src/allmydata/storage/mutable.py b/src/allmydata/storage/mutable.py
index 6520acdee771f1811f084e36b226cd2656c9f5bb..c76195305ab70377dc540288256a0681c779f8bc 100644 (file)
--- a/src/allmydata/storage/mutable.py
+++ b/src/allmydata/storage/mutable.py
@@ -4,7 +4,8 @@ from allmydata.interfaces import BadWriteEnablerError
 from allmydata.util import idlib, log
 from allmydata.util.assertutil import precondition
 from allmydata.storage.lease import LeaseInfo
-from allmydata.storage.common import DataTooLargeError
+from allmydata.storage.common import UnknownMutableContainerVersionError, \
+     DataTooLargeError
 
 # the MutableShareFile is like the ShareFile, but used for mutable data. It
 # has a different layout. See docs/mutable.txt for more details.
@@ -56,7 +57,10 @@ class MutableShareFile:
              write_enabler_nodeid, write_enabler,
              data_length, extra_least_offset) = \
              struct.unpack(">32s20s32sQQ", data)
-            assert magic == self.MAGIC
+            if magic != self.MAGIC:
+                msg = "sharefile %s had magic '%r' but we wanted '%r'" % \
+                      (filename, magic, self.MAGIC)
+                raise UnknownMutableContainerVersionError(msg)
         self.parent = parent # for logging
 
     def log(self, *args, **kwargs):

diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index 82dfa609e36214e80dbc512c6929109ad95eb702..49734ea3cae591d1ac5b37061c963805a00574a0 100644 (file)
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -12,7 +12,8 @@ from allmydata.util import fileutil, hashutil, base32, pollmixin
 from allmydata.storage.server import StorageServer
 from allmydata.storage.mutable import MutableShareFile
 from allmydata.storage.immutable import BucketWriter, BucketReader
-from allmydata.storage.common import DataTooLargeError, storage_index_to_dir
+from allmydata.storage.common import DataTooLargeError, storage_index_to_dir, \
+     UnknownMutableContainerVersionError
 from allmydata.storage.lease import LeaseInfo
 from allmydata.storage.crawler import BucketCountingCrawler
 from allmydata.storage.expirer import LeaseCheckingCrawler
@@ -723,6 +724,20 @@ class MutableServer(unittest.TestCase):
         self.failUnless(isinstance(readv_data, dict))
         self.failUnlessEqual(len(readv_data), 0)
 
+    def test_bad_magic(self):
+        ss = self.create("test_bad_magic")
+        self.allocate(ss, "si1", "we1", self._lease_secret.next(), set([0]), 10)
+        fn = os.path.join(ss.sharedir, storage_index_to_dir("si1"), "0")
+        f = open(fn, "rb+")
+        f.seek(0)
+        f.write("BAD MAGIC")
+        f.close()
+        read = ss.remote_slot_readv
+        e = self.failUnlessRaises(UnknownMutableContainerVersionError,
+                                  read, "si1", [0], [(0,10)])
+        self.failUnless(" had magic " in str(e), e)
+        self.failUnless(" but we wanted " in str(e), e)
+
     def test_container_size(self):
         ss = self.create("test_container_size")
         self.allocate(ss, "si1", "we1", self._lease_secret.next(),