[tor-commits] [stem/master] Temporarily cache 'GETINFO address' responses

[atagar at torproject.org]

commit d0922386bf73ab81e3db9f993adf9842c18f4d2a
Author: Damian Johnson <atagar at torproject.org>
Date:   Wed Oct 25 13:33:34 2017 -0700

    Temporarily cache 'GETINFO address' responses
    
    This is a mutable parameter, but it's both reasonably static and highly
    requested. Caching it for a second by default.
---
 stem/control.py                 | 23 +++++++++++++++++++++++
 test/unit/control/controller.py | 29 +++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index 6746441c..eebc24b6 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -372,6 +372,7 @@ MAPPED_CONFIG_KEYS = {
 # unchangeable GETINFO parameters
 
 CACHEABLE_GETINFO_PARAMS = (
+  'address',
   'version',
   'config-file',
   'exit-policy/default',
@@ -389,6 +390,12 @@ 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').
@@ -1048,6 +1055,9 @@ class Controller(BaseController):
     self._event_listeners_lock = threading.RLock()
     self._enabled_features = []
     self._is_geoip_unavailable = None
+
+    self._address_cached_at = 0
+    self._last_address_exc = None
     self._last_fingerprint_exc = None
 
     super(Controller, self).__init__(control_socket, is_authenticated)
@@ -1145,6 +1155,8 @@ 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:
+        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
 
@@ -1153,6 +1165,9 @@ class Controller(BaseController):
     cached_results = self._get_cache_map(map(str.lower, params), '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)
@@ -1192,6 +1207,10 @@ 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:
         self._last_fingerprint_exc = None
 
@@ -1202,6 +1221,10 @@ class Controller(BaseController):
       else:
         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:
         self._last_fingerprint_exc = exc
 
diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py
index c7e412c5..a9a7e73d 100644
--- a/test/unit/control/controller.py
+++ b/test/unit/control/controller.py
@@ -5,6 +5,7 @@ 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
@@ -46,13 +47,37 @@ class TestControl(unittest.TestCase):
       self.assertTrue(stem.control.event_description(event) is not None)
 
   @patch('stem.control.Controller.msg')
-  def test_get_get_info(self, msg_mock):
+  def test_get_info(self, msg_mock):
     msg_mock.return_value = ControlMessage.from_str('250-hello=hi right back!\r\n250 OK\r\n', 'GETINFO')
     self.assertEqual('hi right back!', self.controller.get_info('hello'))
 
   @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)
+
+    # now that we have a cached failure we should provide that back
+
+    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
+
+    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.assertEqual('17.2.89.80', self.controller.get_info('address'))
+
+  @patch('stem.control.Controller.msg')
   @patch('stem.control.Controller.get_conf')
-  def test_get_get_info_without_fingerprint(self, get_conf_mock, msg_mock):
+  def test_get_info_without_fingerprint(self, get_conf_mock, msg_mock):
     msg_mock.return_value = ControlMessage.from_str('551 Not running in server mode\r\n')
     get_conf_mock.return_value = None