[tor-commits] [stem/master] Invalidate cached address and exit policy when our address changes
atagar at torproject.org
atagar at torproject.org
Thu Apr 19 19:17:01 UTC 2018
commit 331406cb5ffe8e4e216e6d1396664f34d6c3cb28
Author: Damian Johnson <atagar at torproject.org>
Date: Thu Apr 19 12:12:15 2018 -0700
Invalidate cached address and exit policy when our address changes
Great point from dmr and others on #25423 that when our address changes we
need to invalidate our cached exit policy. STATUS_SERVER events provide us
with a notification when our address changes.
Besides our exit policy, this also lets us simplify our caching around
addresses. Stem had a special TTL so we wouldn't cache invalid addresses
indefinitely, but with this change we can now replace that with event driven
invalidation (much better!).
---
stem/control.py | 22 +++++++++-------------
test/unit/control/controller.py | 20 +++++++++++++-------
2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/stem/control.py b/stem/control.py
index 02183d95..ecec96aa 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -389,12 +389,6 @@ CACHEABLE_GETINFO_PARAMS_UNTIL_SETCONF = (
'accounting/enabled',
)
-# 'GETINFO address' isn't technically cachable, but it's also both highly
-# requested and highly static. As such fetching it at a fixed rate. If
-# you'd like you can set this global to zero to disable caching.
-
-CACHE_ADDRESS_FOR = 1.0
-
# GETCONF parameters we shouldn't cache. This includes hidden service
# perameters due to the funky way they're set and retrieved (for instance,
# 'SETCONF HiddenServiceDir' effects 'GETCONF HiddenServiceOptions').
@@ -1055,7 +1049,6 @@ class Controller(BaseController):
self._enabled_features = []
self._is_geoip_unavailable = None
- self._address_cached_at = 0
self._last_address_exc = None
self._last_fingerprint_exc = None
@@ -1076,6 +1069,14 @@ class Controller(BaseController):
self.add_event_listener(_confchanged_listener, EventType.CONF_CHANGED)
+ def _address_changed_listener(event):
+ if event.action == 'EXTERNAL_ADDRESS':
+ self._set_cache({'exit_policy': None})
+ self._set_cache({'address': None}, 'getinfo')
+ self._last_address_exc = None
+
+ self.add_event_listener(_address_changed_listener, EventType.STATUS_SERVER)
+
def close(self):
self.clear_cache()
super(Controller, self).close()
@@ -1153,7 +1154,7 @@ class Controller(BaseController):
for param in params:
if param.startswith('ip-to-country/') and param != 'ip-to-country/0.0.0.0' and self.is_geoip_unavailable():
raise stem.ProtocolError('Tor geoip database is unavailable')
- elif param == 'address' and self._last_address_exc and (time.time() - self._address_cached_at) < CACHE_ADDRESS_FOR:
+ elif param == 'address' and self._last_address_exc:
raise self._last_address_exc # we already know we can't resolve an address
elif param == 'fingerprint' and self._last_fingerprint_exc and self.get_conf('ORPort', None) is None:
raise self._last_fingerprint_exc # we already know we're not a relay
@@ -1164,9 +1165,6 @@ class Controller(BaseController):
cached_results = self._get_cache_map(from_cache, 'getinfo')
for key in cached_results:
- if key == 'address' and (time.time() - self._address_cached_at) > CACHE_ADDRESS_FOR:
- continue # cached address is too old
-
user_expected_key = _case_insensitive_lookup(params, key)
reply[user_expected_key] = cached_results[key]
params.remove(user_expected_key)
@@ -1207,7 +1205,6 @@ class Controller(BaseController):
self._set_cache(to_cache, 'getinfo')
if 'address' in params:
- self._address_cached_at = time.time()
self._last_address_exc = None
if 'fingerprint' in params:
@@ -1221,7 +1218,6 @@ class Controller(BaseController):
return list(reply.values())[0]
except stem.ControllerError as exc:
if 'address' in params:
- self._address_cached_at = time.time()
self._last_address_exc = exc
if 'fingerprint' in params:
diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py
index 92abfb67..e7b014b9 100644
--- a/test/unit/control/controller.py
+++ b/test/unit/control/controller.py
@@ -5,7 +5,6 @@ integ tests, but a few bits lend themselves to unit testing.
import datetime
import io
-import time
import unittest
import stem.descriptor.router_status_entry
@@ -32,7 +31,10 @@ class TestControl(unittest.TestCase):
def setUp(self):
socket = stem.socket.ControlSocket()
- with patch('stem.control.Controller.add_event_listener', Mock()):
+ # When initially constructing a controller we need to suppress msg, so our
+ # constructor's SETEVENTS requests pass.
+
+ with patch('stem.control.BaseController.msg', Mock()):
self.controller = Controller(socket)
def test_event_description(self):
@@ -53,13 +55,10 @@ class TestControl(unittest.TestCase):
@patch('stem.control.Controller.msg')
def test_get_info_address_caching(self, msg_mock):
- test_start = time.time()
msg_mock.return_value = ControlMessage.from_str('551 Address unknown\r\n')
- self.assertEqual(0, self.controller._address_cached_at)
self.assertEqual(None, self.controller._last_address_exc)
self.assertRaisesRegexp(stem.ProtocolError, 'Address unknown', self.controller.get_info, 'address')
- self.assertTrue(test_start <= self.controller._address_cached_at <= time.time())
self.assertEqual("GETINFO response didn't have an OK status:\nAddress unknown", str(self.controller._last_address_exc))
self.assertEqual(1, msg_mock.call_count)
@@ -68,12 +67,19 @@ class TestControl(unittest.TestCase):
self.assertRaisesRegexp(stem.ProtocolError, 'Address unknown', self.controller.get_info, 'address')
self.assertEqual(1, msg_mock.call_count)
- # pretend it's a minute later and try again, now with an address
+ # invalidates the cache, transitioning from no address to having one
msg_mock.return_value = ControlMessage.from_str('250-address=17.2.89.80\r\n250 OK\r\n', 'GETINFO')
self.assertRaisesRegexp(stem.ProtocolError, 'Address unknown', self.controller.get_info, 'address')
- self.controller._address_cached_at -= 60
+ self.controller._handle_event(ControlMessage.from_str('650 STATUS_SERVER NOTICE EXTERNAL_ADDRESS ADDRESS=17.2.89.80 METHOD=DIRSERV\r\n'))
+ self.assertEqual('17.2.89.80', self.controller.get_info('address'))
+
+ # invalidates the cache, transitioning from one address to another
+
+ msg_mock.return_value = ControlMessage.from_str('250-address=80.89.2.17\r\n250 OK\r\n', 'GETINFO')
self.assertEqual('17.2.89.80', self.controller.get_info('address'))
+ self.controller._handle_event(ControlMessage.from_str('650 STATUS_SERVER NOTICE EXTERNAL_ADDRESS ADDRESS=80.89.2.17 METHOD=DIRSERV\r\n'))
+ self.assertEqual('80.89.2.17', self.controller.get_info('address'))
@patch('stem.control.Controller.msg')
@patch('stem.control.Controller.get_conf')
More information about the tor-commits
mailing list