From d8ba8c2eb5d4b1b5de448bc9cebe2c81cde4247d Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Tue, 14 Jul 2009 23:45:10 -0700
Subject: [PATCH] Allow tests to pass with -OO by turning some AssertionErrors
 (the ones that we actually exercise during tests) into more specific
 exceptions, so they don't get optimized away. The best rule to follow is
 probably this: if an exception is worth testing, then it's part of the API,
 and AssertionError should never be part of the API. Closes #749.

---
 src/allmydata/client.py            |  8 +++++---
 src/allmydata/interfaces.py        |  4 ++++
 src/allmydata/mutable/common.py    |  3 ++-
 src/allmydata/mutable/layout.py    | 13 ++++++++----
 src/allmydata/test/test_mutable.py |  8 ++++----
 src/allmydata/test/test_uri.py     |  6 +++---
 src/allmydata/test/test_web.py     |  8 +++++---
 src/allmydata/uri.py               | 32 ++++++++++++++++++++----------
 src/allmydata/web/root.py          |  6 +++---
 9 files changed, 56 insertions(+), 32 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index df35c89a..35f55c09 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -27,7 +27,8 @@ from allmydata.unknown import UnknownNode
 from allmydata.stats import StatsProvider
 from allmydata.history import History
 from allmydata.interfaces import IURI, INewDirectoryURI, IStatsProducer, \
-     IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, RIStubClient
+     IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, RIStubClient, \
+     UnhandledCapTypeError
 
 KiB=1024
 MiB=1024*KiB
@@ -429,9 +430,10 @@ class Client(node.Node, pollmixin.PollMixin):
                     node = LiteralFileNode(u, self) # LIT
                 else:
                     node = FileNode(u, self, self.download_cache_dirman) # CHK
-            else:
-                assert IMutableFileURI.providedBy(u), u
+            elif IMutableFileURI.providedBy(u):
                 node = MutableFileNode(self).init_from_uri(u)
+            else:
+                raise UnhandledCapTypeError("cap is recognized, but has no Node")
             self._node_cache[u_s] = node  # note: WeakValueDictionary
         return self._node_cache[u_s]
 
diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 42188e7b..6a9672e9 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -482,6 +482,10 @@ class CannotPackUnknownNodeError(Exception):
     """UnknownNodes (using filecaps from the future that we don't understand)
     cannot yet be copied safely, so I refuse to copy them."""
 
+class UnhandledCapTypeError(Exception):
+    """I recognize the cap/URI, but I cannot create an IFilesystemNode for
+    it."""
+
 class IFilesystemNode(Interface):
     def get_uri():
         """
diff --git a/src/allmydata/mutable/common.py b/src/allmydata/mutable/common.py
index 869b27d3..49a04ff2 100644
--- a/src/allmydata/mutable/common.py
+++ b/src/allmydata/mutable/common.py
@@ -53,7 +53,8 @@ class CorruptShareError(Exception):
                                                                self.shnum,
                                                                self.reason)
 
-
+class UnknownVersionError(Exception):
+    """The share we received was of a version we don't recognize."""
 
 class ResponseCache:
     """I cache share data, to reduce the number of round trips used during
diff --git a/src/allmydata/mutable/layout.py b/src/allmydata/mutable/layout.py
index a110eb1d..ee89128c 100644
--- a/src/allmydata/mutable/layout.py
+++ b/src/allmydata/mutable/layout.py
@@ -1,6 +1,6 @@
 
 import struct
-from common import NeedMoreDataError
+from common import NeedMoreDataError, UnknownVersionError
 
 PREFIX = ">BQ32s16s" # each version has a different prefix
 SIGNED_PREFIX = ">BQ32s16s BBQQ" # this is covered by the signature
@@ -34,7 +34,9 @@ def unpack_prefix_and_signature(data):
      k, N, segsize, datalen,
      o) = unpack_header(data)
 
-    assert version == 0
+    if version != 0:
+        raise UnknownVersionError("got mutable share version %d, but I only understand version 0" % version)
+
     if len(data) < o['share_hash_chain']:
         raise NeedMoreDataError(o['share_hash_chain'],
                                 o['enc_privkey'], o['EOF']-o['enc_privkey'])
@@ -60,7 +62,9 @@ def unpack_share(data):
      o['enc_privkey'],
      o['EOF']) = struct.unpack(HEADER, data[:HEADER_LENGTH])
 
-    assert version == 0
+    if version != 0:
+        raise UnknownVersionError("got mutable share version %d, but I only understand version 0" % version)
+
     if len(data) < o['EOF']:
         raise NeedMoreDataError(o['EOF'],
                                 o['enc_privkey'], o['EOF']-o['enc_privkey'])
@@ -132,7 +136,8 @@ def pack_checkstring(seqnum, root_hash, IV):
 def unpack_checkstring(checkstring):
     cs_len = struct.calcsize(PREFIX)
     version, seqnum, root_hash, IV = struct.unpack(PREFIX, checkstring[:cs_len])
-    assert version == 0 # TODO: just ignore the share
+    if version != 0: # TODO: just ignore the share
+        raise UnknownVersionError("got mutable share version %d, but I only understand version 0" % version)
     return (seqnum, root_hash, IV)
 
 def pack_prefix(seqnum, root_hash, IV,
diff --git a/src/allmydata/test/test_mutable.py b/src/allmydata/test/test_mutable.py
index a5fc9b89..212cd5da 100644
--- a/src/allmydata/test/test_mutable.py
+++ b/src/allmydata/test/test_mutable.py
@@ -1030,7 +1030,7 @@ class Roundtrip(unittest.TestCase, testutil.ShouldFailMixin, PublishMixin):
                 # should be noted in the servermap's list of problems.
                 if substring:
                     allproblems = [str(f) for f in servermap.problems]
-                    self.failUnless(substring in "".join(allproblems))
+                    self.failUnlessIn(substring, "".join(allproblems))
                 return servermap
             if should_succeed:
                 d1 = self._fn.download_version(servermap, ver)
@@ -1049,9 +1049,9 @@ class Roundtrip(unittest.TestCase, testutil.ShouldFailMixin, PublishMixin):
         return d
 
     def test_corrupt_all_verbyte(self):
-        # when the version byte is not 0, we hit an assertion error in
-        # unpack_share().
-        d = self._test_corrupt_all(0, "AssertionError")
+        # when the version byte is not 0, we hit an UnknownVersionError error
+        # in unpack_share().
+        d = self._test_corrupt_all(0, "UnknownVersionError")
         def _check_servermap(servermap):
             # and the dump should mention the problems
             s = StringIO()
diff --git a/src/allmydata/test/test_uri.py b/src/allmydata/test/test_uri.py
index 7ac63469..0898530d 100644
--- a/src/allmydata/test/test_uri.py
+++ b/src/allmydata/test/test_uri.py
@@ -186,10 +186,10 @@ class Constraint(unittest.TestCase):
     def test_constraint(self):
        good="http://127.0.0.1:3456/uri/URI%3ADIR2%3Agh3l5rbvnv2333mrfvalmjfr4i%3Alz6l7u3z3b7g37s4zkdmfpx5ly4ib4m6thrpbusi6ys62qtc6mma/"
        uri.NewDirectoryURI.init_from_human_encoding(good)
-       self.failUnlessRaises(AssertionError, uri.NewDirectoryURI.init_from_string, good)
+       self.failUnlessRaises(uri.BadURIError, uri.NewDirectoryURI.init_from_string, good)
        bad = good + '==='
-       self.failUnlessRaises(AssertionError, uri.NewDirectoryURI.init_from_human_encoding, bad)
-       self.failUnlessRaises(AssertionError, uri.NewDirectoryURI.init_from_string, bad)
+       self.failUnlessRaises(uri.BadURIError, uri.NewDirectoryURI.init_from_human_encoding, bad)
+       self.failUnlessRaises(uri.BadURIError, uri.NewDirectoryURI.init_from_string, bad)
        fileURI = 'URI:CHK:gh3l5rbvnv2333mrfvalmjfr4i:lz6l7u3z3b7g37s4zkdmfpx5ly4ib4m6thrpbusi6ys62qtc6mma:3:10:345834'
        uri.CHKFileURI.init_from_string(fileURI)
 
diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py
index 28905ef4..f7d45da0 100644
--- a/src/allmydata/test/test_web.py
+++ b/src/allmydata/test/test_web.py
@@ -19,7 +19,8 @@ from allmydata.util.assertutil import precondition
 from allmydata.test.common import FakeDirectoryNode, FakeCHKFileNode, \
      FakeMutableFileNode, create_chk_filenode, WebErrorMixin, ShouldFailMixin
 from allmydata.interfaces import IURI, INewDirectoryURI, \
-     IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, IMutableFileNode
+     IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, IMutableFileNode, \
+     UnhandledCapTypeError
 from allmydata.mutable import servermap, publish, retrieve
 import common_util as testutil
 from allmydata.test.no_network import GridTestMixin
@@ -74,8 +75,9 @@ class FakeClient(service.MultiService):
             return FakeDirectoryNode(self).init_from_uri(u)
         if IFileURI.providedBy(u):
             return FakeCHKFileNode(u, self)
-        assert IMutableFileURI.providedBy(u), u
-        return FakeMutableFileNode(self).init_from_uri(u)
+        if IMutableFileURI.providedBy(u):
+            return FakeMutableFileNode(self).init_from_uri(u)
+        raise UnhandledCapTypeError("cap '%s' is recognized, but has no Node" % auri)
 
     def create_empty_dirnode(self):
         n = FakeDirectoryNode(self)
diff --git a/src/allmydata/uri.py b/src/allmydata/uri.py
index 9be92a41..b37ea8ca 100644
--- a/src/allmydata/uri.py
+++ b/src/allmydata/uri.py
@@ -7,6 +7,9 @@ from allmydata.util import base32, hashutil
 from allmydata.interfaces import IURI, IDirnodeURI, IFileURI, IImmutableFileURI, \
     IVerifierURI, IMutableFileURI, INewDirectoryURI, IReadonlyNewDirectoryURI
 
+class BadURIError(Exception):
+    pass
+
 # the URI shall be an ascii representation of the file. It shall contain
 # enough information to retrieve and validate the contents. It shall be
 # expressed in a limited character set (namely [TODO]).
@@ -60,21 +63,22 @@ class CHKFileURI(_BaseURI):
         self.total_shares = total_shares
         self.size = size
         self.storage_index = hashutil.storage_index_hash(self.key)
-        assert len(self.storage_index) == 16
-        self.storage_index = hashutil.storage_index_hash(key)
-        assert len(self.storage_index) == 16 # sha256 hash truncated to 128
+        if not len(self.storage_index) == 16: # sha256 hash truncated to 128
+            raise BadURIError("storage index must be 16 bytes long")
 
     @classmethod
     def init_from_human_encoding(cls, uri):
         mo = cls.HUMAN_RE.search(uri)
-        assert mo, uri
+        if not mo:
+            raise BadURIError("%s doesn't look like a cap" % (uri,))
         return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)),
                    int(mo.group(3)), int(mo.group(4)), int(mo.group(5)))
 
     @classmethod
     def init_from_string(cls, uri):
         mo = cls.STRING_RE.search(uri)
-        assert mo, uri
+        if not mo:
+            raise BadURIError("%s doesn't look like a cap" % (uri,))
         return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)),
                    int(mo.group(3)), int(mo.group(4)), int(mo.group(5)))
 
@@ -212,13 +216,15 @@ class WriteableSSKFileURI(_BaseURI):
     @classmethod
     def init_from_human_encoding(cls, uri):
         mo = cls.HUMAN_RE.search(uri)
-        assert mo, uri
+        if not mo:
+            raise BadURIError("'%s' doesn't look like a cap" % (uri,))
         return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)))
 
     @classmethod
     def init_from_string(cls, uri):
         mo = cls.STRING_RE.search(uri)
-        assert mo, (uri, cls)
+        if not mo:
+            raise BadURIError("'%s' doesn't look like a %s cap" % (uri, cls))
         return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)))
 
     def to_string(self):
@@ -260,13 +266,15 @@ class ReadonlySSKFileURI(_BaseURI):
     @classmethod
     def init_from_human_encoding(cls, uri):
         mo = cls.HUMAN_RE.search(uri)
-        assert mo, uri
+        if not mo:
+            raise BadURIError("'%s' doesn't look like a cap" % (uri,))
         return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)))
 
     @classmethod
     def init_from_string(cls, uri):
         mo = cls.STRING_RE.search(uri)
-        assert mo, uri
+        if not mo:
+            raise BadURIError("'%s' doesn't look like a cap" % (uri,))
         return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)))
 
     def to_string(self):
@@ -333,7 +341,8 @@ class _NewDirectoryBaseURI(_BaseURI):
     @classmethod
     def init_from_string(cls, uri):
         mo = cls.BASE_STRING_RE.search(uri)
-        assert mo, (uri, cls)
+        if not mo:
+            raise BadURIError("'%s' doesn't look like a %s cap" % (uri, cls))
         bits = uri[mo.end():]
         fn = cls.INNER_URI_CLASS.init_from_string(
             cls.INNER_URI_CLASS.BASE_STRING+bits)
@@ -342,7 +351,8 @@ class _NewDirectoryBaseURI(_BaseURI):
     @classmethod
     def init_from_human_encoding(cls, uri):
         mo = cls.BASE_HUMAN_RE.search(uri)
-        assert mo, (uri, cls)
+        if not mo:
+            raise BadURIError("'%s' doesn't look like a %s cap" % (uri, cls))
         bits = uri[mo.end():]
         while bits and bits[-1] == '/':
             bits = bits[:-1]
diff --git a/src/allmydata/web/root.py b/src/allmydata/web/root.py
index 6fa84b6a..07f55704 100644
--- a/src/allmydata/web/root.py
+++ b/src/allmydata/web/root.py
@@ -12,7 +12,7 @@ import allmydata # to display import path
 from allmydata import get_package_versions_string
 from allmydata import provisioning
 from allmydata.util import idlib, log
-from allmydata.interfaces import IFileNode
+from allmydata.interfaces import IFileNode, UnhandledCapTypeError
 from allmydata.web import filenode, directory, unlinked, status, operations
 from allmydata.web import reliability, storage
 from allmydata.web.common import abbreviate_size, getxmlfile, WebError, \
@@ -79,7 +79,7 @@ class URIHandler(RenderMixin, rend.Page):
         try:
             node = self.client.create_node_from_uri(name)
             return directory.make_handler_for(node, self.client)
-        except (TypeError, AssertionError):
+        except (TypeError, UnhandledCapTypeError, AssertionError):
             raise WebError("'%s' is not a valid file- or directory- cap"
                            % name)
 
@@ -98,7 +98,7 @@ class FileHandler(rend.Page):
         # 'name' must be a file URI
         try:
             node = self.client.create_node_from_uri(name)
-        except (TypeError, AssertionError):
+        except (TypeError, UnhandledCapTypeError, AssertionError):
             raise WebError("'%s' is not a valid file- or directory- cap"
                            % name)
         if not IFileNode.providedBy(node):
-- 
2.45.2