commit 765c497f824fde380d8d1b864e3dec36fcce3b11 Author: Ravi Chandra Padmala <neenaoffline@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)