commit 542fa1f60552c259e3510e27727a5a2d5f20b7e9
Author: Damian Johnson <ata...@torproject.org>
Date: Sat Aug 4 14:55:40 2018 -0700
Provide None from get_exit_policy when not a relay
When not configured to be a tor relay tor's 'GETINFO exit-policy/full' query
won't function. Rather than attempt to guess the exit policy from other
configuration options providing None for 552 responses...
https://trac.torproject.org/projects/tor/ticket/25853
https://gitweb.torproject.org/torspec.git/commit/?id=c5453a0
The above spec addition makes it ambiguous if 552 means 'not a relay' or
'we had an error' so reached out for clarificaition on how these should
be handled...
https://trac.torproject.org/projects/tor/ticket/27034
In the process also had to fix get_info to raise OperationFailed rather than
a ProtocolError so callers can get the underlying response codes.
---
docs/change_log.rst | 2 ++
stem/control.py | 41 +++++++++++++----------------------------
stem/response/getinfo.py | 8 ++++++++
test/unit/control/controller.py | 29 ++++++++++++++---------------
4 files changed, 37 insertions(+), 43 deletions(-)
diff --git a/docs/change_log.rst b/docs/change_log.rst
index ca8b6805..282a02c8 100644
--- a/docs/change_log.rst
+++ b/docs/change_log.rst
@@ -52,7 +52,9 @@ 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`)
* More reliable caching during configuration changes, especially in
multiple-controller situations (:trac:`25821`)
+ * :func:`~stem.control.Controller.get_info` commonly raised
:class:`stem.ProtocolError` when it should provide :class:`stem.OperationFailed`
* :func:`~stem.control.Controller.get_microdescriptors` reads descriptors
from the control port if available (:spec:`b5396d5`)
+ * :func:`~stem.control.Controller.get_exit_policy` now provides None if not
configured to be a relay (:trac:`25853`, :spec:`c5453a0`)
* Added the delivered_read, delivered_written, overhead_read, and
overhead_written attributes to
:class:`~stem.response.events.CircuitBandwidthEvent` (:spec:`fbb38ec`)
* The *config* attribute of :class:`~stem.response.events.ConfChangedEvent`
couldn't represent tor configuration options with multiple values. It has been
replaced with new *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
diff --git a/stem/control.py b/stem/control.py
index 77e6140e..19a8759d 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -1127,6 +1127,10 @@ class Controller(BaseController):
.. versionchanged:: 1.1.0
Added the get_bytes argument.
+ .. versionchanged:: 1.7.0
+ Errors commonly provided a :class:`stem.ProtocolError` when we should
+ raise a :class:`stem.OperationFailed`.
+
:param str,list params: GETINFO option or options to be queried
:param object default: response if the query fails
:param bool get_bytes: provides **bytes** values rather than a **str**
under python 3.x
@@ -1275,10 +1279,13 @@ class Controller(BaseController):
parsing the user's torrc entries. This should be more reliable for
some edge cases. (:trac:`25739`)
+ .. versionchanged:: 1.7.0
+ Returning **None** if not contigured to be a relay.
+
:param object default: response if the query fails
:returns: :class:`~stem.exit_policy.ExitPolicy` of the tor instance that
- we're connected to
+ we're connected to, this is **None** if not configured to be a relay
:raises:
* :class:`stem.ControllerError` if unable to query the policy
@@ -1292,34 +1299,12 @@ class Controller(BaseController):
if not policy:
try:
policy =
stem.exit_policy.ExitPolicy(*self.get_info('exit-policy/full').splitlines())
- except stem.ProtocolError:
- # When tor is unable to determine our address 'GETINFO
- # exit-policy/full' fails with...
- #
- # ProtocolError: GETINFO response didn't have an OK status:
- # router_get_my_routerinfo returned NULL
- #
- # https://trac.torproject.org/projects/tor/ticket/25842
- #
- # Failing back to the legacy method we used for getting our exit
- # policy.
+ self._set_cache({'exit_policy': policy})
+ except stem.OperationFailed as exc:
+ if exc.code == '552':
+ return None # not configured to be a relay
- rules = []
-
- if self.get_conf('ExitRelay') == '0':
- rules.append('reject *:*')
-
- if self.get_conf('ExitPolicyRejectPrivate') == '1':
- rules.append('reject private:*')
-
- for policy_line in self.get_conf('ExitPolicy', multiple = True):
- rules += policy_line.split(',')
-
- rules += self.get_info('exit-policy/default').split(',')
-
- policy = stem.exit_policy.get_config_policy(rules,
self.get_info('address', None))
-
- self._set_cache({'exit_policy': policy})
+ raise
return policy
diff --git a/stem/response/getinfo.py b/stem/response/getinfo.py
index 03a2cf38..00c77b92 100644
--- a/stem/response/getinfo.py
+++ b/stem/response/getinfo.py
@@ -30,12 +30,20 @@ class GetInfoResponse(stem.response.ControlMessage):
if not self.is_ok() or not remaining_lines.pop() == b'OK':
unrecognized_keywords = []
+ error_code, error_msg = None, None
+
for code, _, line in self.content():
+ if code != '250':
+ error_code = code
+ error_msg = line
+
if code == '552' and line.startswith('Unrecognized key "') and
line.endswith('"'):
unrecognized_keywords.append(line[18:-1])
if unrecognized_keywords:
raise stem.InvalidArguments('552', 'GETINFO request contained
unrecognized keywords: %s\n' % ', '.join(unrecognized_keywords),
unrecognized_keywords)
+ elif error_code:
+ raise stem.OperationFailed(error_code, error_msg)
else:
raise stem.ProtocolError("GETINFO response didn't have an OK
status:\n%s" % self)
diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py
index d8e1cc9d..3bdb7dce 100644
--- a/test/unit/control/controller.py
+++ b/test/unit/control/controller.py
@@ -58,19 +58,19 @@ class TestControl(unittest.TestCase):
msg_mock.return_value = ControlMessage.from_str('551 Address unknown\r\n')
self.assertEqual(None, self.controller._last_address_exc)
- self.assertRaisesRegexp(stem.ProtocolError, 'Address unknown',
self.controller.get_info, 'address')
- self.assertEqual("GETINFO response didn't have an OK status:\nAddress
unknown", str(self.controller._last_address_exc))
+ self.assertRaisesRegexp(stem.OperationFailed, 'Address unknown',
self.controller.get_info, 'address')
+ self.assertEqual('Address 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.assertRaisesRegexp(stem.OperationFailed, 'Address unknown',
self.controller.get_info, 'address')
self.assertEqual(1, msg_mock.call_count)
# 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.assertRaisesRegexp(stem.OperationFailed, 'Address unknown',
self.controller.get_info, 'address')
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'))
@@ -88,19 +88,19 @@ class TestControl(unittest.TestCase):
get_conf_mock.return_value = None
self.assertEqual(None, self.controller._last_fingerprint_exc)
- self.assertRaisesRegexp(stem.ProtocolError, 'Not running in server mode',
self.controller.get_info, 'fingerprint')
- self.assertEqual("GETINFO response didn't have an OK status:\nNot running
in server mode", str(self.controller._last_fingerprint_exc))
+ self.assertRaisesRegexp(stem.OperationFailed, 'Not running in server
mode', self.controller.get_info, 'fingerprint')
+ self.assertEqual('Not running in server mode',
str(self.controller._last_fingerprint_exc))
self.assertEqual(1, msg_mock.call_count)
# now that we have a cached failure we should provide that back
- self.assertRaisesRegexp(stem.ProtocolError, 'Not running in server mode',
self.controller.get_info, 'fingerprint')
+ self.assertRaisesRegexp(stem.OperationFailed, 'Not running in server
mode', self.controller.get_info, 'fingerprint')
self.assertEqual(1, msg_mock.call_count)
# ... but if we become a relay we'll call it again
get_conf_mock.return_value = '443'
- self.assertRaisesRegexp(stem.ProtocolError, 'Not running in server mode',
self.controller.get_info, 'fingerprint')
+ self.assertRaisesRegexp(stem.OperationFailed, 'Not running in server
mode', self.controller.get_info, 'fingerprint')
self.assertEqual(2, msg_mock.call_count)
@patch('stem.control.Controller.get_info')
@@ -158,17 +158,11 @@ class TestControl(unittest.TestCase):
self.controller._is_caching_enabled = True
@patch('stem.control.Controller.get_info')
- @patch('stem.control.Controller.get_conf')
- def test_get_exit_policy(self, get_conf_mock, get_info_mock):
+ def test_get_exit_policy(self, get_info_mock):
"""
Exercises the get_exit_policy() method.
"""
- get_conf_mock.side_effect = lambda param, **kwargs: {
- 'ExitPolicyRejectPrivate': '1',
- 'ExitPolicy': ['accept *:80, accept *:443', 'accept 43.5.5.5,reject
*:22'],
- }[param]
-
get_info_mock.side_effect = lambda param, default = None: {
'exit-policy/full': 'reject *:25,reject *:119,reject *:135-139,reject
*:445,reject *:563,reject *:1214,reject *:4661-4666,reject *:6346-6429,reject
*:6699,reject *:6881-6999,accept *:*',
}[param]
@@ -190,6 +184,11 @@ class TestControl(unittest.TestCase):
self.assertEqual(expected, self.controller.get_exit_policy())
@patch('stem.control.Controller.get_info')
+ def test_get_exit_policy_if_not_relaying(self, get_info_mock):
+ get_info_mock.side_effect = stem.OperationFailed('552', 'Not running in
server mode')
+ self.assertEqual(None, self.controller.get_exit_policy())
+
+ @patch('stem.control.Controller.get_info')
@patch('stem.control.Controller.get_conf')
def test_get_ports(self, get_conf_mock, get_info_mock):
"""
_______________________________________________
tor-commits mailing list
tor-commits@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
