commit ffdd61ea41ca844047ac57725639a7cf0e22d41d Author: Damian Johnson atagar@torproject.org Date: Sun Dec 30 16:03:30 2012 -0800

Uniformly providing a default argument for getters

As disussed on 'https://trac.torproject.org/7832' it's important from a usability standpoint for all of our getter methods to support a 'default' argument. It's also nice to have class-wide uniformity on this since it makes the Controller class more intuitive to use.

This also reorganizes the method listing in the header to group related methods, and has a couple small bits of backward incompatability (trying to get these breakages in early before making our initial release announcement)...

* renamed protocolinfo() to get_protocolinfo() to match the other getters * renamed one of the 'circuit' arguments to 'circuit_id' (we did that for all of the methods but one) --- stem/control.py | 300 +++++++++++++++++++++++-------------- test/integ/control/controller.py | 2 +- 2 files changed, 187 insertions(+), 115 deletions(-)

diff --git a/stem/control.py b/stem/control.py index 641b647..fdcf332 100644 --- a/stem/control.py +++ b/stem/control.py @@ -13,39 +13,50 @@ providing its own for interacting at a higher level. | |- from_port - Provides a Controller based on a port connection. | +- from_socket_file - Provides a Controller based on a socket file connection. | - |- add_event_listener - attaches an event listener to be notified of tor events - |- remove_event_listener - removes a listener so it isn't notified of further events - |- is_caching_enabled - true if the controller has enabled caching - |- is_geoip_unavailable - true if we've discovered our geoip db to be unavailable - |- clear_cache - clears any cached results + |- authenticate - authenticates this controller with tor + | + |- get_version - convenience method to get tor version + |- get_socks_listeners - provides where tor is listening for SOCKS connections + |- get_protocolinfo - information about the controller interface + | |- get_info - issues a GETINFO query for a parameter |- get_conf - gets the value of a configuration option - |- get_conf_mapping - gets the values of multiple configuration options + |- get_conf_map - gets the values of multiple configuration options + | + |- get_server_descriptor - querying the server descriptor for a relay + |- get_server_descriptors - provides all presently available server descriptors + |- get_network_status - querying the router status entry for a relay + |- get_network_statuses - provides all preently available router status entries + | |- set_conf - sets the value of a configuration option |- reset_conf - reverts configuration options to their default values |- set_options - sets or resets the values of multiple configuration options + | + |- add_event_listener - attaches an event listener to be notified of tor events + |- remove_event_listener - removes a listener so it isn't notified of further events + | |- 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_caching_enabled - true if the controller has enabled caching + |- clear_cache - clears any cached results + | |- 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 + | + |- get_circuit - provides an active circuit + |- get_circuits - provides a list of active circuits |- new_circuit - create new circuits |- extend_circuit - create new circuits and extend existing ones - |- close_circuit - close a circuit - |- close_stream - close a stream |- repurpose_circuit - change a circuit's purpose - |- map_address - maps one address to another such that connections to the original are replaced with the other - |- get_circuit - provides an active circuit - |- get_circuits - provides a list of active circuits + |- close_circuit - close a circuit + | |- attach_stream - attach a stream to a circuit - |- get_version - convenience method to get tor version - |- get_server_descriptor - querying the server descriptor for a relay - |- get_server_descriptors - provides all presently available server descriptors - |- get_network_status - querying the router status entry for a relay - |- get_network_statuses - provides all preently available router status entries - |- authenticate - convenience method to authenticate the controller - +- protocolinfo - convenience method to get the protocol info + |- close_stream - close a stream + | + |- signal - sends a signal to the tor client + |- is_geoip_unavailable - true if we've discovered our geoip db to be unavailable + +- map_address - maps one address to another such that connections to the original are replaced with the other

BaseController - Base controller class asynchronous message handling |- msg - communicates with the tor process @@ -850,34 +861,43 @@ class Controller(BaseController): if default == UNDEFINED: raise exc else: return default

- def get_version(self): + def get_version(self, default = UNDEFINED): """ A convenience method to get tor version that current controller is connected to.

+ :param object default: response if the query fails + :returns: :class:`~stem.version.Version` of the tor instance that we're connected to

:raises: * :class:`stem.ControllerError` if unable to query the version * **ValueError** if unable to parse the version + + An exception is only raised if we weren't provided a default response. """

- if not self.is_caching_enabled(): - return stem.version.Version(self.get_info("version")) - elif not "version" in self._request_cache: - version = stem.version.Version(self.get_info("version")) - self._request_cache["version"] = version - - return self._request_cache["version"] + try: + if not self.is_caching_enabled(): + return stem.version.Version(self.get_info("version")) + elif not "version" in self._request_cache: + version = stem.version.Version(self.get_info("version")) + self._request_cache["version"] = version + + return self._request_cache["version"] + except Exception, exc: + if default == UNDEFINED: raise exc + else: return default

- def get_server_descriptor(self, relay): + def get_server_descriptor(self, relay, default = UNDEFINED): """ Provides the server descriptor for the relay with the given fingerprint or nickname. If the relay identifier could be either a fingerprint *or* nickname then it's queried as a fingerprint.

:param str relay: fingerprint or nickname of the relay to be queried + :param object default: response if the query fails

:returns: :class:`~stem.descriptor.server_descriptor.RelayDescriptor` for the given relay

@@ -885,45 +905,62 @@ class Controller(BaseController): * :class:`stem.ControllerError` if unable to query the descriptor * **ValueError** if **relay** doesn't conform with the pattern for being a fingerprint or nickname + + An exception is only raised if we weren't provided a default response. """

- if stem.util.tor_tools.is_valid_fingerprint(relay): - query = "desc/id/%s" % relay - elif stem.util.tor_tools.is_valid_nickname(relay): - query = "desc/name/%s" % relay - else: - raise ValueError("'%s' isn't a valid fingerprint or nickname" % relay) - - desc_content = self.get_info(query) - return stem.descriptor.server_descriptor.RelayDescriptor(desc_content) + try: + if stem.util.tor_tools.is_valid_fingerprint(relay): + query = "desc/id/%s" % relay + elif stem.util.tor_tools.is_valid_nickname(relay): + query = "desc/name/%s" % relay + else: + raise ValueError("'%s' isn't a valid fingerprint or nickname" % relay) + + desc_content = self.get_info(query) + return stem.descriptor.server_descriptor.RelayDescriptor(desc_content) + except Exception, exc: + if default == UNDEFINED: raise exc + else: return default

- def get_server_descriptors(self): + def get_server_descriptors(self, default = UNDEFINED): """ Provides an iterator for all of the server descriptors that tor presently knows about.

+ :param list default: items to provide if the query fails + :returns: iterates over :class:`~stem.descriptor.server_descriptor.RelayDescriptor` for relays in the tor network

- :raises: :class:`stem.ControllerError` if unable to query tor + :raises: :class:`stem.ControllerError` if unable to query tor and no + default was provided """

- # TODO: We should iterate over the descriptors as they're read from the - # socket rather than reading the whole thing into memory. - - desc_content = self.get_info("desc/all-recent") - - for desc in stem.descriptor.server_descriptor.parse_file(StringIO.StringIO(desc_content)): - yield desc + try: + # TODO: We should iterate over the descriptors as they're read from the + # socket rather than reading the whole thing into memory. + + desc_content = self.get_info("desc/all-recent") + + for desc in stem.descriptor.server_descriptor.parse_file(StringIO.StringIO(desc_content)): + yield desc + except Exception, exc: + if default == UNDEFINED: raise exc + else: + if entry is not None: + for entry in default: + yield entry

- def get_network_status(self, relay): + def get_network_status(self, relay, default = UNDEFINED): """ Provides the router status entry for the relay with the given fingerprint or nickname. If the relay identifier could be either a fingerprint *or* nickname then it's queried as a fingerprint.

:param str relay: fingerprint or nickname of the relay to be queried + :param object default: response if the query fails

:returns: :class:`~stem.descriptor.router_status_entry.RouterStatusEntryV2` for the given relay @@ -932,43 +969,59 @@ class Controller(BaseController): * :class:`stem.ControllerError` if unable to query the descriptor * **ValueError** if **relay** doesn't conform with the patter for being a fingerprint or nickname + + An exception is only raised if we weren't provided a default response. """

- if stem.util.tor_tools.is_valid_fingerprint(relay): - query = "ns/id/%s" % relay - elif stem.util.tor_tools.is_valid_nickname(relay): - query = "ns/name/%s" % relay - else: - raise ValueError("'%s' isn't a valid fingerprint or nickname" % relay) - - desc_content = self.get_info(query) - return stem.descriptor.router_status_entry.RouterStatusEntryV2(desc_content) + try: + if stem.util.tor_tools.is_valid_fingerprint(relay): + query = "ns/id/%s" % relay + elif stem.util.tor_tools.is_valid_nickname(relay): + query = "ns/name/%s" % relay + else: + raise ValueError("'%s' isn't a valid fingerprint or nickname" % relay) + + desc_content = self.get_info(query) + return stem.descriptor.router_status_entry.RouterStatusEntryV2(desc_content) + except Exception, exc: + if default == UNDEFINED: raise exc + else: return default

- def get_network_statuses(self): + def get_network_statuses(self, default = UNDEFINED): """ Provides an iterator for all of the router status entries that tor presently knows about.

+ :param list default: items to provide if the query fails + :returns: iterates over :class:`~stem.descriptor.router_status_entry.RouterStatusEntryV2` for relays in the tor network

- :raises: :class:`stem.ControllerError` if unable to query tor + :raises: :class:`stem.ControllerError` if unable to query tor and no + default was provided """

- # TODO: We should iterate over the descriptors as they're read from the - # socket rather than reading the whole thing into memeory. - - desc_content = self.get_info("ns/all") - - desc_iterator = stem.descriptor.router_status_entry.parse_file( - StringIO.StringIO(desc_content), - True, - entry_class = stem.descriptor.router_status_entry.RouterStatusEntryV2, - ) - - for desc in desc_iterator: - yield desc + try: + # TODO: We should iterate over the descriptors as they're read from the + # socket rather than reading the whole thing into memeory. + + desc_content = self.get_info("ns/all") + + desc_iterator = stem.descriptor.router_status_entry.parse_file( + StringIO.StringIO(desc_content), + True, + entry_class = stem.descriptor.router_status_entry.RouterStatusEntryV2, + ) + + for desc in desc_iterator: + yield desc + except Exception, exc: + if default == UNDEFINED: raise exc + else: + if entry is not None: + for entry in default: + yield entry

def authenticate(self, *args, **kwargs): """ @@ -979,10 +1032,12 @@ class Controller(BaseController): import stem.connection stem.connection.authenticate(self, *args, **kwargs)

- def protocolinfo(self): + def get_protocolinfo(self, default = UNDEFINED): """ A convenience method to get the protocol info of the controller.

+ :param object default: response if the query fails + :returns: :class:`~stem.response.protocolinfo.ProtocolInfoResponse` provided by tor

:raises: @@ -990,10 +1045,17 @@ class Controller(BaseController): malformed * :class:`stem.SocketError` if problems arise in establishing or using the socket + + An exception is only raised if we weren't provided a default response. """

import stem.connection - return stem.connection.get_protocolinfo(self) + + try: + return stem.connection.get_protocolinfo(self) + except Exception, exc: + if default == UNDEFINED: raise exc + else: return default

def get_conf(self, param, default = UNDEFINED, multiple = False): """ @@ -1325,48 +1387,55 @@ class Controller(BaseController): else: raise stem.ProtocolError("SAVECONF returned unexpected response code")

- def get_socks_listeners(self): + def get_socks_listeners(self, default = UNDEFINED): """ Provides the SOCKS **(address, port)** tuples that tor has open.

+ :param object default: response if the query fails + :returns: list of **(address, port)** tuples for the available SOCKS listeners

:raises: :class:`stem.ControllerError` if unable to determine the listeners + and no default was provided """

- proxy_addrs = [] - try: - 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() instead - socks_port = self.get_conf('SocksPort') + proxy_addrs = []

- for listener in self.get_conf('SocksListenAddress', multiple = True): - if ':' in listener: + try: + 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)) - 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] + except stem.InvalidArguments: + # tor version is old (pre-tor-0.2.2.26-beta), use get_conf() instead + socks_port = self.get_conf('SocksPort') + + for listener in self.get_conf('SocksListenAddress', multiple = True): + 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] + except Exception, exc: + if default == UNDEFINED: raise exc + else: return default

def is_feature_enabled(self, feature): """ @@ -1391,8 +1460,11 @@ class Controller(BaseController): elif feature == "VERBOSE_NAMES": defaulted_version = stem.version.Requirement.FEATURE_VERBOSE_NAMES

- if defaulted_version and self.get_version().meets_requirements(defaulted_version): - self._enabled_features.append(feature) + if defaulted_version: + our_version = self.get_version(None) + + if our_version and our_version.meets_requirements(defaulted_version): + self._enabled_features.append(feature)

return feature in self._enabled_features

@@ -1480,9 +1552,9 @@ class Controller(BaseController): :returns: str of the circuit id of the newly created circuit """

- return self.extend_circuit(0, path, purpose) + return self.extend_circuit('0', path, purpose)

- def extend_circuit(self, circuit = "0", path = None, purpose = "general"): + def extend_circuit(self, circuit_id = "0", path = None, purpose = "general"): """ Either requests the creation of a new circuit or extends an existing one.

@@ -1502,7 +1574,7 @@ class Controller(BaseController): 20 EXTENDED $718BCEA286B531757ACAFF93AE04910EA73DE617=KsmoinOK,$649F2D0ACF418F7CFC6539AB2257EB2D5297BAFA=Eskimo BUILD_FLAGS=NEED_CAPACITY PURPOSE=GENERAL TIME_CREATED=2012-12-06T13:51:11.433755 19 BUILT $718BCEA286B531757ACAFF93AE04910EA73DE617=KsmoinOK,$30BAB8EE7606CBD12F3CC269AE976E0153E7A58D=Pascal1,$2765D8A8C4BBA3F89585A9FFE0E8575615880BEB=Anthracite PURPOSE=GENERAL TIME_CREATED=2012-12-06T13:50:56.969938

- :param str circuit: id of a circuit to be extended + :param str circuit_id: id of a circuit to be extended :param list,str path: one or more relays to make a circuit through, this is required if the circuit id is non-zero :param str purpose: "general" or "controller" @@ -1513,15 +1585,15 @@ class Controller(BaseController): """

# we might accidently get integer circuit ids - circuit = str(circuit) + circuit_id = str(circuit_id)

- if path is None and circuit == '0': + if path is None and circuit_id == '0': path_opt_version = stem.version.Requirement.EXTENDCIRCUIT_PATH_OPTIONAL

if not self.get_version().meets_requirements(path_opt_version): raise stem.InvalidRequest(512, "EXTENDCIRCUIT requires the path prior to version %s" % path_opt_version)

- args = [circuit] + args = [circuit_id] if type(path) == str: path = [path] if path: args.append(",".join(path)) if purpose: args.append("purpose=%s" % purpose) @@ -1600,8 +1672,8 @@ class Controller(BaseController): Note: Tor attaches streams to circuits automatically unless the __LeaveStreamsUnattached configuration variable is set to "1"

- :param int stream_id: id of the stream that must be attached - :param int circuit_id: id of the circuit to which it must be attached + :param str stream_id: id of the stream that must be attached + :param str circuit_id: id of the circuit to which it must be attached :param int hop: hop in the circuit that must be used as an exit node

:raises: @@ -1610,7 +1682,7 @@ class Controller(BaseController): """

hop_str = " HOP=" + str(hop) if hop else "" - response = self.msg("ATTACHSTREAM %i %i%s" % (stream_id, circuit_id, hop_str)) + response = self.msg("ATTACHSTREAM %s %s%s" % (stream_id, circuit_id, hop_str)) stem.response.convert("SINGLELINE", response)

if not response.is_ok(): diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py index 2dac687..40c0f8c 100644 --- a/test/integ/control/controller.py +++ b/test/integ/control/controller.py @@ -253,7 +253,7 @@ class TestController(unittest.TestCase): runner = test.runner.get_runner()

with runner.get_tor_controller(False) as controller: - protocolinfo = controller.protocolinfo() + protocolinfo = controller.get_protocolinfo() self.assertTrue(isinstance(protocolinfo, stem.response.protocolinfo.ProtocolInfoResponse))

# Doing a sanity test on the ProtocolInfoResponse instance returned.