[tor-commits] [stem/master] Changes for review

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


commit 765c497f824fde380d8d1b864e3dec36fcce3b11
Author: Ravi Chandra Padmala <neenaoffline at gmail.com>
Date:   Fri Jun 29 23:12:34 2012 +0530

    Changes for review
---
 stem/control.py                  |   72 +++++++++++++++++++++++++++-----------
 stem/response/getconf.py         |    7 ++--
 stem/socket.py                   |   23 +++++-------
 stem/util/__init__.py            |    2 +-
 stem/util/control.py             |   26 ++++++++++++++
 test/integ/control/controller.py |   29 ++++++++-------
 test/runner.py                   |    2 -
 7 files changed, 108 insertions(+), 53 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index d4c9db5..94427a7 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -31,6 +31,7 @@ interacting at a higher level.
 
 from __future__ import with_statement
 
+import re
 import time
 import Queue
 import threading
@@ -39,6 +40,7 @@ import stem.connection
 import stem.response
 import stem.socket
 import stem.version
+import stem.util.control
 import stem.util.log as log
 
 # state changes a control socket can have
@@ -405,11 +407,11 @@ class Controller(BaseController):
   """
   
   _mapped_config_keys = {
-      "HiddenServiceDir": "HiddenServiceOptions",
-      "HiddenServicePort": "HiddenServiceOptions",
-      "HiddenServiceVersion": "HiddenServiceOptions",
-      "HiddenServiceAuthorizeClient": "HiddenServiceOptions",
-      "HiddenServiceOptions": "HiddenServiceOptions"
+      "hiddenservicedir": "hiddenserviceoptions",
+      "hiddenserviceport": "hiddenserviceoptions",
+      "hiddenserviceversion": "hiddenserviceoptions",
+      "hiddenserviceauthorizeclient": "hiddenserviceoptions",
+      "hiddenserviceoptions": "hiddenserviceoptions"
       }
   
   def from_port(control_addr = "127.0.0.1", control_port = 9051):
@@ -546,7 +548,8 @@ class Controller(BaseController):
     Queries the control socket for the value of a given configuration option. If
     provided a default then that's returned as if the GETCONF option is undefined
     or if the call fails for any reason (invalid configuration option, error
-    response, control port closed, initiated, etc).
+    response, control port closed, initiated, etc). If the configuration key
+    consists of whitespace only, None is returned, unless a default value is given.
     
     :param str param: GETCONF option to be queried
     :param object default: response if the query fails
@@ -561,24 +564,43 @@ class Controller(BaseController):
       * default if one was provided and our call failed
     
     :raises:
-      :class:`stem.socket.ControllerError` if the call fails, and we weren't provided a default response
+      :class:`stem.socket.ControllerError` if the call fails and we weren't provided a default response
       :class:`stem.socket.InvalidArguments` if the configuration option requested was invalid
     """
     
+    if re.match("^\s*$", param):
+      if default != UNDEFINED: return default
+      else: return
     # automagically change the requested parameter if it's context sensitive
     # and cannot be returned on it's own.
-    if param.lower() in self._mapped_config_keys.keys():
-      return self.get_conf_map(self._mapped_config_keys[param], default, multiple)[param]
-    else:
-      return self.get_conf_map(param, default, multiple)[param]
+    param = param.lower()
+    return self.get_conf_map(self._mapped_config_keys.get(param, param), default, multiple)[param]
   
-  def get_conf_map(self, param, default = UNDEFINED, multiple = False):
+  def get_conf_map(self, param, default = UNDEFINED, multiple = True):
     """
     Queries the control socket for the values of given configuration options and
     provides a mapping of the keys to the values. If provided a default then
     that's returned as if the GETCONF option is undefined or if the call fails
     for any reason (invalid configuration option, error response, control port
-    closed, initiated, etc).
+    closed, initiated, etc). Configuration keys that contain only whitespace are
+    ignored.
+    
+    There's three use cases for GETCONF:
+    - a single value is provided
+    - multiple values are provided for the option queried
+    - a set of options that weren't necessarily requested are returned (for
+      instance querying HiddenServiceOptions gives HiddenServiceDir,
+      HiddenServicePort, etc)
+    
+    The vast majority of the options fall into the first two categories, in
+    which case calling get_conf() is sufficient. However, for batch queries or
+    the special options that give a set of values this provides back the full
+    response. As of tor version 0.2.1.25 HiddenServiceOptions was the only
+    option like this.
+    
+    The get_conf() and get_conf_map() functions both try to account for these
+    special mappings, so queried like get_conf("HiddenServicePort") should
+    behave as you'd expect.
     
     :param str,list param: GETCONF option(s) to be queried
     :param object default: response if the query fails
@@ -588,12 +610,11 @@ class Controller(BaseController):
     :returns:
       Response depends upon how we were called as follows...
       
-      * dict with param (str) => response mappings (str) if multiple was False
-      * dict with param (str) => response mappings (list) if multiple was True
-      * dict with param (str) => default mappings if a default value was provided and our call failed
+      * dict of 'config key => value' mappings, the value is a list if 'multiple' is True and otherwise is a str of just the first value
+      * default if one was provided and our call failed
     
     :raises:
-      :class:`stem.socket.ControllerError` if the call fails, and we weren't provided a default response
+      :class:`stem.socket.ControllerError` if the call fails and we weren't provided a default response
       :class:`stem.socket.InvalidArguments` if the configuration option requested was invalid
     """
     
@@ -601,14 +622,23 @@ class Controller(BaseController):
       param = [param]
     
     try:
-      if param == [""] or param == []:
-        raise stem.socket.InvalidRequest("Received empty parameter")
+      # remove strings which contain only whitespace
+      param = filter(lambda x: not re.match("^\s*$", x), param)
+      if param == []: return {}
       
       response = self.msg("GETCONF %s" % ' '.join(param))
       stem.response.convert("GETCONF", response)
       
-      requested_params = set(map(lambda x: x.lower(), param))
-      reply_params = set(map(lambda x: x.lower(), response.entries.keys()))
+      # map the entries back to the parameters given so the capitalization
+      # matches. Note: when the same configuration key is provided multiple
+      # times, this uses the first one and ignores the rest.
+      for key in response.entries:
+        if not key.lower() in self._mapped_config_keys:
+          response.entries[stem.util.control.case_insensitive_lookup(param, key)] = response.entries[key]
+          if key != stem.util.control.case_insensitive_lookup(param, key): del response.entries[key]
+      
+      requested_params = set(param)
+      reply_params = set(response.entries.keys())
       
       # if none of the requested parameters are context sensitive and if the
       # parameters received don't match the parameters requested
diff --git a/stem/response/getconf.py b/stem/response/getconf.py
index f2dc6b2..30b7f68 100644
--- a/stem/response/getconf.py
+++ b/stem/response/getconf.py
@@ -5,7 +5,10 @@ class GetConfResponse(stem.response.ControlMessage):
   """
   Reply for a GETCONF query.
   
-  :var dict entries: mapping between the queried options (string) and their values (list of strings)
+  Note that configuration parameters won't match what we queried for if it's one
+  of the special mapping options (ex. "HiddenServiceOptions").
+  
+  :var dict entries: mapping between the config parameter (string) and their values (list of strings)
   """
   
   def _parse_message(self):
@@ -42,7 +45,5 @@ class GetConfResponse(stem.response.ControlMessage):
       else:
         key, value = (line.pop(), None)
       
-      entry = self.entries.get(key, None)
-      
       self.entries.setdefault(key, []).append(value)
 
diff --git a/stem/socket.py b/stem/socket.py
index db2e451..62312a8 100644
--- a/stem/socket.py
+++ b/stem/socket.py
@@ -554,21 +554,21 @@ class InvalidRequest(ControllerError):
   """
   Base Exception class for invalid requests
   
-  :var str code: The error code returned by Tor (if applicable)
-  :var str message: The error message returned by Tor (if applicable) or a human
-    readable error message
+  :var str code: error code returned by Tor
+  :var str message: error message returned by Tor or a human readable error message
   """
   
   def __init__(self, code = None, message = None):
     """
     Initializes an InvalidRequest object.
     
-    :param str code: The error code returned by Tor
-    :param str message: The error message returned by Tor
+    :param str code: error code returned by Tor
+    :param str message: error message returned by Tor or a human readable error message
     
     :returns: object of InvalidRequest class
     """
     
+    super(InvalidRequest, self).__init__()
     self.code = code
     self.message = message
 
@@ -576,9 +576,8 @@ class InvalidArguments(InvalidRequest):
   """
   Exception class for invalid requests which contain invalid arguments.
   
-  :var str code: The error code returned by Tor (if applicable)
-  :var str message: The error message returned by Tor (if applicable) or a human
-    readable error message
+  :var str code: error code returned by Tor
+  :var str message: error message returned by Tor or a human readable error message
   :var list arguments: a list of arguments which were invalid
   """
   
@@ -586,16 +585,14 @@ class InvalidArguments(InvalidRequest):
     """
     Initializes an InvalidArguments object.
     
-    :param str code: The error code returned by Tor (if applicable)
-    :param str message: The error message returned by Tor (if applicable) or a
-      human readable error message
+    :param str code: error code returned by Tor
+    :param str message: error message returned by Tor or a human readable error message
     :param list arguments: a list of arguments which were invalid
     
     :returns: object of InvalidArguments class
     """
     
-    self.code = code
-    self.message = message
+    super(InvalidArguments, self).__init__(code, message)
     self.arguments = arguments
 
 class SocketError(ControllerError):
diff --git a/stem/util/__init__.py b/stem/util/__init__.py
index 5f6bb24..75a2da4 100644
--- a/stem/util/__init__.py
+++ b/stem/util/__init__.py
@@ -2,5 +2,5 @@
 Utility functions used by the stem library.
 """
 
-__all__ = ["conf", "connection", "enum", "log", "proc", "system", "term", "tor_tools"]
+__all__ = ["conf", "connection", "control", "enum", "log", "proc", "system", "term", "tor_tools"]
 
diff --git a/stem/util/control.py b/stem/util/control.py
new file mode 100644
index 0000000..f6b0da0
--- /dev/null
+++ b/stem/util/control.py
@@ -0,0 +1,26 @@
+"""
+Helper functions utilized by the controller classes.
+
+**Module Overview:**
+
+::
+  case_insensitive_lookup - does case insensitive lookups on python dictionaries
+"""
+
+def case_insensitive_lookup(lst, key, start=None, stop=None):
+  """
+  Returns the first value equal to key in lst while ignoring case.
+  
+  :param list lst: list of strings
+  :param str key: string to be looked up
+  :param int start: index from where the lookup should begin
+  :param int stop: index where the lookup ends
+  
+  :returns: case-insensitive equivalent of key in lst
+  
+  :raises: ValueError if such a key doesn't exist
+  """
+  
+  for i in lst[start:stop]:
+    if i.lower() == key.lower():
+      return i
diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py
index 329481b..a9eee1b 100644
--- a/test/integ/control/controller.py
+++ b/test/integ/control/controller.py
@@ -143,8 +143,7 @@ class TestController(unittest.TestCase):
     runner = test.runner.get_runner()
     
     with runner.get_tor_controller() as controller:
-      # successful single query
-      socket = runner.get_tor_socket()
+      socket = controller.get_socket()
       if isinstance(socket, stem.socket.ControlPort):
         connection_value = str(socket.get_port())
         config_key = "ControlPort"
@@ -152,17 +151,18 @@ class TestController(unittest.TestCase):
         connection_value = str(socket.get_socket_path())
         config_key = "ControlSocket"
       
+      # successful single query
       self.assertEqual(connection_value, controller.get_conf(config_key))
       self.assertEqual(connection_value, controller.get_conf(config_key, "la-di-dah"))
       
       # succeessful batch query
-      expected = {config_key: connection_value}
+      expected = {config_key: [connection_value]}
       self.assertEqual(expected, controller.get_conf_map([config_key]))
       self.assertEqual(expected, controller.get_conf_map([config_key], "la-di-dah"))
       
-      getconf_params = set(["ControlPort", "DirPort", "DataDirectory"])
-      self.assertEqual(getconf_params, set(controller.get_conf_map(["ControlPort",
-        "DirPort", "DataDirectory"])))
+      getconf_params = set(["ControlPORT", "dirport", "datadirectory"])
+      self.assertEqual(getconf_params, set(controller.get_conf_map(["ControlPORT",
+        "dirport", "datadirectory"], multiple=False)))
       
       # non-existant option(s)
       self.assertRaises(stem.socket.InvalidArguments, controller.get_conf, "blarg")
@@ -175,16 +175,19 @@ class TestController(unittest.TestCase):
           controller.get_conf_map(["erfusdj", "afiafj"], "la-di-dah", multiple = True))
       
       # multivalue configuration keys
-      nodefamilies = [node_family[11:].strip() for node_family in
-          runner.get_torrc_contents().split("\n") if node_family.startswith("NodeFamily ")]
-      self.assertEqual(nodefamilies, controller.get_conf("NodeFamily", multiple = True))
+      nodefamilies = [("abc", "xyz", "pqrs"), ("mno", "tuv", "wxyz")]
+      controller.msg("SETCONF %s" % " ".join(["nodefamily=\"" + ",".join(x) + "\"" for x in nodefamilies]))
+      self.assertEqual([",".join(n) for n in nodefamilies], controller.get_conf("nodefamily", multiple = True))
+      controller.msg("RESETCONF NodeFamily")
       
       # empty input
-      self.assertRaises(stem.socket.InvalidRequest, controller.get_conf, "")
-      self.assertRaises(stem.socket.InvalidRequest, controller.get_conf_map, [])
-      self.assertRaises(stem.socket.InvalidRequest, controller.get_conf_map, "")
+      self.assertEqual(None, controller.get_conf(""))
+      self.assertEqual({}, controller.get_conf_map([]))
+      self.assertEqual({}, controller.get_conf_map([""]))
+      self.assertEqual(None, controller.get_conf("          "))
+      self.assertEqual({}, controller.get_conf_map(["    ", "        "]))
       
       self.assertEqual("la-di-dah", controller.get_conf("", "la-di-dah"))
-      self.assertEqual({"": "la-di-dah"}, controller.get_conf_map("", "la-di-dah"))
+      self.assertEqual({}, controller.get_conf_map("", "la-di-dah"))
       self.assertEqual({}, controller.get_conf_map([], "la-di-dah"))
 
diff --git a/test/runner.py b/test/runner.py
index 571bb9d..3b696b3 100644
--- a/test/runner.py
+++ b/test/runner.py
@@ -71,8 +71,6 @@ BASE_TORRC = """# configuration for stem integration tests
 DataDirectory %s
 SocksPort 0
 DownloadExtraInfo 1
-NodeFamily dummystemnodexyz1x1,dummystemnodexyz1x2,dummystemnodexyz1x3
-NodeFamily dummystemnodexyz2x1,dummystemnodexyz2x2,dummystemnodexyz2x3
 """
 
 # We make some paths relative to stem's base directory (the one above us)





More information about the tor-commits mailing list