commit f9b370c455d4128aabe73702a7a445dfe17d4a2f Author: Damian Johnson <atagar@torproject.org> Date: Sat Jul 7 18:50:17 2012 -0700 Revisions for prior SETCONF and RESETCONF additions Several revisions to the prior changes, the most prominent of which is to use an API more similar to arm's. The SETCONF and RESETCONF methods are functionally identical except how they handle undefined values, so there's little reason to provide separate methods in our controller. Just using an optional 'reset' arg instead. One thing that arm got wrong (and so did stem) is that our method to change multiple options should accept a dict rather than a tuple list. Order *might* matter for the accursed HiddenService options, but for the 99.9% use case a tuple list is a very strange argument to accept for this. This also includes some testing fixes, for instance... * Testing when we have a list argument (for instance, setting our ExitPolicy). * Failing our test if one of the calls that should fail actually succeeds. * Reverting the configuration options that we change after the test. * An identical test for context sensitive options was added to both the getconf and setconf test. Definitely a good use case to check and it could belong with either setconf or getconf, but doing the same test twice is pointless. --- stem/control.py | 114 +++++++++++++---------------- stem/response/__init__.py | 13 ++-- test/integ/control/controller.py | 150 +++++++++++++++----------------------- test/unit/response/singleline.py | 4 +- 4 files changed, 119 insertions(+), 162 deletions(-) diff --git a/stem/control.py b/stem/control.py index 7eca284..f377cd3 100644 --- a/stem/control.py +++ b/stem/control.py @@ -581,9 +581,7 @@ class Controller(BaseController): return default if default != UNDEFINED else None entries = self.get_conf_map(param, default, multiple) - - if entries == default: return default - else: return _case_insensitive_lookup(entries, param) + return _case_insensitive_lookup(entries, param, default) def get_conf_map(self, param, default = UNDEFINED, multiple = True): """ @@ -666,46 +664,22 @@ class Controller(BaseController): if default != UNDEFINED: return default else: raise exc - def _set_conf(self, params, command="SETCONF"): - controlitems = [command] - - for key, value in params: - if type(value) in (list, tuple): - controlitems.extend(["%s=\"%s\"" % (key, val.strip()) for val in value]) - elif value: - controlitems.append("%s=\"%s\"" % (key, value.strip())) - else: - controlitems.append(key) - - response = self.msg(" ".join(controlitems)) - stem.response.convert("SINGLELINE", response) - - if response.is_ok(): - return True - elif response.code == "552": - if response.message.startswith("Unrecognized option: Unknown option '"): - key = response.message[37:response.message.find("\'", 37)] - raise stem.socket.InvalidArguments(response.code, response.message, [key]) - raise stem.socket.InvalidRequest(response.code, response.message) - elif response.code in ("513", "553"): - raise stem.socket.InvalidRequest(response.code, response.message) - else: - raise stem.socket.ProtocolError("%s returned unexpected status code" % command) - - def set_conf(self, *params): + def set_option(self, param, value, reset = False): """ - Changes the configuration of one or more configuration variables using the - control socket. + Changes the value of a tor configuration option via either a SETCONF or + RESETCONF query. Both behave identically unless our value is None, in which + case SETCONF sets the value to 0 or NULL, and RESETCONF returns it to its + default value. - :param list,dict params: Can be one of... + Our value can be any of the following... - * a list containing tuples of configuration keys (string) and their corresponding values (string or list of strings) - * a dict containing config key-value pairs + * a string to set a single value + * a list of strings to set a series of values (for instance the ExitPolicy) + * None to either set the value to 0/NULL or reset it to its default - Or - - :param str key: configuration key - :param str value: configuration value + :param str param: configuration option to be set + :param str,list value: value to set the parameter to + :param bool reset: issues a SETCONF if False, and RESETCONF if True :raises: :class:`stem.socket.ControllerError` if the call fails @@ -713,28 +687,24 @@ class Controller(BaseController): :class:`stem.socket.InvalidRequest` if the configuration setting is impossible or if there's a syntax error in the configuration values """ - if len(params) == 2: - arg = [params] - elif len(params) == 1: - if type(params[0]) == dict: - arg = params[0].items() - else: - arg = params[0] - else: - raise TypeError("set_conf expected 1 or 2 arguments, got %d", len(args)) - - self._set_conf(arg, "SETCONF") + self.set_options({param: value}, reset) - def reset_conf(self, params): + def set_options(self, params, reset = False): """ - Resets the configuration of one or more configuration variables using the - control socket. + Changes multiple tor configurations, in a similar fashion to + :func:`stem.control.Controller.set_option`. For example... + + :: - :param str,list,dict params: Can be one of... + my_controller.set_options({ + "Nickname", "caerSidi", + "ExitPolicy": ["accept *:80", "accept *:443", "reject *:*"], + "ContactInfo": "caerSidi-exit@someplace.com", + "Log": None, + }) - * a single configuration key (string) - * a list of configuration keys (list of strings) to be reset and/or tuples of configuration keys (string) and their corresponding values (string or list of strings) - * a dict containing configuration key/value pairs. + :param dict params: mapping of configuration options to the values we're setting it to + :param bool reset: issues a SETCONF if False, and RESETCONF if True :raises: :class:`stem.socket.ControllerError` if the call fails @@ -742,14 +712,30 @@ class Controller(BaseController): :class:`stem.socket.InvalidRequest` if the configuration setting is impossible or if there's a syntax error in the configuration values """ - if type(params) == str: - arg = [(params, None)] - elif type(params) == dict: - arg = params.items() - else: - arg = map(lambda item: (item, None) if type(item) == str else item, params) + # constructs the SETCONF or RESETCONF query + query_comp = ["RESETCONF" if reset else "SETCONF"] - self._set_conf(arg, "RESETCONF") + for param, value in params.items(): + if isinstance(value, str): + query_comp.append("%s=\"%s\"" % (param, value.strip())) + elif value: + query_comp.extend(["%s=\"%s\"" % (param, val.strip()) for val in value]) + else: + query_comp.append(param) + + response = self.msg(" ".join(query_comp)) + stem.response.convert("SINGLELINE", response) + + if not response.is_ok(): + if response.code == "552": + if response.message.startswith("Unrecognized option: Unknown option '"): + key = response.message[37:response.message.find("\'", 37)] + raise stem.socket.InvalidArguments(response.code, response.message, [key]) + raise stem.socket.InvalidRequest(response.code, response.message) + elif response.code in ("513", "553"): + raise stem.socket.InvalidRequest(response.code, response.message) + else: + raise stem.socket.ProtocolError("%s returned unexpected status code" % command) def _case_insensitive_lookup(entries, key, default = UNDEFINED): """ diff --git a/stem/response/__init__.py b/stem/response/__init__.py index 2ae0b82..1d96988 100644 --- a/stem/response/__init__.py +++ b/stem/response/__init__.py @@ -421,11 +421,12 @@ def _get_quote_indeces(line, escaped): class SingleLineResponse(ControlMessage): """ - A reply that contains only a single line + Reply to a request that performs an action rather than querying data. These + requests only contain a single line, which is 'OK' if successful, and a + description of the problem if not. - :var dict entries: - mapping between the queried options (string) and their values (string/list - of strings) + :var str code: status code for our line + :var str message: content of the line """ def is_ok(self, strict = False): @@ -433,7 +434,7 @@ class SingleLineResponse(ControlMessage): Checks if the response code is "250". If strict is True, checks if the response is "250 OK" - :param bool strict: If True, check if the message is "250 OK" + :param bool strict: checks for a "250 OK" message if True :returns: * If strict is False: True if the response code is "250", False otherwise @@ -452,5 +453,5 @@ class SingleLineResponse(ControlMessage): elif len(content) == 0: raise stem.socket.ProtocolError("Received empty response") else: - self.code, self.delimiter, self.message = content[0] + self.code, _, self.message = content[0] diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py index eb2386c..b7eea7e 100644 --- a/test/integ/control/controller.py +++ b/test/integ/control/controller.py @@ -193,104 +193,72 @@ class TestController(unittest.TestCase): self.assertEqual("la-di-dah", controller.get_conf("", "la-di-dah")) self.assertEqual({}, controller.get_conf_map("", "la-di-dah")) self.assertEqual({}, controller.get_conf_map([], "la-di-dah")) - - # context-sensitive keys - tmpdir = tempfile.mkdtemp() - keys = [ - ("HiddenServiceDir", tmpdir), - ("HiddenServicePort", "17234 127.0.0.1:17235") - ] - controller.set_conf(keys) - self.assertEqual(tmpdir, controller.get_conf("HiddenServiceDir")) - self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("HiddenServicePort")) - shutil.rmtree(tmpdir) - def test_setconf(self): + def test_set_options(self): """ - Exercises Controller.set_conf with valid and invalid requests. + Exercises set_option() and set_options() with valid and invalid requests. """ - runner = test.runner.get_runner() - - with runner.get_tor_controller() as controller: - # Single key, valid and invalid - connlimit = int(controller.get_conf("ConnLimit")) - controller.set_conf("connlimit", str(connlimit - 1)) - self.assertEqual(connlimit - 1, int(controller.get_conf("ConnLimit"))) - try: - controller.set_conf("invalidkeyboo", "abcde") - except stem.socket.InvalidArguments, exc: - self.assertEqual(["invalidkeyboo"], exc.arguments) - - settings = { - "connlimit": str(connlimit - 2), - "contactinfo": "stem@testing" - } - controller.set_conf(settings) - self.assertEqual(connlimit - 2, int(controller.get_conf("ConnLimit"))) - self.assertEqual("stem@testing", controller.get_conf("contactinfo")) - - settings["bombay"] = "vadapav" - try: - controller.set_conf(settings) - except stem.socket.InvalidArguments, exc: - self.assertEqual(["bombay"], exc.arguments) - - tmpdir = tempfile.mkdtemp() - settings = [ - ("HiddenServiceDir", tmpdir), - ("HiddenServicePort", "17234 127.0.0.1:17235") - ] - controller.set_conf(settings) - self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("hiddenserviceport")) - self.assertEqual(tmpdir, controller.get_conf("hiddenservicedir")) - shutil.rmtree(tmpdir) - - def test_resetconf(self): - """ - Exercises Controller.reset_conf with valid and invalid requests. - """ + if test.runner.require_control(self): return runner = test.runner.get_runner() + tmpdir = tempfile.mkdtemp() with runner.get_tor_controller() as controller: - # Single valid key - controller.set_conf("contactinfo", "stem@testing") - self.assertEqual("stem@testing", controller.get_conf("contactinfo")) - controller.reset_conf("contactinfo") - self.assertEqual(None, controller.get_conf("contactinfo")) - - # Invalid key try: - controller.reset_conf(("invalidkeyboo", "abcde")) - except stem.socket.InvalidArguments, exc: - self.assertEqual(["invalidkeyboo"], exc.arguments) - - # Multiple keys, list & dict - settings = { - "connlimit": "314", - "contactinfo": "stem@testing" - } - controller.reset_conf(settings) - self.assertEqual("314", controller.get_conf("ConnLimit")) - self.assertEqual("stem@testing", controller.get_conf("contactinfo")) - - settings = [ - ("connlimit", "786"), - ("contactinfo", "stem testing") - ] - controller.reset_conf(settings) - self.assertEqual("786", controller.get_conf("ConnLimit")) - self.assertEqual("stem testing", controller.get_conf("contactinfo")) - - # context-sensitive keys - tmpdir = tempfile.mkdtemp() - settings = [ - ("HiddenServiceDir", tmpdir), - ("HiddenServicePort", "17234 127.0.0.1:17235") - ] - controller.reset_conf(settings) - self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("hiddenserviceport")) - self.assertEqual(tmpdir, controller.get_conf("hiddenservicedir")) - shutil.rmtree(tmpdir) + # successfully set a single option + connlimit = int(controller.get_conf("ConnLimit")) + controller.set_option("connlimit", str(connlimit - 1)) + self.assertEqual(connlimit - 1, int(controller.get_conf("ConnLimit"))) + + # successfully set a single list option + exit_policy = ["accept *:7777", "reject *:*"] + controller.set_option("ExitPolicy", exit_policy) + self.assertEqual(exit_policy, controller.get_conf("ExitPolicy", multiple = True)) + + # fail to set a single option + try: + controller.set_option("invalidkeyboo", "abcde") + self.fail() + except stem.socket.InvalidArguments, exc: + self.assertEqual(["invalidkeyboo"], exc.arguments) + + # successfully sets multiple config options + controller.set_options({ + "connlimit": str(connlimit - 2), + "contactinfo": "stem@testing", + }) + + self.assertEqual(connlimit - 2, int(controller.get_conf("ConnLimit"))) + self.assertEqual("stem@testing", controller.get_conf("contactinfo")) + + # fail to set multiple config options + try: + controller.set_options({ + "contactinfo": "stem@testing", + "bombay": "vadapav", + }) + self.fail() + except stem.socket.InvalidArguments, exc: + self.assertEqual(["bombay"], exc.arguments) + + # context-sensitive keys + controller.set_options({ + "HiddenServiceDir": tmpdir, + "HiddenServicePort": "17234 127.0.0.1:17235", + }) + + self.assertEqual(tmpdir, controller.get_conf("HiddenServiceDir")) + self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("HiddenServicePort")) + finally: + # reverts configuration changes + controller.set_options({ + "ExitPolicy": "reject *:*", + "ConnLimit": None, + "ContactInfo": None, + "HiddenServiceDir": None, + "HiddenServicePort": None, + }, reset = True) + + shutil.rmtree(tmpdir) diff --git a/test/unit/response/singleline.py b/test/unit/response/singleline.py index 8781bf9..aa9c915 100644 --- a/test/unit/response/singleline.py +++ b/test/unit/response/singleline.py @@ -1,5 +1,5 @@ """ -Unit tests for the stem.response.getconf.GetConfResponse class. +Unit tests for the stem.response.SingleLineResponse class. """ import unittest @@ -16,6 +16,7 @@ class TestSingleLineResponse(unittest.TestCase): message = mocking.get_message("552 NOTOK") stem.response.convert("SINGLELINE", message) self.assertEqual(False, message.is_ok()) + message = mocking.get_message("250 KK") stem.response.convert("SINGLELINE", message) self.assertEqual(True, message.is_ok()) @@ -23,6 +24,7 @@ class TestSingleLineResponse(unittest.TestCase): message = mocking.get_message("250 OK") stem.response.convert("SINGLELINE", message) self.assertEqual(True, message.is_ok(True)) + message = mocking.get_message("250 HMM") stem.response.convert("SINGLELINE", message) self.assertEqual(False, message.is_ok(True))