commit aa5d17329201a9723d14b71d050c34228a3f2886 Author: Isis Lovecruft <isis@torproject.org> Date: Fri Nov 22 00:38:01 2013 +0000 Catch all parser exceptions in parse.networkstatus.parseRLine(). Rather than raising exceptions during parsing, we should try to log appropriate messages. If the error had a known cause, i.e. one that is due to a badly formatted descriptor, then we should make note of the relevant information from the bad descriptor, and reset any affected variables to None. If the error is possibly do to a newly implemented change in the descriptor format in tor, we should likewise log the event, generate a loud warning about a possible descriptor format change, and continue as before. In networkstatus documents and [bridge-]server-descriptors, it is possible that the `ORAddress` lines may soon change (#9729): 1. to allow more than one ORAddress line, and 2. to allow extra IPv4 addresses in the ORAddress lines. Additionally, remove exception and handlers which are never raised in parseRLine(). 1. The catch for `IndexError`s in networkstatus.parseRLine() is removed: At the beginning of `parseRLine()`, a `NetworkstatusParsingError` is raised if there are less than eight fields in the descriptor, so there is no way that an `IndexError` would ever get raised. Coverage branch reports also show that this `except:` block is never touched. 2. Remove the `InvalidNetworkstatusDescriptorDigest` exception class: …as well as all corresponding code for raising and handling it. The way it was used, there is no possible way that these lines will get executed, because the descriptor digest will either be a str or None. If None, then an error is raised earlier for having too few fields in the 'r'-line. If a str, then `binascii.a2b_base64() will always produce some kind of string, and its exceptions are handled separate. Therefore, there is no way for the descriptor digest to be 0, None, or False. In `bridgedb.parse.networkstatus.parseRLine()` function: * CHANGE to catch and handle all `InvalidNetworkstatusRouterIdentity`, and `NetworkstatusParsingErrors`, exceptions raised during parsing. * REMOVE unnecessary `except IndexError` block. * REMOVE `InvalidNetworkstatusDescriptorDigest` exception class. * CHANGE logging of caught exceptions to use `exc`, not `exc.message`. The later has been deprecated since Python2.6. --- lib/bridgedb/parse/networkstatus.py | 81 ++++++++++++++++------------------- 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/lib/bridgedb/parse/networkstatus.py b/lib/bridgedb/parse/networkstatus.py index a20ddb4..294e465 100644 --- a/lib/bridgedb/parse/networkstatus.py +++ b/lib/bridgedb/parse/networkstatus.py @@ -27,6 +27,9 @@ import binascii import logging import string import time +import warnings + +from twisted.python.log import showwarning from bridgedb.parse import addr from bridgedb.parse import padBase64 @@ -38,9 +41,6 @@ class NetworkstatusParsingError(Exception): class InvalidNetworkstatusRouterIdentity(ValueError): """The ID field of a networkstatus document 'r'-line is invalid.""" -class InvalidNetworkstatusDescriptorDigest(ValueError): - """Descriptor digest of a networkstatus document 'r'-line is invalid.""" - class InvalidRouterNickname(ValueError): """Router nickname doesn't follow tor-spec.""" @@ -94,60 +94,55 @@ def parseRLine(line): (nickname, ID, descDigest, timestamp, ORaddr, ORport, dirport) = (None for x in xrange(7)) - if not line.startswith('r '): - raise NetworkstatusParsingError( - "Networkstatus parser received non 'r'-line: %r" % line) + try: + if not line.startswith('r '): + raise NetworkstatusParsingError( + "Networkstatus parser received non 'r'-line: %r" % line) - line = line[2:] # Chop of the 'r ' + line = line[2:] # Chop of the 'r ' + fields = line.split() - fields = line.split() - if len(fields) != 8: - raise NetworkstatusParsingError( - "Wrong number of fields in networkstatus 'r'-line: %r" % line) + if len(fields) != 8: + raise NetworkstatusParsingError( + "Wrong number of fields in networkstatus 'r'-line: %r" % line) - try: nickname, ID = fields[:2] - isValidRouterNickname(nickname) if ID.endswith('='): raise InvalidNetworkstatusRouterIdentity( - "Skipping networkstatus parsing for router with nickname %r:"\ - "\n\tUnpadded, base64-encoded networkstatus router identity "\ + "Skipping networkstatus parsing for router with nickname "\ + "'%s':\n Unpadded, base64-encoded networkstatus router identity "\ "string ends with '=': %r" % (nickname, ID)) - try: - ID = padBase64(ID) # Add the trailing equals sign back in - except (AttributeError, ValueError) as error: - raise InvalidNetworkstatusRouterIdentity(error.message) - - ID = binascii.a2b_base64(ID) - if not ID: + paddedID = padBase64(ID) # Add the trailing equals sign back in + debasedID = binascii.a2b_base64(paddedID) + if not debasedID: raise InvalidNetworkstatusRouterIdentity( - "Skipping networkstatus parsing for router with nickname %r:"\ - "\n\tBase64-encoding for networkstatus router identity string"\ - "is invalid!\n\tLine: %r" % (nickname, line)) - - except IndexError as error: - logging.error(error.message) + "Skipping networkstatus parsing for router with nickname "\ + "'%s':\n Base64-encoding for networkstatus router identity "\ + "string is invalid!\n Line: %r" % (nickname, line)) + ID = debasedID + + except NetworkstatusParsingError as error: + logging.error(error) + nickname, ID = None, None except InvalidRouterNickname as error: - logging.error(error.message) + logging.error(error) nickname = None except InvalidNetworkstatusRouterIdentity as error: - logging.error(error.message) + logging.error(error) ID = None - - try: - descDigest = binascii.a2b_base64(fields[2]) - except (AttributeError, ValueError) as error: - raise InvalidNetworkstatusDescriptorDigest(error.message) - - - timestamp = time.mktime(time.strptime(" ".join(fields[3:5]), - "%Y-%m-%d %H:%M:%S")) - ORaddr = fields[5] - ORport = fields[6] - dirport = fields[7] - + else: + try: + descDigest = binascii.a2b_base64(fields[2]) + timestamp = time.mktime(time.strptime(" ".join(fields[3:5]), + "%Y-%m-%d %H:%M:%S")) + ORaddr = fields[5] + ORport = fields[6] + dirport = fields[7] + except (AttributeError, ValueError, IndexError) as error: + logging.error(error) + descDigest = None finally: return (nickname, ID, descDigest, timestamp, ORaddr, ORport, dirport)