[tor-commits] [stem/master] Better handling quoted key/value mappings in events

[atagar at torproject.org]

commit ae1f59b6b0ec1550434bc1508c7cd9ce866fa616
Author: Damian Johnson <atagar at torproject.org>
Date:   Mon Dec 3 21:04:10 2012 -0800

    Better handling quoted key/value mappings in events
    
    We were being pretty picky about key/value mappings with quoted values, only
    accepting them if they were in both our _KEYWORD_ARGS and _QUOTED. The trouble
    with this was...
    
    1. It make our parsing code more convoluted than it needed to be.
    
    2. If tor added a new quoted key/value mapping then it would likely break our
       parser. This is because we'd mis-interpret the value as being positional
       fields. For instance if...
    
       650 MY_EVENT NEW_USER username=atagar
    
       ... became...
    
       650 MY_EVENT NEW_USER username=atagar message="hello world"
    
       Then we'd misinterpret the event as having no keyword arguments, and rather
       have four positional arguments (the last being 'world"').
    
       This isn't strictly wrong according to the control-spec. It's good about
       specifiying how new positional and keyword arguments should be handled, but
       quiet about quoted values.
    
       However, accepting quoted mappings by default is more intuitive and less
       likely to cause sadness down the road so making it so.
---
 stem/control.py         |    1 +
 stem/response/events.py |   64 ++++++++++------------------------------------
 2 files changed, 15 insertions(+), 50 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index 859cc16..8665a51 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -91,6 +91,7 @@ providing its own for interacting at a higher level.
   **ERR**               :class:`stem.response.events.LogEvent`
   **GUARD**             :class:`stem.response.events.GuardEvent`
   **INFO**              :class:`stem.response.events.LogEvent`
+  **NEWCONSENSUS**      :class:`stem.response.events.NewConsensusEvent`
   **NEWDESC**           :class:`stem.response.events.NewDescEvent`
   **NOTICE**            :class:`stem.response.events.LogEvent`
   **NS**                :class:`stem.response.events.NetworkStatusEvent`
diff --git a/stem/response/events.py b/stem/response/events.py
index e5ee85c..1559ec5 100644
--- a/stem/response/events.py
+++ b/stem/response/events.py
@@ -13,7 +13,8 @@ from stem.util import connection, log, str_tools, tor_tools
 # because some positional arguments, like circuit paths, can have an equal
 # sign.
 
-KW_ARG = re.compile("([A-Za-z0-9_]+)=(.*)")
+KW_ARG = re.compile("^(.*) ([A-Za-z0-9_]+)=(\S*)$")
+QUOTED_KW_ARG = re.compile("^(.*) ([A-Za-z0-9_]+)=\"(.*)\"$")
 
 class Event(stem.response.ControlMessage):
   """
@@ -32,11 +33,6 @@ class Event(stem.response.ControlMessage):
   _QUOTED = ()
   _SKIP_PARSING = False
   
-  # If set then we'll parse anything that looks like a quoted key/value
-  # mapping, reguardless of if it shows up in _QUOTED.
-  
-  _PERMISSIVE_QUOTED_MAPPINGS = False
-  
   def _parse_message(self, arrived_at):
     if not str(self).strip():
       raise stem.ProtocolError("Received a blank tor event. Events must at the very least have a type.")
@@ -67,52 +63,26 @@ class Event(stem.response.ControlMessage):
     **_POSITIONAL_ARGS** and **_KEYWORD_ARGS**.
     """
     
-    # Whoever decided to allow for quoted attributes in events should be
-    # punished. Preferably under some of those maritime laws that allow for
-    # flogging. Event parsing was nice until we threw this crap in...
-    #
-    # Pulling quoted keyword arguments out here. Quoted positonal arguments
-    # are handled later.
-    
-    content = str(self)
-    
-    if self._PERMISSIVE_QUOTED_MAPPINGS:
-      while True:
-        match = re.match("^(.*) (\S*)=\"(.*)\"(.*)$", content)
-        
-        if match:
-          prefix, keyword, value, suffix = match.groups()
-          content = prefix + suffix
-          self.keyword_args[keyword] = value
-        else:
-          break
-    else:
-      for keyword in set(self._QUOTED).intersection(set(self._KEYWORD_ARGS.keys())):
-        match = re.match("^(.*) %s=\"(.*)\"(.*)$" % keyword, content)
-        
-        if match:
-          prefix, value, suffix = match.groups()
-          content = prefix + suffix
-          self.keyword_args[keyword] = value
-    
-    fields = content.split()[1:]
-    
     # Tor events contain some number of positional arguments followed by
     # key/value mappings. Parsing keyword arguments from the end until we hit
     # something that isn't a key/value mapping. The rest are positional.
     
-    while fields:
-      kw_match = KW_ARG.match(fields[-1])
+    content = str(self)
+    
+    while True:
+      match = QUOTED_KW_ARG.match(content)
       
-      if kw_match:
-        k, v = kw_match.groups()
-        self.keyword_args[k] = v
-        fields.pop() # remove the field
+      if not match:
+        match = KW_ARG.match(content)
+      
+      if match:
+        content, keyword, value = match.groups()
+        self.keyword_args[keyword] = value
       else:
-        # not a key/value mapping, the remaining fields are positional
-        self.positional_args = fields
         break
     
+    self.positional_args = content.split()[1:]
+    
     # Setting attributes for the fields that we recognize. Unrecognized fields
     # only appear in our 'positional_args' and 'keyword_args' attributes.
     
@@ -181,11 +151,6 @@ class AddrMapEvent(Event):
   :var datetime utc_expiry: expiration time of the resolution in UTC
   """
   
-  # TODO: The spec for this event is a little vague. Making a couple guesses
-  # about it...
-  #
-  # https://trac.torproject.org/7515
-  
   _POSITIONAL_ARGS = ("hostname", "destination", "expiry")
   _KEYWORD_ARGS = {
     "error": "error",
@@ -698,7 +663,6 @@ class StatusEvent(Event):
   """
   
   _POSITIONAL_ARGS = ("runlevel", "action")
-  _PERMISSIVE_QUOTED_MAPPINGS = True
   
   def _parse(self):
     if self.type == 'STATUS_GENERAL':