Stop using assert() inappropriately. Fixes #1981

This patch also enables the Python optimizer (-OO) and hash randomization
(-R) within the setup-test-sh script, just so those alternate code-paths
get used and tested. Added .pyo files to .gitignore and `make clean`.

.gitignore

@@ -1,4 +1,5 @@
 *.pyc
+*.pyo
 *~
 *.po
 *.debhelper

Makefile

@@ -107,6 +107,7 @@ pytests:
 
 clean:
 	@rm -f `find . -name \\*.pyc` \
+	       `find . -name \\*.pyo` \
 	       `find . -name \\*.mo` \
 	        mailpile-tmp.py mailpile.py \
 	        ChangeLog AUTHORS \

mailpile/auth.py

@@ -40,8 +40,9 @@ def VerifyAndStorePassphrase(config, passphrase=None, sps=None,
         sps = SecurePassphraseStorage(passphrase)
         passphrase = 'this probably does not really overwrite :-( '
 
-    assert(sps is not None)
-    assert(config.load_master_key(sps))
+    safe_assert(
+        (sps is not None) and
+        (config.load_master_key(sps)))
 
     # Fun side effect: changing the passphrase invalidates the message cache
     import mailpile.mailutils
@@ -141,7 +142,7 @@ def _do_redirect(self):
         # arbitrary URLs on the Internet.
         if path:
             url = urlparse(path)
-            assert(not url.scheme and not url.netloc)
+            safe_assert(not url.scheme and not url.netloc)
 
         if (path and
                not path[1:].startswith(DeAuthenticate.SYNOPSIS[2] or '!') and
@@ -443,7 +444,7 @@ def command(self):
         if self.data.get('_method', None) == 'GET':
             return self._success(pass_prompt, result)
 
-        assert(fingerprint is not None)
+        safe_assert(fingerprint is not None)
         fingerprint = fingerprint.lower()
         if fingerprint in config.secrets:
             if config.secrets[fingerprint].policy == 'protect':
@@ -461,7 +462,7 @@ def command(self):
             update_ms = update_mr = (account is not None)
 
         if update_ms or update_mr:
-            assert(account is not None)
+            safe_assert(account is not None)
 
         if not pass_check(password, account=account, fingerprint=fingerprint):
             result['error'] = _('Password incorrect! Try again?')

mailpile/config/base.py

@@ -265,7 +265,7 @@ def reset(self, rules=True, data=True):
             raise Exception(_('Please override this method'))
 
         def set_rules(self, rules):
-            assert(isinstance(rules, dict))
+            safe_assert(isinstance(rules, dict))
             self.reset()
             for key, rule in rules.iteritems():
                 self.add_rule(key, rule)

mailpile/crypto/records.py

@@ -191,7 +191,8 @@ def __init__(self, fn, key,
 
         if not self._parse_header():
             self._write_header()
-        assert(max_bytes <= self._max_bytes)
+        if max_bytes > self._max_bytes:
+            raise AssertionError('max_bytes mismatch')
 
     def _calculate_constants(self):
         # Calculate our constants! Magic numbers:

mailpile/crypto/state.py

@@ -48,16 +48,17 @@ def __init__(self, parent=None, copy=None, bubbly=True):
     def _set_status(self, value):
         if value not in self.STATUSES:
             print 'Bogus status for %s: %s' % (type(self), value)
-        assert(value in self.STATUSES)
+            raise ValueError('Invalid status: %s' % value)
         self._status = value
         self.mix_bubbles()
 
     def __setitem__(self, item, value):
-        assert(item in self.KEYS)
+        if item not in self.KEYS:
+            raise KeyError('Invalid key: %s' % item)
         if item == "status":
             if value not in self.STATUSES:
                 print 'Bogus status for %s: %s' % (type(self), value)
-            assert(value in self.STATUSES)
+                raise ValueError('Invalid value for %s: %s' % (key, value))
             if self._status is None:  # Capture initial value
                 self._status = value
         dict.__setitem__(self, item, value)

mailpile/crypto/streamer.py

@@ -44,7 +44,7 @@
 from mailpile.i18n import ngettext as _n
 from mailpile.crypto.gpgi import GPG_BINARY
 from mailpile.safe_popen import Popen, PIPE
-from mailpile.util import CryptoLock, safe_remove
+from mailpile.util import CryptoLock, safe_remove, safe_assert
 from mailpile.util import sha512b64 as genkey
 
 from mailpile.crypto.aes_utils import getrandbits
@@ -952,7 +952,7 @@ def _mk_command(self):
         elif self.decryptor is not None:
             return None
         elif self.state == self.STATE_PGP_DATA:
-            assert(self.gpgi is not None)
+            safe_assert(self.gpgi is not None)
             if self.gpg_pass:
                 return self.gpgi.common_args(will_send_passphrase=True)
             else:

mailpile/httpd.py

@@ -83,10 +83,12 @@ class HttpRequestHandler(SimpleXMLRPCRequestHandler):
     _HTML_RE = re.compile('[<>\'\"]+')
 
     def assert_no_newline(self, data):
-        assert(re.search(self._NEWLINE_RE, str(data) or '') is None)
+        if re.search(self._NEWLINE_RE, str(data) or '') is not None:
+            raise ValueError()
 
     def assert_no_html(self, data):
-        assert(re.search(self._HTML_RE, data or '') is None)
+        if re.search(self._HTML_RE, data or '') is not None:
+            raise ValueError()
 
     def send_header(self, hdr, value):
         self.assert_no_newline(value)

mailpile/i18n.py

@@ -22,8 +22,8 @@ def _fmt_safe(translation, original):
         return FORMAT_CHECKED[translation]
     if '%' in original:
         try:
-            assert(len([c for c in translation if c == '%'])
-                   == len([c for c in original if c == '%']))
+            safe_assert(len([c for c in translation if c == '%'])
+                        == len([c for c in original if c == '%']))
             bogon = translation % 1
             FORMAT_CHECKED[translation] = translation
         except TypeError:

mailpile/mail_source/imap.py

@@ -177,7 +177,7 @@ def __init__(self, session, conn, idle_mailbox=None, idle_callback=None):
     def _mk_proxy(self, method):
         def proxy_method(*args, **kwargs):
             try:
-                assert(self._lock.locked())
+                safe_assert(self._lock.locked())
                 if 'mailbox' in kwargs:
                     # We're sharing this connection, so all mailbox methods
                     # need to tell us which mailbox they're operating on.
@@ -227,7 +227,7 @@ def _shutdown(self):
             self._update_name()
 
     def close(self):
-        assert(self._lock.locked())
+        safe_assert(self._lock.locked())
         self._selected = None
         if '(closed)' not in self.name:
             self.name += ' (closed)'
@@ -237,7 +237,7 @@ def select(self, mailbox='INBOX', readonly=False):
         # This routine caches the SELECT operations, because we will be
         # making lots and lots of superfluous ones "just in case" as part
         # of multiplexing one IMAP connection for multiple mailboxes.
-        assert(self._lock.locked())
+        safe_assert(self._lock.locked())
         if self._selected and self._selected[0] == (mailbox, readonly):
             return self._selected[1]
         elif self._selected:

mailpile/mailutils/__init__.py

@@ -157,7 +157,7 @@ def MakeGnuPG(*args, **kwargs):
         try:
             if not hasattr(fd, 'read'):  # Not a file, is it a function?
                 fd = fd()
-            assert(hasattr(fd, 'read'))
+            safe_assert(hasattr(fd, 'read'))
         except (TypeError, AssertionError):
             return None
 
@@ -287,7 +287,7 @@ def PrepareMessage(config, msg,
 
     rcpts = rcpts or []
     if bounce:
-        assert(len(rcpts) > 0)
+        safe_assert(len(rcpts) > 0)
 
     # Iterate through headers to figure out what we want to do...
     need_rcpts = not rcpts

mailpile/plugins/__init__.py

@@ -141,7 +141,7 @@ def discover(self, paths, update=False):
                 try:
                     with open(manifest_filename) as mfd:
                         manifest = json.loads(self._uncomment(mfd.read()))
-                        assert(manifest.get('name') == subdir)
+                        safe_assert(manifest.get('name') == subdir)
                         # FIXME: Need more sanity checks
                         self.DISCOVERED[pname] = (plug_path, manifest)
                 except (ValueError, AssertionError):
@@ -167,7 +167,7 @@ def _import(self, full_name, full_path):
                 sys.modules[mp] = imp.new_module(mp)
                 sys.modules[module].__dict__[parent] = sys.modules[mp]
             module = mp
-        assert(module == full_name)
+        safe_assert(module == full_name)
 
         # load actual module
         sys.modules[full_name].__file__ = full_path
@@ -715,8 +715,8 @@ def register_slow_periodic_job(self, name, period, callback):
 
     def register_worker(self, thread_obj):
         self._compat_check()
-        assert(hasattr(thread_obj, 'start'))
-        assert(hasattr(thread_obj, 'quit'))
+        safe_assert(hasattr(thread_obj, 'start'))
+        safe_assert(hasattr(thread_obj, 'quit'))
         # FIXME: complain about duplicates?
         self.WORKERS.append(thread_obj)
 

mailpile/plugins/contacts.py

@@ -1287,9 +1287,9 @@ def command(self):
         session, config = self.session, self.session.config
 
         # OK, fetch the VCard.
-        assert('rid' in self.data and len(self.data['rid']) == 1)
+        safe_assert('rid' in self.data and len(self.data['rid']) == 1)
         vcard = config.vcards.get_vcard(self.data['rid'][0])
-        assert(vcard)
+        safe_assert(vcard)
 
         if self.data.get('_method') == 'POST':
             self._update_vcard_from_post(vcard)

mailpile/plugins/core.py

@@ -476,7 +476,7 @@ class Cleanup(Command):
 
     @classmethod
     def AddTask(cls, task, last=False, first=False):
-        assert(not (first and last))
+        safe_assert(not (first and last))
         if (first or last) and not cls.TASKS:
             cls.TASKS = [lambda: True]
         if first:

mailpile/plugins/keylookup/__init__.py

@@ -255,7 +255,7 @@ def command(self):
             address = self.data.get('address', [''])[0]
             fprints = self.data.get('fingerprints', [])
             origins = self.data.get('origins', [])
-        assert(address or fprints or origins)
+        safe_assert(address or fprints or origins)
 
         result = lookup_crypto_keys(self.session, address,
                                     get=[f.strip() for f in fprints],
@@ -308,7 +308,7 @@ def _seen_enough_signatures(self, idx, email, keyinfo):
 
     def command(self):
         emails = set(list(self.args)) | set(self.data.get('email', []))
-        assert(emails)
+        safe_assert(emails)
 
         idx = self._idx()
         gnupg = self._gnupg(dry_run=True)

mailpile/plugins/setup_magic.py

@@ -820,11 +820,12 @@ def _test_login_and_proto(self, email, settings):
 
         if settings['protocol'].startswith('smtp'):
             try:
-                assert(SendMail(self.session, None,
-                                [(email,
-                                  [email, '[email protected]'], None,
-                                  [event])],
-                                test_only=True, test_route=settings))
+                safe_assert(
+                    SendMail(self.session, None,
+                             [(email,
+                               [email, '[email protected]'], None,
+                               [event])],
+                             test_only=True, test_route=settings))
                 return True, True
             except (IOError, OSError, AssertionError, SendMailError):
                 pass
@@ -1161,7 +1162,7 @@ def setup_command(self, session):
 
         if route_id:
             route = self.session.config.routes[route_id]
-            assert(route)
+            safe_assert(route)
         else:
             route = {}
             for k in CONFIG_RULES['routes'][1]:
@@ -1178,16 +1179,17 @@ def setup_command(self, session):
         if not fromaddr or '@' not in fromaddr:
             fromaddr = '%s@%s' % (route.get('username', 'test'),
                                   route.get('host', 'example.com'))
-        assert(fromaddr)
+        safe_assert(fromaddr)
 
         error_info = {'error': _('Unknown error')}
         try:
-            assert(SendMail(self.session, None,
-                            [(fromaddr,
-                              [fromaddr, '[email protected]'],
-                              None,
-                              [self.event])],
-                            test_only=True, test_route=route))
+            safe_assert(
+                SendMail(self.session, None,
+                         [(fromaddr,
+                           [fromaddr, '[email protected]'],
+                           None,
+                           [self.event])],
+                         test_only=True, test_route=route))
             return self._success(_('Route is working'),
                                  result=route)
         except OSError:
@@ -1241,7 +1243,7 @@ def auto_configure_tor(self, session, hostport=None):
             with ConnBroker.context(need=need_tor) as context:
                 motd = urlopen(MOTD_URL_TOR_ONLY_NO_MARS,
                                data=None, timeout=10).read()
-                assert(motd.strip().endswith('}'))
+                safe_assert(motd.strip().endswith('}'))
             session.config.sys.proxy.protocol = 'tor'
             message = _('Successfully configured and enabled Tor!')
         except (IOError, AssertionError):

mailpile/safe_popen.py

@@ -85,10 +85,11 @@ def __init__(self, args, bufsize=0,
         # platforms, so we don't allow the programmer to configure them
         # at all.
         if SERIALIZE_POPEN_STRICT:
-            assert(preexec_fn is None)
-            assert(close_fds is None)
-            assert(startupinfo is None)
-            assert(creationflags is None)
+            if not ((preexec_fn is None) and
+                    (close_fds is None) and
+                    (startupinfo is None) and
+                    (creationflags is None)):
+                raise AssertionError("Unsafe use of POpen API!")
 
         # The goal of the following sections is to achieve two things:
         #

mailpile/security.py

@@ -383,7 +383,7 @@ def __init__(self, sps):
                 self.offset = 0
 
             def seek(self, offset, whence=0):
-                assert(whence == 0)
+                safe_assert(whence == 0)
                 self.offset = offset
 
             def read(self, ignored_bytecount=None):

mailpile/util.py

@@ -216,6 +216,10 @@ class AccessError(Exception):
     pass
 
 
+class InternalError(AssertionError):
+    pass
+
+
 class UrlRedirectException(Exception):
     """An exception indicating we need to redirecting to another URL."""
     def __init__(self, url):
@@ -247,6 +251,12 @@ def __exit__(self, *args, **kwargs):
             raise raised[0]
 
 
+def safe_assert(check, *args):
+    """A safe-to-use assert() replacement that never gets compiled out."""
+    if not check:
+        raise InternalError(*args)
+
+
 def thread_context_push(**kwargs):
     if not hasattr(THREAD_LOCAL, 'context'):
         THREAD_LOCAL.context = []

mailpile/vcard.py

@@ -947,7 +947,7 @@ def recent_history(self, now=None):
             return {}
 
     def record_history(self, what, when, mid, now=None):
-        assert(what[0] in ('s', 'r'))
+        safe_assert(what[0] in ('s', 'r'))
         with self._lock:
             try:
                 history_vcl = self.get('x-mailpile-history')

mailpile/vfs.py

@@ -23,6 +23,7 @@
 import posixpath
 
 from mailpile.i18n import gettext as _
+from mailpile.util import safe_assert
 
 
 VFS_HANDLERS = []
@@ -36,7 +37,7 @@ def register_handler(prio, obj):
 
 def register_alias(name, prefix):
     global VFS_ALIASES
-    assert(name[:1] == '/')
+    safe_assert(name[:1] == '/')
     VFS_ALIASES[name] = prefix
 
 
@@ -51,7 +52,7 @@ class FilePath(object):
     is suitable for writing to a config file or JSON stream.
     """
     def __init__(self, cooked_fp=None, binary_fp=None, flags=None):
-        assert((cooked_fp or binary_fp) and not (cooked_fp and binary_fp))
+        safe_assert((cooked_fp or binary_fp) and not (cooked_fp and binary_fp))
         if cooked_fp:
             if isinstance(cooked_fp, FilePath):
                 self.raw_fp = cooked_fp.raw_fp