commit 79fc8cd7514cda072abf855b6a1908f0dd10bb17 Author: Damian Johnson <atagar@torproject.org> Date: Sat Dec 29 23:27:06 2012 -0800 Revisions for prior mocking and controller changes Changes include... * Rewrite of get_socks_ports() with the following changes... * Renaming to get_socks_listeners() since it provides (address, port) tuples, not just ports. * Better error handling. Previously malformed content such as invalid ports would cause us to raise ValueErrors. * The method wasn't documented as raising any exceptions, but if the get_conf() calls fail then a ControllerError will be raised. * Minor corrections to the return_for_args() examples. * Renamed return_for_args()'s "method" arg to "is_method" (it's nice when the an arg name can imply the type). * I'm not sure what was meant by "Also, remove the the space in the join so that there is only one parameter passed to the string substitution.". Personally I'd find a message like... "Unrecognized argument sent for return_for_args(). Got 'foo, bar' but we only recognize 'foo, blarg'." ... to be a bit nicer than... "Unrecognized argument sent for return_for_args(). Got 'foo, bar' but we only recognize 'foo;blarg'." --- stem/control.py | 53 ++++++++++++++++++--------- test/integ/control/controller.py | 2 +- test/mocking.py | 68 +++++++++++++++++------------------ test/unit/control/controller.py | 72 +++++++++++++++++++++++++++++-------- test/unit/tutorial.py | 2 +- 5 files changed, 126 insertions(+), 71 deletions(-) diff --git a/stem/control.py b/stem/control.py index 8a36285..704bb5e 100644 --- a/stem/control.py +++ b/stem/control.py @@ -26,6 +26,7 @@ providing its own for interacting at a higher level. |- set_options - sets or resets the values of multiple configuration options |- load_conf - loads configuration information as if it was in the torrc |- save_conf - saves configuration information to the torrc + |- get_socks_listeners - provides (address, port) where tor is listening for SOCKS connections |- is_feature_enabled - checks if a given controller feature is enabled |- enable_feature - enables a controller feature that has been disabled by default |- signal - sends a signal to the tor client @@ -1320,32 +1321,48 @@ class Controller(BaseController): else: raise stem.ProtocolError("SAVECONF returned unexpected response code") - def get_socks_ports(self): + def get_socks_listeners(self): """ - Returns a list of SOCKS proxy ports open on the controlled tor instance. + Provides the SOCKS **(address, port)** tuples that tor has open. - :returns: list of **(host, port)** tuples or an empty list if there are no - SOCKS listeners + :returns: list of **(address, port)** tuples for the available SOCKS + listeners + + :raises: :class:`stem.ControllerError` if unable to determine the listeners """ + proxy_addrs = [] + try: - raw_addrs = self.get_info("net/listeners/socks").split() - # remove the surrounding quotes from each listener - raw_addrs = [x.replace("\"", "") for x in raw_addrs] + for listener in self.get_info("net/listeners/socks").split(): + if not (listener.startswith('"') and listener.endswith('"')): + raise stem.ProtocolError("'GETINFO net/listeners/socks' responses are expected to be quoted: %s" % listener) + elif not ':' in listener: + raise stem.ProtocolError("'GETINFO net/listeners/socks' had a listener without a colon: %s" % listener) + + listener = listener[1:-1] # strip quotes + addr, port = listener.split(':') + proxy_addrs.append((addr, port)) except stem.InvalidArguments: - # tor version is old (pre-tor-0.2.2.26-beta); use get_conf() + # tor version is old (pre-tor-0.2.2.26-beta), use get_conf() instead socks_port = self.get_conf('SocksPort') - raw_addrs = [] + for listener in self.get_conf('SocksListenAddress', multiple = True): - if listener.count(':') == 0: - listener = listener + ":" + socks_port - raw_addrs.append(listener) - # both processes above give a list of strings of the form host:port - proxy_addrs = [] - for proxy in raw_addrs: - proxy_pair = proxy.split(":") - proxy_addrs.append(tuple((proxy_pair[0], int(proxy_pair[1])))) - return proxy_addrs + if ':' in listener: + addr, port = listener.split(':') + proxy_addrs.append((addr, port)) + else: + proxy_addrs.append((listener, socks_port)) + + # validate that address/ports are valid, and convert ports to ints + + for addr, port in proxy_addrs: + if not stem.util.connection.is_valid_ip_address(addr): + raise stem.ProtocolError("Invalid address for a SOCKS listener: %s" % addr) + elif not stem.util.connection.is_valid_port(port): + raise stem.ProtocolError("Invalid port for a SOCKS listener: %s" % port) + + return [(addr, int(port)) for (addr, port) in proxy_addrs] def is_feature_enabled(self, feature): """ diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py index 5c146e0..86056aa 100644 --- a/test/integ/control/controller.py +++ b/test/integ/control/controller.py @@ -467,7 +467,7 @@ class TestController(unittest.TestCase): runner = test.runner.get_runner() with runner.get_tor_controller() as controller: - self.assertEqual([('127.0.0.1', 1112)], controller.get_socks_ports()) + self.assertEqual([('127.0.0.1', 1112)], controller.get_socks_listeners()) def test_enable_feature(self): """ diff --git a/test/mocking.py b/test/mocking.py index 074c977..a86b958 100644 --- a/test/mocking.py +++ b/test/mocking.py @@ -204,63 +204,61 @@ def return_true(): return return_value(True) def return_false(): return return_value(False) def return_none(): return return_value(None) -def return_for_args(args_to_return_value, default = None, method = False): +def return_for_args(args_to_return_value, default = None, is_method = False): """ Returns a value if the arguments to it match something in a given 'argument => return value' mapping. Otherwise, a default function is called with the arguments. - The mapped argument is a tuple (not a list) of parameters to a function (or - method). Positional arguments must be in the order used to call the mocked - function and keyword arguments must be strings of the form 'k=v' in - alphabetical order. Some examples: + The mapped argument is a tuple (not a list) of parameters to a function or + method. Positional arguments must be in the order used to call the mocked + function, and keyword arguments must be strings of the form 'k=v'. Keyword + arguments **must** appear in alphabetical order. For example... :: - Mocked functions can return different types depending on input: + mocking.mock("get_answer", mocking.return_for_args({ + ("breakfast_menu",): "spam", + ("lunch_menu",): "eggs and spam", + (42,): ["life", "universe", "everything"], + })) - mocking.mock("get_answer", mocking.return_for_args({ - ("breakfast_menu",): "spam", - ("lunch_menu",): "eggs and spam", - (42,): ["life", "universe", "everything"], - }) - - mocking.mock("align_text", { - ("Stem", "alignment=left", "size=10"): "Stem ", - ("Stem", "alignment=center", "size=10"): " Stem ", - ("Stem", "alignment=right", "size=10"): " Stem", - }) - - The mocked method returns one of three circuit ids depending on the input: - - :: + mocking.mock("align_text", mocking.return_for_args({ + ("Stem", "alignment=left", "size=10"): "Stem ", + ("Stem", "alignment=center", "size=10"): " Stem ", + ("Stem", "alignment=right", "size=10"): " Stem", + })) - mocking.mock_method(Controller, "new_circuit", mocking.return_for_args({ - (): "1", - ("path=['718BCEA286B531757ACAFF93AE04910EA73DE617', " + \ - "'30BAB8EE7606CBD12F3CC269AE976E0153E7A58D', " + \ - "'2765D8A8C4BBA3F89585A9FFE0E8575615880BEB']",): "2" - ("path=['1A', '2B', '3C']", "purpose=controller"): "3" - }, method = True) + mocking.mock_method(Controller, "new_circuit", mocking.return_for_args({ + (): "1", + ("path=['718BCEA286B531757ACAFF93AE04910EA73DE617', " + \ + "'30BAB8EE7606CBD12F3CC269AE976E0153E7A58D', " + \ + "'2765D8A8C4BBA3F89585A9FFE0E8575615880BEB']",): "2" + ("path=['1A', '2B', '3C']", "purpose=controller"): "3" + }, is_method = True)) - :param dict,tuple args_to_return_value: mapping of arguments to the value we should provide - :param functor default: returns the value of this function if the args don't match something that we have, we raise a ValueError by default - :param bool method: removes the 'self' reference before processing the remainder of the parameters + :param dict args_to_return_value: mapping of arguments to the value we should provide + :param functor default: returns the value of this function if the args don't + match something that we have, we raise a ValueError by default + :param bool is_method: handles this like a method, removing the 'self' + reference """ def _return_value(*args, **kwargs): - # strip off the 'self' for mock classes - if args and method: + # strip off the 'self' if we're mocking a method + if args and is_method: args = args[1:] if len(args) > 2 else [args[1]] if kwargs: - args.extend(['='.join((str(k),str(kwargs[k]))) for k in sorted(kwargs.keys())]) + args.extend(["%s=%s" % (k, kwargs[k]) for k in sorted(kwargs.keys())]) + args = tuple(args) + if args in args_to_return_value: return args_to_return_value[args] elif default is None: arg_label = ", ".join([str(v) for v in args]) - arg_keys = ";".join([str(v) for v in args_to_return_value.keys()]) + arg_keys = ", ".join([str(v) for v in args_to_return_value.keys()]) raise ValueError("Unrecognized argument sent for return_for_args(). Got '%s' but we only recognize '%s'." % (arg_label, arg_keys)) else: return default(args) diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index f3849eb..695c2de 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -77,54 +77,94 @@ class TestControl(unittest.TestCase): # EventType.SIGNAL was added in tor version 0.2.3.1-alpha self.assertRaises(InvalidRequest, self.controller.add_event_listener, mocking.no_op(), EventType.SIGNAL) - def test_socks_port_old_tor(self): + def test_get_socks_listeners_old(self): """ - Exercises the get_socks_ports method as if talking to an old tor process. + Exercises the get_socks_listeners() method as though talking to an old tor + instance. """ # An old tor raises stem.InvalidArguments for get_info about socks, but - # get_socks_ports returns the socks information, anyway. + # get_socks_listeners should work anyway. + mocking.mock_method(Controller, "get_info", mocking.raise_exception(InvalidArguments)) + mocking.mock_method(Controller, "get_conf", mocking.return_for_args({ ("SocksPort",): "9050", ("SocksListenAddress", "multiple=True"): ["127.0.0.1"] - }, method = True)) - self.assertEqual([('127.0.0.1', 9050)], self.controller.get_socks_ports()) + }, is_method = True)) + self.assertEqual([('127.0.0.1', 9050)], self.controller.get_socks_listeners()) # Again, an old tor, but SocksListenAddress overrides the port number. + mocking.mock_method(Controller, "get_conf", mocking.return_for_args({ ("SocksPort",): "9050", ("SocksListenAddress", "multiple=True"): ["127.0.0.1:1112"] - }, method = True)) - self.assertEqual([('127.0.0.1', 1112)], self.controller.get_socks_ports()) + }, is_method = True)) + self.assertEqual([('127.0.0.1', 1112)], self.controller.get_socks_listeners()) # Again, an old tor, but multiple listeners + mocking.mock_method(Controller, "get_conf", mocking.return_for_args({ ("SocksPort",): "9050", ("SocksListenAddress", "multiple=True"): ["127.0.0.1:1112", "127.0.0.1:1114"] - }, method = True)) - self.assertEqual([('127.0.0.1', 1112), ('127.0.0.1', 1114)], self.controller.get_socks_ports()) + }, is_method = True)) + self.assertEqual([('127.0.0.1', 1112), ('127.0.0.1', 1114)], self.controller.get_socks_listeners()) # Again, an old tor, but no SOCKS listeners + mocking.mock_method(Controller, "get_conf", mocking.return_for_args({ ("SocksPort",): "0", ("SocksListenAddress", "multiple=True"): [] - }, method = True)) - self.assertEqual([], self.controller.get_socks_ports()) + }, is_method = True)) + self.assertEqual([], self.controller.get_socks_listeners()) + + # Where tor provides invalid ports or addresses + + mocking.mock_method(Controller, "get_conf", mocking.return_for_args({ + ("SocksPort",): "blarg", + ("SocksListenAddress", "multiple=True"): ["127.0.0.1"] + }, is_method = True)) + self.assertRaises(stem.ProtocolError, self.controller.get_socks_listeners) + + mocking.mock_method(Controller, "get_conf", mocking.return_for_args({ + ("SocksPort",): "0", + ("SocksListenAddress", "multiple=True"): ["127.0.0.1:abc"] + }, is_method = True)) + self.assertRaises(stem.ProtocolError, self.controller.get_socks_listeners) + + mocking.mock_method(Controller, "get_conf", mocking.return_for_args({ + ("SocksPort",): "40", + ("SocksListenAddress", "multiple=True"): ["500.0.0.1"] + }, is_method = True)) + self.assertRaises(stem.ProtocolError, self.controller.get_socks_listeners) - def test_socks_port_new_tor(self): + def test_get_socks_listeners_new(self): """ - Exercises the get_socks_ports method as if talking to a newer tor process. + Exercises the get_socks_listeners() method as if talking to a newer tor + instance. """ # multiple SOCKS listeners mocking.mock_method(Controller, "get_info", mocking.return_value( - "\"127.0.0.1:1112\" \"127.0.0.1:1114\"" + '"127.0.0.1:1112" "127.0.0.1:1114"' )) self.assertEqual([('127.0.0.1', 1112), ('127.0.0.1', 1114)], - self.controller.get_socks_ports()) + self.controller.get_socks_listeners()) # no SOCKS listeners mocking.mock_method(Controller, "get_info", mocking.return_value("")) - self.assertEqual([], self.controller.get_socks_ports()) + self.assertEqual([], self.controller.get_socks_listeners()) + + # check where GETINFO provides malformed content + + invalid_responses = ( + '"127.0.0.1"', # address only + '"1112"', # port only + '"5127.0.0.1:1112"', # invlaid address + '"127.0.0.1:991112"', # invalid port + ) + + for response in invalid_responses: + mocking.mock_method(Controller, "get_info", mocking.return_value(response)) + self.assertRaises(stem.ProtocolError, self.controller.get_socks_listeners) diff --git a/test/unit/tutorial.py b/test/unit/tutorial.py index b586549..79baa8c 100644 --- a/test/unit/tutorial.py +++ b/test/unit/tutorial.py @@ -21,7 +21,7 @@ class TestTutorial(unittest.TestCase): 'get_info': mocking.return_for_args({ ('traffic/read',): '1234', ('traffic/written',): '5678', - }, method = True), + }, is_method = True), }) controller.authenticate()