[bridgedb/develop] Fix an additional four bugs in bridgdb.Bridges.parseExtraInfoFile(). - tor-commits

20 Sep 2014

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:

    