commit 514026ceb102a359011016f842cd63fd5e6dd1ea Author: Isis Lovecruft isis@torproject.org Date: Sat Sep 6 03:00:41 2014 +0000
Fix an additional four bugs in bridgdb.Bridges.parseExtraInfoFile().
In addition to the one-character bug for #12932 which was fixed in commit 487c1b6c16e979bc448ddd415364e28bd764d29a, there were an additional four bugs in the legacy parser, `bridgedb.Bridges.parseExtraInfoFile()` (which I am about to deprecate anyway for #9380):
# get the transport line if ID and line.startswith("transport "): fields = line[10:].split() # [ arglist ] field, optional if len(fields) >= 3: arglist = fields[2:] # BUGS 1 and 2 # parse arglist [k=v,...k=v] as argdict {k:v,...,k:v} argdict = {} for arg in arglist: try: k,v = arg.split('=') # BUG 3 except ValueError: continue # BUG 4 argdict[k] = v logging.debug(" Parsing Argument: %s: %s", k, v)
BUG 1: This assumes the PT arguments are space-separated in the extrainfo descriptor. They are not; they are comma-separated.
BUG 2: This would result in parsing the entire, comma-separated group of PT arguments into:
{"key1": "a,key2=b,key3=c"}
BUG 3: This would produce a ValueError, because there's more than one '=' character. (Meaning that the whole set of arguments would be discarded due to Bug #4.)
BUG 4: The whole set of arguments gets discarded, without even so much as a log message, if there was more than one argument.
These are all bug fixes on a single commit, 4300329a30f3b6aa3e390b140193dd50faa6e03f, from #4568. And I'm still deprecating the entire function anyway (for #9380) because the rest of it is likely just as full of bugs.
* FIXES #12932 https://bugs.torproject.org/12932 --- lib/bridgedb/Bridges.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py index 613e2db..a3f50b4 100644 --- a/lib/bridgedb/Bridges.py +++ b/lib/bridgedb/Bridges.py @@ -596,16 +596,24 @@ def parseExtraInfoFile(f): # get the transport line if ID and line.startswith("transport "): fields = line[10:].split() - # [ arglist ] field, optional + if len(fields) >= 3: - arglist = fields[2:] - # parse arglist [k=v,...k=v] as argdict {k:v,...,k:v} argdict = {} - for arg in arglist: - try: k,v = arg.split('=') - except ValueError: continue - argdict[k] = v - logging.debug(" Parsing Argument: %s: %s", k, v) + # PT argumentss are comma-separated in the extrainfo + # descriptors. While there *shouldn't* be anything after them + # that was separated by a space (and hence would wind up being + # in a different `field`), if there was we'll join it to the + # rest of the PT arguments with a comma so that they are + # parsed as if they were PT arguments as well: + allargs = ','.join(fields[2:]) + for arg in allargs.split(','): + try: + k, v = arg.split('=') + except ValueError: + logging.warn(" Couldn't parse K=V from PT arg: %r" % arg) + else: + logging.debug(" Parsed PT Argument: %s: %s" % (k, v)) + argdict[k] = v
# get the required fields, method name and address if len(fields) >= 2: