[tor-commits] [stem/master] Multiple fixes to GETCONF parsing

atagar at torproject.org atagar at torproject.org
Wed Jul 4 21:34:20 UTC 2012


commit 3b25147b7fcaadbc7e2aec6ec89158ed1d738687
Author: Ravi Chandra Padmala <neenaoffline at gmail.com>
Date:   Tue Jun 12 19:59:30 2012 +0530

    Multiple fixes to GETCONF parsing
    
    * Add stem.socket.InvalidArguments and make the GETCONF parser raise it instead
      of stem.socket.InvalidRequest.
    
    * Fix the parser to parser to correctly parse multivalue configuration keys
      (such as ExitPolicy). Add tests for the same.
    
    * Fix tests to run against a Tor client using a control socket file.
    
    * Minor changes to documentation.
---
 stem/control.py                  |    2 +-
 stem/response/__init__.py        |    2 +-
 stem/response/getconf.py         |   49 ++++++++++++++++++++++++--------------
 stem/socket.py                   |   21 +++++++++++++++-
 test/integ/control/controller.py |   20 ++++++++++-----
 test/unit/response/getconf.py    |   33 ++++++++++++++++++++++++-
 6 files changed, 97 insertions(+), 30 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index c6bf29d..f0459cb 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -550,7 +550,7 @@ class Controller(BaseController):
     
     :raises:
       :class:`stem.socket.ControllerError` if the call fails, and we weren't provided a default response
-      :class:`stem.socket.InvalidRequest` if configuration option requested was invalid.
+      :class:`stem.socket.InvalidArguments` if configuration options requested was invalid
     """
     
     if isinstance(param, str):
diff --git a/stem/response/__init__.py b/stem/response/__init__.py
index a9b69a3..43ecb3b 100644
--- a/stem/response/__init__.py
+++ b/stem/response/__init__.py
@@ -60,7 +60,7 @@ def convert(response_type, message):
   
   :raises:
     * :class:`stem.socket.ProtocolError` the message isn't a proper response of that type
-    * :class:`stem.socket.InvalidRequest` the request was invalid
+    * :class:`stem.socket.InvalidArguments` the request's arguments are invalid
     * TypeError if argument isn't a :class:`stem.response.ControlMessage` or response_type isn't supported
   """
   
diff --git a/stem/response/getconf.py b/stem/response/getconf.py
index c1872cd..cffbc61 100644
--- a/stem/response/getconf.py
+++ b/stem/response/getconf.py
@@ -1,8 +1,24 @@
-import re
-
 import stem.socket
 import stem.response
 
+def _getval(dictionary, key):
+  try:
+    return dictionary[key]
+  except KeyError:
+    pass
+
+def _split_line(line):
+  try:
+    if '=' in line:
+      if line[line.find("=") + 1] == "\"":
+        return line.pop_mapping(True)
+      else:
+        return line.split("=", 1)
+    else:
+      return (line, None)
+  except IndexError:
+    return (line[:-1], None)
+
 class GetConfResponse(stem.response.ControlMessage):
   """
   Reply for a GETCONF query.
@@ -25,29 +41,26 @@ class GetConfResponse(stem.response.ControlMessage):
     if not self.is_ok():
       unrecognized_keywords = []
       for code, _, line in self.content():
-        if code == '552':
-          try:
-            # to parse: 552 Unrecognized configuration key "zinc"
-            unrecognized_keywords.append(re.search('"([^"]+)"', line).groups()[0])
-          except:
-            pass
+        if code == '552' and line.startswith("Unrecognized configuration key \"") and line.endswith("\""):
+          unrecognized_keywords.append(line[32:-1])
 
       if unrecognized_keywords:
-        raise stem.socket.InvalidRequest("GETCONF request contained unrecognized keywords: %s\n" \
-            % ', '.join(unrecognized_keywords))
+        raise stem.socket.InvalidArguments("GETCONF request contained unrecognized keywords: %s\n" \
+            % ', '.join(unrecognized_keywords), unrecognized_keywords)
       else:
         raise stem.socket.ProtocolError("GETCONF response contained a non-OK status code:\n%s" % self)
     
     while remaining_lines:
       line = remaining_lines.pop(0)
 
-      if '=' in line:
-        if line[line.find("=") + 1] == "\"":
-          key, value = line.pop_mapping(True)
-        else:
-          key, value = line.split("=", 1)
+      key, value = _split_line(line)
+      entry = _getval(self.entries, key)
+
+      if type(entry) == str and entry != value:
+        self.entries[key] = [entry]
+        self.entries[key].append(value)
+      elif type(entry) == list and not value in entry:
+        self.entries[key].append(value)
       else:
-        key, value = (line, None)
-      
-      self.entries[key] = value
+        self.entries[key] = value
 
diff --git a/stem/socket.py b/stem/socket.py
index 258a7dc..b933f50 100644
--- a/stem/socket.py
+++ b/stem/socket.py
@@ -28,7 +28,8 @@ as instances of the :class:`stem.response.ControlMessage` class.
   
   ControllerError - Base exception raised when using the controller.
     |- ProtocolError - Malformed socket data.
-    |- InvalidRequest - Invalid request parameters.
+    |- InvalidRequest - Invalid request.
+       +- InvalidArguments - Invalid request parameters.
     +- SocketError - Communication with the socket failed.
        +- SocketClosed - Socket has been shut down.
 """
@@ -552,6 +553,24 @@ class ProtocolError(ControllerError):
 class InvalidRequest(ControllerError):
   "Base Exception class for invalid requests"
 
+class InvalidArguments(InvalidRequest):
+  """
+  Exception class for invalid requests which contain invalid arguments.
+
+  :var list arguments: a list of parameters which were invalid
+  """
+
+  def __init__(self, message, arguments):
+    """
+    Initializes an InvalidArguments object.
+
+    :param str message: error message
+    :param list arguments: a list of parameters which were invalid
+
+    :returns: object of InvalidArguments class
+    """
+    self.arguments = arguments
+
 class SocketError(ControllerError):
   "Error arose while communicating with the control socket."
 
diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py
index 479c75b..2441eac 100644
--- a/test/integ/control/controller.py
+++ b/test/integ/control/controller.py
@@ -145,16 +145,22 @@ class TestController(unittest.TestCase):
     with runner.get_tor_controller() as controller:
       # successful single query
       
-      control_port = str(runner.get_tor_socket().get_port())
-      torrc_path = runner.get_torrc_path()
-      self.assertEqual(control_port, controller.get_conf("ControlPort"))
-      self.assertEqual(control_port, controller.get_conf("ControlPort", "la-di-dah"))
+      socket = runner.get_tor_socket()
+      if isinstance(socket, stem.socket.ControlPort):
+        socket = str(socket.get_port())
+        config_key = "ControlPort"
+      elif isinstance(socket, stem.socket.ControlSocketFile):
+        socket = str(socket.get_socket_path())
+        config_key = "ControlSocket"
+      
+      self.assertEqual(socket, controller.get_conf(config_key))
+      self.assertEqual(socket, controller.get_conf(config_key, "la-di-dah"))
       
       # succeessful batch query
       
-      expected = {"ControlPort": control_port}
-      self.assertEqual(expected, controller.get_conf(["ControlPort"]))
-      self.assertEqual(expected, controller.get_conf(["ControlPort"], "la-di-dah"))
+      expected = {config_key: socket}
+      self.assertEqual(expected, controller.get_conf([config_key]))
+      self.assertEqual(expected, controller.get_conf([config_key], "la-di-dah"))
       
       getconf_params = set(["ControlPort", "DirPort", "DataDirectory"])
       self.assertEqual(getconf_params, set(controller.get_conf(["ControlPort",
diff --git a/test/unit/response/getconf.py b/test/unit/response/getconf.py
index 5dd8964..a9f1cc8 100644
--- a/test/unit/response/getconf.py
+++ b/test/unit/response/getconf.py
@@ -20,6 +20,15 @@ BATCH_RESPONSE = """\
 250-DataDirectory=/tmp/fake dir
 250 DirPort"""
 
+MULTIVALUE_RESPONSE = """\
+250-ControlPort=9100
+250-ExitPolicy=accept 34.3.4.5
+250-ExitPolicy=accept 3.4.53.3
+250-ExitPolicy=reject 23.245.54.3
+250-ExitPolicy=accept 34.3.4.5
+250-ExitPolicy=accept 3.4.53.3
+250 ExitPolicy=reject 23.245.54.3"""
+
 UNRECOGNIZED_KEY_RESPONSE = "552 Unrecognized configuration key \"yellowbrickroad\""
 
 INVALID_RESPONSE = """\
@@ -67,15 +76,35 @@ class TestGetConfResponse(unittest.TestCase):
     
     self.assertEqual(expected, control_message.entries)
   
+  def test_multivalue_response(self):
+    """
+    Parses a GETCONF reply containing a single key with multiple parameters.
+    """
+    
+    control_message = mocking.get_message(MULTIVALUE_RESPONSE)
+    stem.response.convert("GETCONF", control_message)
+    
+    expected = {
+      "ControlPort": "9100",
+      "ExitPolicy": ["accept 34.3.4.5", "accept 3.4.53.3", "reject 23.245.54.3"]
+    }
+    
+    self.assertEqual(expected, control_message.entries)
+  
   def test_unrecognized_key_response(self):
     """
     Parses a GETCONF reply that contains an error code with an unrecognized key.
     """
     
     control_message = mocking.get_message(UNRECOGNIZED_KEY_RESPONSE)
-    self.assertRaises(stem.socket.InvalidRequest, stem.response.convert, "GETCONF", control_message)
+    self.assertRaises(stem.socket.InvalidArguments, stem.response.convert, "GETCONF", control_message)
+    
+    try:
+      stem.response.convert("GETCONF", control_message)
+    except stem.socket.InvalidArguments, exc:
+      self.assertEqual(exc.arguments, ["yellowbrickroad"])
   
-  def test_invalid_multiline_content(self):
+  def test_invalid_content(self):
     """
     Parses a malformed GETCONF reply that contains an invalid response code.
     This is a proper controller message, but malformed according to the





More information about the tor-commits mailing list