commit f19b9273630b05c42b8fe4d59f449f001d870cba Author: Damian Johnson atagar@torproject.org Date: Wed May 16 13:01:28 2018 -0700
ConfChangedEvent could not represent multiple config values
Great catch from dmr. The ConfChangedEvent's config was a simple {str => str} mapping, but some tor configuration options can have multiple values. For instance...
650-CONF_CHANGED 650-ExitPolicy=accept 34.3.4.5 650-ExitPolicy=accept 3.4.53.3 650 OK
We can't change the event's config attribute without breaking backward compatability, but that's ok. Fiddling with this event a bit it's usage was actually kinda clunky since we bundled config unsetting with modifications. As such, splitting the 'config' attribute into a 'changed' and 'unset'. This time with changed as a {str => list} mapping.
Here's a little demo of what the new usage might look like...
def print_config_changes(event): for key, values in my_event.changed: print('We changed %s to %s' % (key, ', '.join(values))
for key in my_event.unset: print('We removed our configuration of %s' % key)
my_controller.add_event_listener(print_config_changes, EventType.CONF_CHANGED) --- docs/change_log.rst | 1 + stem/response/events.py | 19 ++++++++++++++++--- test/unit/response/events.py | 33 ++++++++++++++++++++++++++++++--- 3 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/docs/change_log.rst b/docs/change_log.rst index 54a1e628..549e89c5 100644 --- a/docs/change_log.rst +++ b/docs/change_log.rst @@ -52,6 +52,7 @@ The following are only available within Stem's `git repository * Stacktrace if :func:`stem.connection.connect` had a string port argument * More reliable ExitPolicy resolution (:trac:`25739`) * Added the delivered_read, delivered_written, overhead_read, and overhead_written attributes to :class:`~stem.response.events.CircuitBandwidthEvent` (:spec:`fbb38ec`) + * Replaced the *config* attribute of :class:`~stem.response.events.ConfChangedEvent` with a *changed* and *unset* attributes * Replaced socket's :func:`~stem.socket.ControlPort.get_address`, :func:`~stem.socket.ControlPort.get_port`, and :func:`~stem.socket.ControlSocketFile.get_socket_path` with attributes * Removed 'raw' argument from :func:`~stem.socket.ControlSocket.send`
diff --git a/stem/response/events.py b/stem/response/events.py index 1048e2ba..a50d90f8 100644 --- a/stem/response/events.py +++ b/stem/response/events.py @@ -570,15 +570,26 @@ class ConfChangedEvent(Event):
The CONF_CHANGED event was introduced in tor version 0.2.3.3-alpha.
- :var dict config: mapping of configuration options to their new values - (**None** if the option is being unset) + .. deprecated:: 1.7.0 + Deprecated the *config* attribute. Some tor configuration options (like + ExitPolicy) can have multiple values, so a simple 'str => str' mapping + meant that we only provided the last. + + .. versionchanged:: 1.7.0 + Added the changed and unset attributes. + + :var dict changed: mapping of configuration options to a list of their new + values + :var list unset: configuration options that have been unset """
_SKIP_PARSING = True _VERSION_ADDED = stem.version.Requirement.EVENT_CONF_CHANGED
def _parse(self): - self.config = {} + self.changed = {} + self.unset = [] + self.config = {} # TODO: remove in stem 2.0
# Skip first and last line since they're the header and footer. For # instance... @@ -592,8 +603,10 @@ class ConfChangedEvent(Event): for line in str(self).splitlines()[1:-1]: if '=' in line: key, value = line.split('=', 1) + self.changed.setdefault(key, []).append(value) else: key, value = line, None + self.unset.append(key)
self.config[key] = value
diff --git a/test/unit/response/events.py b/test/unit/response/events.py index 69a3624b..82506e6d 100644 --- a/test/unit/response/events.py +++ b/test/unit/response/events.py @@ -252,6 +252,13 @@ CONF_CHANGED_EVENT = """650-CONF_CHANGED 650 OK """
+CONF_CHANGED_EVENT_MULTIPLE = """650-CONF_CHANGED +650-ExitPolicy=accept 34.3.4.5 +650-ExitPolicy=accept 3.4.53.3 +650-MaxCircuitDirtiness=20 +650 OK +""" + # GUARD events from tor v0.2.1.30.
GUARD_NEW = '650 GUARD ENTRY $36B5DBA788246E8369DBAF58577C6BC044A9A374 NEW' @@ -863,15 +870,35 @@ class TestEvents(unittest.TestCase):
def test_conf_changed(self): event = _get_event(CONF_CHANGED_EVENT) + self.assertTrue(isinstance(event, stem.response.events.ConfChangedEvent)) + + self.assertEqual({ + 'ExitNodes': ['caerSidi'], + 'MaxCircuitDirtiness': ['20'], + }, event.changed)
- expected_config = { + self.assertEqual(['ExitPolicy'], event.unset) + + self.assertEqual({ 'ExitNodes': 'caerSidi', 'MaxCircuitDirtiness': '20', 'ExitPolicy': None, - } + }, event.config)
+ event = _get_event(CONF_CHANGED_EVENT_MULTIPLE) self.assertTrue(isinstance(event, stem.response.events.ConfChangedEvent)) - self.assertEqual(expected_config, event.config) + + self.assertEqual({ + 'ExitPolicy': ['accept 34.3.4.5', 'accept 3.4.53.3'], + 'MaxCircuitDirtiness': ['20'], + }, event.changed) + + self.assertEqual([], event.unset) + + self.assertEqual({ + 'ExitPolicy': 'accept 3.4.53.3', # overwrote with second value + 'MaxCircuitDirtiness': '20', + }, event.config)
def test_descchanged_event(self): # all we can check for is that the event is properly parsed as a