[tor-bugs] #25821 [Core Tor/Stem]: Stem getconf cache doesn't clear for CONF_CHANGED events; probably should set value

[Tor Bug Tracker & Wiki]

#25821: Stem getconf cache doesn't clear for CONF_CHANGED events; probably should
set value
---------------------------+------------------------
 Reporter:  dmr            |          Owner:  atagar
     Type:  defect         |         Status:  new
 Priority:  Medium         |      Milestone:
Component:  Core Tor/Stem  |        Version:
 Severity:  Normal         |     Resolution:
 Keywords:                 |  Actual Points:
Parent ID:                 |         Points:
 Reviewer:                 |        Sponsor:
---------------------------+------------------------

Comment (by dmr):

 Here's the code as of
 [[https://gitweb.torproject.org/stem.git/tree/stem/control.py?id=82b22046f40f49579ff37c4247db50050334998a#n1071|`82b22046f40f49579ff37c4247db50050334998a`]]:
 {{{
 #!python
     def _confchanged_listener(event):
       if self.is_caching_enabled():
         self._set_cache(dict((k, None) for k in event.config), 'getconf')

         # ...

     self.add_event_listener(_confchanged_listener, EventType.CONF_CHANGED)
 }}}

 This code actually usually won't clear the cache when it intends to. The
 cache key is `lower()`ed when set in `set_options()`
 ([[https://gitweb.torproject.org/stem.git/tree/stem/control.py?id=82b22046f40f49579ff37c4247db50050334998a#n2402|code
 link]]):
 {{{
 #!python
       if self.is_caching_enabled():
         to_cache = {}

         for param, value in params:
           param = param.lower()

           # ...

           to_cache[param] = value

           # ...

         # 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})
 }}}
 ... but we can't guarantee that the event we receive will have a given
 case. In my tests, it was always the canonical torrc case, but that's not
 concretely specified in the control spec
 ([[https://gitweb.torproject.org/torspec.git/tree/control-
 spec.txt?id=d4a64fbf5aaba383638d9f3c70bd2951f8c5ad89#n259|"most keywords
 are case-sensitive")]].

 At first, this looks like a simple change to fix (use `k.lower()`), but
 when we consider what happens when a config options is set, we see it's
 not so straightforward - the cache gets cleared.

 Here's code for an adhoc test:
 {{{
 #!python
 # run this example interactively (don't copy/paste everything) in tor-
 prompt
 def print_getconf_cache_contents():
   global controller

   with controller._cache_lock:
     cache_keys = [k for k in controller._request_cache.keys() if
 k.startswith('getconf.')]
     if cache_keys:
       print("printing cache")
       for k in cache_keys:
         value = controller._request_cache[k]
         print("config cache %s has value: %s" % (k, value))
     else:
       print("cache is empty")

 def print_conf_event_and_cache(event):
   # blank line to make prettier at a prompt
   print('')

   for k,v in event.config.items():
     print("Config %s set to: %s" % (k, v))

   print_getconf_cache_contents()

 # in tor-prompt, controller is simply available for us to use
 controller.add_event_listener(print_conf_event_and_cache,
 stem.control.EventType.CONF_CHANGED)

 # for example simplicity WAIT for the output between each set_conf() call
 controller.set_conf('ContactInfo', 'hi there')
 # ^ generate output via our listener
 # with k.lower() implementation, note that the cache doesn't have
 'getconf.contactinfo'

 # simulate a SETCONF from another controller
 SETCONF ContactInfo="hello back"
 # ^ generate output via our listener
 # with unchanged implementation, note that the cache is still set to "hi
 there"
 # with k.lower() implementation, note that the cache doesn't have
 'getconf.contactinfo'

 # simulate a SETCONF from another controller
 SETCONF ContactInfo=
 # ^ generate output via our listener
 # with unchanged implementation, note that the cache is still set to "hi
 there"
 # with k.lower() implementation, note that the cache doesn't have
 'getconf.contactinfo'

 # clean up
 controller.remove_event_listener(print_conf_event_and_cache)
 }}}

 To do a simple change to the implementation, change this line in
 `_confchanged_listener()` from:
 {{{
 #!python
         self._set_cache(dict((k, None) for k in event.config), 'getconf')
 }}}
 to:
 {{{
 #!python
         self._set_cache(dict((k.lower(), None) for k in event.config),
 'getconf')
 }}}
 (adding `.lower()` after `k`)

 You can see that the cache doesn't clear, but when it does with a simple
 `.lower()` change... it doesn't really retain data for long (because the
 `set_conf()` causes a `CONF_CHANGED`, which clears the cache).

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25821#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online