From a20ebb883a83ce18db2a9fbac7d3c9ae1b2fdb39 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Wed, 9 Apr 2014 01:35:52 +0100 Subject: [PATCH] Add CorruptStoredShareError as a superclass for all corrupt share errors raised by a storage server (rebased). refs #1566 Signed-off-by: Daira Hopwood --- .../storage/backends/cloud/immutable.py | 16 ++++++++++------ src/allmydata/storage/backends/cloud/mutable.py | 14 +++++++++----- .../storage/backends/disk/disk_backend.py | 9 +++------ src/allmydata/storage/backends/disk/immutable.py | 10 ++++++---- src/allmydata/storage/backends/disk/mutable.py | 6 ++++-- src/allmydata/storage/common.py | 8 +++++++- 6 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/allmydata/storage/backends/cloud/immutable.py b/src/allmydata/storage/backends/cloud/immutable.py index cc292982..45e60bd1 100644 --- a/src/allmydata/storage/backends/cloud/immutable.py +++ b/src/allmydata/storage/backends/cloud/immutable.py @@ -10,7 +10,8 @@ from allmydata.interfaces import IShareForReading, IShareForWriting from allmydata.util.assertutil import precondition, _assert from allmydata.util.mathutil import div_ceil -from allmydata.storage.common import UnknownImmutableContainerVersionError, DataTooLargeError +from allmydata.storage.common import CorruptStoredShareError, UnknownImmutableContainerVersionError, \ + DataTooLargeError from allmydata.storage.backends.cloud import cloud_common from allmydata.storage.backends.cloud.cloud_common import get_chunk_key, \ BackpressurePipeline, ChunkCache, CloudShareBase, CloudShareReaderMixin @@ -157,7 +158,7 @@ class ImmutableCloudShareForReading(CloudShareBase, ImmutableCloudShareMixin, Cl chunksize = len(first_chunkdata) if chunksize < self.HEADER_SIZE: msg = "%r had incomplete header (%d bytes)" % (self, chunksize) - raise UnknownImmutableContainerVersionError(msg) + raise UnknownImmutableContainerVersionError(shnum, msg) self._total_size = total_size self._chunksize = chunksize @@ -167,19 +168,22 @@ class ImmutableCloudShareForReading(CloudShareBase, ImmutableCloudShareMixin, Cl #print "ImmutableCloudShareForReading", total_size, chunksize, self._key header = first_chunkdata[:self.HEADER_SIZE] - (version, unused, num_leases) = struct.unpack(self.HEADER, header) + try: + (version, unused, num_leases) = struct.unpack(self.HEADER, header) + except struct.error, e: + raise CorruptStoredShareError(shnum, "invalid immutable share header for shnum %d: %s" % (shnum, e)) if version != 1: msg = "%r had version %d but we wanted 1" % (self, version) - raise UnknownImmutableContainerVersionError(msg) + raise UnknownImmutableContainerVersionError(shnum, msg) # We cannot write leases in share files, but allow them to be present # in case a share file is copied from a disk backend, or in case we # need them in future. self._data_length = total_size - self.DATA_OFFSET - (num_leases * self.LEASE_SIZE) - # TODO: raise a better exception. - _assert(self._data_length >= 0, data_length=self._data_length) + if self._data_length < 0: + raise CorruptStoredShareError("calculated data length for shnum %d is %d" % (shnum, self._data_length)) # Boilerplate is in CloudShareBase, read implementation is in CloudShareReaderMixin. # So nothing to implement here. Yay! diff --git a/src/allmydata/storage/backends/cloud/mutable.py b/src/allmydata/storage/backends/cloud/mutable.py index f8ad0969..7ee1af62 100644 --- a/src/allmydata/storage/backends/cloud/mutable.py +++ b/src/allmydata/storage/backends/cloud/mutable.py @@ -12,7 +12,8 @@ from allmydata.util import idlib, log from allmydata.util.assertutil import precondition, _assert from allmydata.util.mathutil import div_ceil from allmydata.util.hashutil import timing_safe_compare -from allmydata.storage.common import UnknownMutableContainerVersionError, DataTooLargeError +from allmydata.storage.common import CorruptStoredShareError, UnknownMutableContainerVersionError, \ + DataTooLargeError from allmydata.storage.backends.base import testv_compare from allmydata.mutable.layout import MUTABLE_MAGIC, MAX_MUTABLE_SHARE_SIZE from allmydata.storage.backends.cloud import cloud_common @@ -73,15 +74,18 @@ class MutableCloudShare(CloudShareBase, CloudShareReaderMixin): if len(first_chunkdata) < self.HEADER_SIZE: msg = "%r had incomplete header (%d bytes)" % (self, len(first_chunkdata)) - raise UnknownMutableContainerVersionError(msg) + raise UnknownMutableContainerVersionError(shnum, msg) header = first_chunkdata[:self.HEADER_SIZE] - (magic, write_enabler_nodeid, real_write_enabler, - data_length, extra_lease_offset) = struct.unpack(self.HEADER, header) + try: + (magic, write_enabler_nodeid, real_write_enabler, + data_length, extra_lease_offset) = struct.unpack(self.HEADER, header) + except struct.error, e: + raise CorruptStoredShareError(shnum, "invalid mutable share header for shnum %d: %s" % (shnum, e)) if magic != self.MAGIC: msg = "%r had magic %r but we wanted %r" % (self, magic, self.MAGIC) - raise UnknownMutableContainerVersionError(msg) + raise UnknownMutableContainerVersionError(shnum, msg) self._write_enabler_nodeid = write_enabler_nodeid self._real_write_enabler = real_write_enabler diff --git a/src/allmydata/storage/backends/disk/disk_backend.py b/src/allmydata/storage/backends/disk/disk_backend.py index e3d09a53..5255cdfe 100644 --- a/src/allmydata/storage/backends/disk/disk_backend.py +++ b/src/allmydata/storage/backends/disk/disk_backend.py @@ -1,13 +1,12 @@ -import struct, os.path +import os.path from twisted.internet import defer from zope.interface import implements from allmydata.interfaces import IStorageBackend, IShareSet from allmydata.util import fileutil, log -from allmydata.storage.common import si_b2a, si_a2b, NUM_RE, \ - UnknownMutableContainerVersionError, UnknownImmutableContainerVersionError +from allmydata.storage.common import si_b2a, si_a2b, NUM_RE, CorruptStoredShareError from allmydata.storage.bucket import BucketWriter from allmydata.storage.backends.base import Backend, ShareSet from allmydata.storage.backends.disk.immutable import load_immutable_disk_share, create_immutable_disk_share @@ -141,9 +140,7 @@ class DiskShareSet(ShareSet): sharefile = os.path.join(self._sharehomedir, shnumstr) try: shares[shnum] = get_disk_share(sharefile, si, shnum) - except (UnknownMutableContainerVersionError, - UnknownImmutableContainerVersionError, - struct.error): + except CorruptStoredShareError: corrupted.add(shnum) valid = [shares[shnum] for shnum in sorted(shares.keys())] diff --git a/src/allmydata/storage/backends/disk/immutable.py b/src/allmydata/storage/backends/disk/immutable.py index b7bba0db..056863c5 100644 --- a/src/allmydata/storage/backends/disk/immutable.py +++ b/src/allmydata/storage/backends/disk/immutable.py @@ -8,7 +8,7 @@ from allmydata.interfaces import IShareForReading, IShareForWriting from allmydata.util import fileutil from allmydata.util.assertutil import precondition, _assert -from allmydata.storage.common import si_b2a, UnknownImmutableContainerVersionError, \ +from allmydata.storage.common import si_b2a, CorruptStoredShareError, UnknownImmutableContainerVersionError, \ DataTooLargeError @@ -79,17 +79,19 @@ class ImmutableDiskShare(object): f = open(self._home, 'rb') try: (version, unused, num_leases) = struct.unpack(self.HEADER, f.read(self.HEADER_SIZE)) + except struct.error, e: + raise CorruptStoredShareError(shnum, "invalid immutable share header for shnum %d: %s" % (shnum, e)) finally: f.close() if version != 1: msg = "sharefile %r had version %d but we wanted 1" % (self._home, version) - raise UnknownImmutableContainerVersionError(msg) + raise UnknownImmutableContainerVersionError(shnum, msg) filesize = os.stat(self._home).st_size self._data_length = filesize - self.DATA_OFFSET - (num_leases * self.LEASE_SIZE) - # TODO: raise a better exception. - _assert(self._data_length >= 0, data_length=self._data_length) + if self._data_length < 0: + raise CorruptStoredShareError("calculated data length for shnum %d is %d" % (shnum, self._data_length)) def __repr__(self): return ("" diff --git a/src/allmydata/storage/backends/disk/mutable.py b/src/allmydata/storage/backends/disk/mutable.py index 8eaadd25..253df514 100644 --- a/src/allmydata/storage/backends/disk/mutable.py +++ b/src/allmydata/storage/backends/disk/mutable.py @@ -9,7 +9,7 @@ from allmydata.interfaces import IMutableShare, BadWriteEnablerError from allmydata.util import fileutil, idlib, log from allmydata.util.assertutil import precondition, _assert from allmydata.util.hashutil import timing_safe_compare -from allmydata.storage.common import si_b2a, UnknownMutableContainerVersionError, \ +from allmydata.storage.common import si_b2a, CorruptStoredShareError, UnknownMutableContainerVersionError, \ DataTooLargeError from allmydata.storage.backends.base import testv_compare from allmydata.mutable.layout import MUTABLE_MAGIC, MAX_MUTABLE_SHARE_SIZE @@ -72,7 +72,9 @@ class MutableDiskShare(object): if magic != self.MAGIC: msg = "sharefile %r had magic '%r' but we wanted '%r'" % \ (self._home, magic, self.MAGIC) - raise UnknownMutableContainerVersionError(msg) + raise UnknownMutableContainerVersionError(shnum, msg) + except struct.error, e: + raise CorruptStoredShareError(shnum, "invalid mutable share header for shnum %d: %s" % (shnum, e)) finally: f.close() self.parent = parent # for logging diff --git a/src/allmydata/storage/common.py b/src/allmydata/storage/common.py index d2170426..5417da30 100644 --- a/src/allmydata/storage/common.py +++ b/src/allmydata/storage/common.py @@ -13,7 +13,13 @@ PREFIX = re.compile("^[%s]{2}$" % (base32.z_base_32_alphabet,)) class DataTooLargeError(Exception): pass -class UnknownContainerVersionError(Exception): + +class CorruptStoredShareError(Exception): + def __init__(self, shnum, *rest): + Exception.__init__(self, shnum, *rest) + self.shnum = shnum + +class UnknownContainerVersionError(CorruptStoredShareError): pass class UnknownMutableContainerVersionError(UnknownContainerVersionError): -- 2.45.2