commit 82a3cc0500391b2a806f345e2e460238505236cd Author: Damian Johnson atagar@torproject.org Date: Sat Sep 23 13:47:42 2017 -0700
Better caching for get_listeners
We cached the GETINFO results but not all the other validation and such around the results. --- stem/control.py | 142 ++++++++++++++++++++++------------------ stem/version.py | 2 +- test/unit/control/controller.py | 9 +++ 3 files changed, 87 insertions(+), 66 deletions(-)
diff --git a/stem/control.py b/stem/control.py index ef9919be..a03835b9 100644 --- a/stem/control.py +++ b/stem/control.py @@ -387,10 +387,6 @@ CACHEABLE_GETINFO_PARAMS = (
CACHEABLE_GETINFO_PARAMS_UNTIL_SETCONF = ( 'accounting/enabled', - 'net/listeners/control', - 'net/listeners/dir', - 'net/listeners/or', - 'net/listeners/socks', )
# GETCONF parameters we shouldn't cache. This includes hidden service @@ -1323,79 +1319,85 @@ class Controller(BaseController): and no default was provided """
- proxy_addrs = [] - query = 'net/listeners/%s' % listener_type.lower() + listeners = self._get_cache(listener_type, 'listeners')
- try: - for listener in self.get_info(query).split(): - if not (listener.startswith('"') and listener.endswith('"')): - raise stem.ProtocolError("'GETINFO %s' responses are expected to be quoted: %s" % (query, listener)) - elif ':' not in listener: - raise stem.ProtocolError("'GETINFO %s' had a listener without a colon: %s" % (query, listener)) + if listeners is None: + proxy_addrs = [] + query = 'net/listeners/%s' % listener_type.lower()
- listener = listener[1:-1] # strip quotes - addr, port = listener.rsplit(':', 1) + try: + for listener in self.get_info(query).split(): + if not (listener.startswith('"') and listener.endswith('"')): + raise stem.ProtocolError("'GETINFO %s' responses are expected to be quoted: %s" % (query, listener)) + elif ':' not in listener: + raise stem.ProtocolError("'GETINFO %s' had a listener without a colon: %s" % (query, listener))
- # Skip unix sockets, for instance... - # - # GETINFO net/listeners/control - # 250-net/listeners/control="unix:/tmp/tor/socket" - # 250 OK - - if addr == 'unix': - continue - - if addr.startswith('[') and addr.endswith(']'): - addr = addr[1:-1] # unbracket ipv6 address - - proxy_addrs.append((addr, port)) - except stem.InvalidArguments: - # Tor version is old (pre-tor-0.2.2.26-beta), use get_conf() instead. - # Some options (like the ORPort) can have optional attributes after the - # actual port number. - - port_option = { - Listener.OR: 'ORPort', - Listener.DIR: 'DirPort', - Listener.SOCKS: 'SocksPort', - Listener.TRANS: 'TransPort', - Listener.NATD: 'NatdPort', - Listener.DNS: 'DNSPort', - Listener.CONTROL: 'ControlPort', - }[listener_type] - - listener_option = { - Listener.OR: 'ORListenAddress', - Listener.DIR: 'DirListenAddress', - Listener.SOCKS: 'SocksListenAddress', - Listener.TRANS: 'TransListenAddress', - Listener.NATD: 'NatdListenAddress', - Listener.DNS: 'DNSListenAddress', - Listener.CONTROL: 'ControlListenAddress', - }[listener_type] - - port_value = self.get_conf(port_option).split()[0] - - for listener in self.get_conf(listener_option, multiple = True): - if ':' in listener: + listener = listener[1:-1] # strip quotes addr, port = listener.rsplit(':', 1)
+ # Skip unix sockets, for instance... + # + # GETINFO net/listeners/control + # 250-net/listeners/control="unix:/tmp/tor/socket" + # 250 OK + + if addr == 'unix': + continue + if addr.startswith('[') and addr.endswith(']'): addr = addr[1:-1] # unbracket ipv6 address
proxy_addrs.append((addr, port)) - else: - proxy_addrs.append((listener, port_value)) + except stem.InvalidArguments: + # Tor version is old (pre-tor-0.2.2.26-beta), use get_conf() instead. + # Some options (like the ORPort) can have optional attributes after the + # actual port number. + + port_option = { + Listener.OR: 'ORPort', + Listener.DIR: 'DirPort', + Listener.SOCKS: 'SocksPort', + Listener.TRANS: 'TransPort', + Listener.NATD: 'NatdPort', + Listener.DNS: 'DNSPort', + Listener.CONTROL: 'ControlPort', + }[listener_type] + + listener_option = { + Listener.OR: 'ORListenAddress', + Listener.DIR: 'DirListenAddress', + Listener.SOCKS: 'SocksListenAddress', + Listener.TRANS: 'TransListenAddress', + Listener.NATD: 'NatdListenAddress', + Listener.DNS: 'DNSListenAddress', + Listener.CONTROL: 'ControlListenAddress', + }[listener_type] + + port_value = self.get_conf(port_option).split()[0] + + for listener in self.get_conf(listener_option, multiple = True): + if ':' in listener: + addr, port = listener.rsplit(':', 1) + + if addr.startswith('[') and addr.endswith(']'): + addr = addr[1:-1] # unbracket ipv6 address + + proxy_addrs.append((addr, port)) + else: + proxy_addrs.append((listener, port_value)) + + # validate that address/ports are valid, and convert ports to ints
- # validate that address/ports are valid, and convert ports to ints + for addr, port in proxy_addrs: + if not stem.util.connection.is_valid_ipv4_address(addr) and not stem.util.connection.is_valid_ipv6_address(addr): + raise stem.ProtocolError('Invalid address for a %s listener: %s' % (listener_type, addr)) + elif not stem.util.connection.is_valid_port(port): + raise stem.ProtocolError('Invalid port for a %s listener: %s' % (listener_type, port))
- for addr, port in proxy_addrs: - if not stem.util.connection.is_valid_ipv4_address(addr) and not stem.util.connection.is_valid_ipv6_address(addr): - raise stem.ProtocolError('Invalid address for a %s listener: %s' % (listener_type, addr)) - elif not stem.util.connection.is_valid_port(port): - raise stem.ProtocolError('Invalid port for a %s listener: %s' % (listener_type, port)) + listeners = [(addr, int(port)) for (addr, port) in proxy_addrs] + self._set_cache({listener_type: listeners}, 'listeners')
- return [(addr, int(port)) for (addr, port) in proxy_addrs] + return listeners
@with_default() def get_accounting_stats(self, default = UNDEFINED): @@ -2405,6 +2407,7 @@ class Controller(BaseController): # reset any getinfo parameters that can be changed by a SETCONF
self._set_cache(dict([(k.lower(), None) for k in CACHEABLE_GETINFO_PARAMS_UNTIL_SETCONF]), 'getinfo') + self._set_cache(None, 'listeners')
self._set_cache(to_cache, 'getconf') self._set_cache({'get_custom_options': None}) @@ -3101,6 +3104,15 @@ class Controller(BaseController): if not self.is_caching_enabled(): return
+ # if no params are provided then clear the namespace + + if not params and namespace: + for cache_key in self._request_cache.keys(): + if cache_key.startswith('%s.' % namespace): + del self._request_cache[cache_key] + + return + for key, value in list(params.items()): if namespace: cache_key = '%s.%s' % (namespace, key) diff --git a/stem/version.py b/stem/version.py index 87070093..5f7822ba 100644 --- a/stem/version.py +++ b/stem/version.py @@ -159,7 +159,7 @@ class Version(object): such as "0.1.4" or "0.2.2.23-alpha (git-7dcd105be34a4f44)".
.. versionchanged:: 1.6.0 - Added all_extra parameter.. + Added all_extra parameter.
:var int major: major version :var int minor: minor version diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index 17ca2bfc..ae5be387 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -162,6 +162,7 @@ class TestControl(unittest.TestCase):
self.assertEqual([('127.0.0.1', 9050)], self.controller.get_listeners(Listener.CONTROL)) self.assertEqual([9050], self.controller.get_ports(Listener.CONTROL)) + self.controller.clear_cache()
# non-local addresss
@@ -172,6 +173,7 @@ class TestControl(unittest.TestCase):
self.assertEqual([('27.4.4.1', 9050)], self.controller.get_listeners(Listener.CONTROL)) self.assertEqual([], self.controller.get_ports(Listener.CONTROL)) + self.controller.clear_cache()
# Exercise via the GETINFO option.
@@ -184,6 +186,7 @@ class TestControl(unittest.TestCase): )
self.assertEqual([1112, 1114], self.controller.get_ports(Listener.CONTROL)) + self.controller.clear_cache()
# IPv6 address
@@ -196,6 +199,7 @@ class TestControl(unittest.TestCase):
# unix socket file
+ self.controller.clear_cache() get_info_mock.return_value = '"unix:/tmp/tor/socket"'
self.assertEqual([], self.controller.get_listeners(Listener.CONTROL)) @@ -220,6 +224,7 @@ class TestControl(unittest.TestCase): }[param]
self.assertEqual([('127.0.0.1', 9050)], self.controller.get_socks_listeners()) + self.controller.clear_cache()
# Again, an old tor, but SocksListenAddress overrides the port number.
@@ -229,6 +234,7 @@ class TestControl(unittest.TestCase): }[param]
self.assertEqual([('127.0.0.1', 1112)], self.controller.get_socks_listeners()) + self.controller.clear_cache()
# Again, an old tor, but multiple listeners
@@ -238,6 +244,7 @@ class TestControl(unittest.TestCase): }[param]
self.assertEqual([('127.0.0.1', 1112), ('127.0.0.1', 1114)], self.controller.get_socks_listeners()) + self.controller.clear_cache()
# Again, an old tor, but no SOCKS listeners
@@ -247,6 +254,7 @@ class TestControl(unittest.TestCase): }[param]
self.assertEqual([], self.controller.get_socks_listeners()) + self.controller.clear_cache()
# Where tor provides invalid ports or addresses
@@ -289,6 +297,7 @@ class TestControl(unittest.TestCase):
# no SOCKS listeners
+ self.controller.clear_cache() get_info_mock.return_value = '' self.assertEqual([], self.controller.get_socks_listeners())