commit 3b25147b7fcaadbc7e2aec6ec89158ed1d738687 Author: Ravi Chandra Padmala neenaoffline@gmail.com Date: Tue Jun 12 19:59:30 2012 +0530

Multiple fixes to GETCONF parsing

* Add stem.socket.InvalidArguments and make the GETCONF parser raise it instead of stem.socket.InvalidRequest.

* Fix the parser to parser to correctly parse multivalue configuration keys (such as ExitPolicy). Add tests for the same.

* Fix tests to run against a Tor client using a control socket file.

* Minor changes to documentation. --- stem/control.py | 2 +- stem/response/__init__.py | 2 +- stem/response/getconf.py | 49 ++++++++++++++++++++++++-------------- stem/socket.py | 21 +++++++++++++++- test/integ/control/controller.py | 20 ++++++++++----- test/unit/response/getconf.py | 33 ++++++++++++++++++++++++- 6 files changed, 97 insertions(+), 30 deletions(-)

diff --git a/stem/control.py b/stem/control.py index c6bf29d..f0459cb 100644 --- a/stem/control.py +++ b/stem/control.py @@ -550,7 +550,7 @@ class Controller(BaseController):

:raises: :class:`stem.socket.ControllerError` if the call fails, and we weren't provided a default response - :class:`stem.socket.InvalidRequest` if configuration option requested was invalid. + :class:`stem.socket.InvalidArguments` if configuration options requested was invalid """

if isinstance(param, str): diff --git a/stem/response/__init__.py b/stem/response/__init__.py index a9b69a3..43ecb3b 100644 --- a/stem/response/__init__.py +++ b/stem/response/__init__.py @@ -60,7 +60,7 @@ def convert(response_type, message):

:raises: * :class:`stem.socket.ProtocolError` the message isn't a proper response of that type - * :class:`stem.socket.InvalidRequest` the request was invalid + * :class:`stem.socket.InvalidArguments` the request's arguments are invalid * TypeError if argument isn't a :class:`stem.response.ControlMessage` or response_type isn't supported """

diff --git a/stem/response/getconf.py b/stem/response/getconf.py index c1872cd..cffbc61 100644 --- a/stem/response/getconf.py +++ b/stem/response/getconf.py @@ -1,8 +1,24 @@ -import re - import stem.socket import stem.response

+def _getval(dictionary, key): + try: + return dictionary[key] + except KeyError: + pass + +def _split_line(line): + try: + if '=' in line: + if line[line.find("=") + 1] == """: + return line.pop_mapping(True) + else: + return line.split("=", 1) + else: + return (line, None) + except IndexError: + return (line[:-1], None) + class GetConfResponse(stem.response.ControlMessage): """ Reply for a GETCONF query. @@ -25,29 +41,26 @@ class GetConfResponse(stem.response.ControlMessage): if not self.is_ok(): unrecognized_keywords = [] for code, _, line in self.content(): - if code == '552': - try: - # to parse: 552 Unrecognized configuration key "zinc" - unrecognized_keywords.append(re.search('"([^"]+)"', line).groups()[0]) - except: - pass + if code == '552' and line.startswith("Unrecognized configuration key "") and line.endswith("""): + unrecognized_keywords.append(line[32:-1])

if unrecognized_keywords: - raise stem.socket.InvalidRequest("GETCONF request contained unrecognized keywords: %s\n" \ - % ', '.join(unrecognized_keywords)) + raise stem.socket.InvalidArguments("GETCONF request contained unrecognized keywords: %s\n" \ + % ', '.join(unrecognized_keywords), unrecognized_keywords) else: raise stem.socket.ProtocolError("GETCONF response contained a non-OK status code:\n%s" % self)

while remaining_lines: line = remaining_lines.pop(0)

- if '=' in line: - if line[line.find("=") + 1] == """: - key, value = line.pop_mapping(True) - else: - key, value = line.split("=", 1) + key, value = _split_line(line) + entry = _getval(self.entries, key) + + if type(entry) == str and entry != value: + self.entries[key] = [entry] + self.entries[key].append(value) + elif type(entry) == list and not value in entry: + self.entries[key].append(value) else: - key, value = (line, None) - - self.entries[key] = value + self.entries[key] = value

diff --git a/stem/socket.py b/stem/socket.py index 258a7dc..b933f50 100644 --- a/stem/socket.py +++ b/stem/socket.py @@ -28,7 +28,8 @@ as instances of the :class:`stem.response.ControlMessage` class.

ControllerError - Base exception raised when using the controller. |- ProtocolError - Malformed socket data. - |- InvalidRequest - Invalid request parameters. + |- InvalidRequest - Invalid request. + +- InvalidArguments - Invalid request parameters. +- SocketError - Communication with the socket failed. +- SocketClosed - Socket has been shut down. """ @@ -552,6 +553,24 @@ class ProtocolError(ControllerError): class InvalidRequest(ControllerError): "Base Exception class for invalid requests"

+class InvalidArguments(InvalidRequest): + """ + Exception class for invalid requests which contain invalid arguments. + + :var list arguments: a list of parameters which were invalid + """ + + def __init__(self, message, arguments): + """ + Initializes an InvalidArguments object. + + :param str message: error message + :param list arguments: a list of parameters which were invalid + + :returns: object of InvalidArguments class + """ + self.arguments = arguments + class SocketError(ControllerError): "Error arose while communicating with the control socket."

diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py index 479c75b..2441eac 100644 --- a/test/integ/control/controller.py +++ b/test/integ/control/controller.py @@ -145,16 +145,22 @@ class TestController(unittest.TestCase): with runner.get_tor_controller() as controller: # successful single query

- control_port = str(runner.get_tor_socket().get_port()) - torrc_path = runner.get_torrc_path() - self.assertEqual(control_port, controller.get_conf("ControlPort")) - self.assertEqual(control_port, controller.get_conf("ControlPort", "la-di-dah")) + socket = runner.get_tor_socket() + if isinstance(socket, stem.socket.ControlPort): + socket = str(socket.get_port()) + config_key = "ControlPort" + elif isinstance(socket, stem.socket.ControlSocketFile): + socket = str(socket.get_socket_path()) + config_key = "ControlSocket" + + self.assertEqual(socket, controller.get_conf(config_key)) + self.assertEqual(socket, controller.get_conf(config_key, "la-di-dah"))

# succeessful batch query

- expected = {"ControlPort": control_port} - self.assertEqual(expected, controller.get_conf(["ControlPort"])) - self.assertEqual(expected, controller.get_conf(["ControlPort"], "la-di-dah")) + expected = {config_key: socket} + self.assertEqual(expected, controller.get_conf([config_key])) + self.assertEqual(expected, controller.get_conf([config_key], "la-di-dah"))

getconf_params = set(["ControlPort", "DirPort", "DataDirectory"]) self.assertEqual(getconf_params, set(controller.get_conf(["ControlPort", diff --git a/test/unit/response/getconf.py b/test/unit/response/getconf.py index 5dd8964..a9f1cc8 100644 --- a/test/unit/response/getconf.py +++ b/test/unit/response/getconf.py @@ -20,6 +20,15 @@ BATCH_RESPONSE = """\ 250-DataDirectory=/tmp/fake dir 250 DirPort"""

+MULTIVALUE_RESPONSE = """\ +250-ControlPort=9100 +250-ExitPolicy=accept 34.3.4.5 +250-ExitPolicy=accept 3.4.53.3 +250-ExitPolicy=reject 23.245.54.3 +250-ExitPolicy=accept 34.3.4.5 +250-ExitPolicy=accept 3.4.53.3 +250 ExitPolicy=reject 23.245.54.3""" + UNRECOGNIZED_KEY_RESPONSE = "552 Unrecognized configuration key "yellowbrickroad""

INVALID_RESPONSE = """\ @@ -67,15 +76,35 @@ class TestGetConfResponse(unittest.TestCase):

self.assertEqual(expected, control_message.entries)

+ def test_multivalue_response(self): + """ + Parses a GETCONF reply containing a single key with multiple parameters. + """ + + control_message = mocking.get_message(MULTIVALUE_RESPONSE) + stem.response.convert("GETCONF", control_message) + + expected = { + "ControlPort": "9100", + "ExitPolicy": ["accept 34.3.4.5", "accept 3.4.53.3", "reject 23.245.54.3"] + } + + self.assertEqual(expected, control_message.entries) + def test_unrecognized_key_response(self): """ Parses a GETCONF reply that contains an error code with an unrecognized key. """

control_message = mocking.get_message(UNRECOGNIZED_KEY_RESPONSE) - self.assertRaises(stem.socket.InvalidRequest, stem.response.convert, "GETCONF", control_message) + self.assertRaises(stem.socket.InvalidArguments, stem.response.convert, "GETCONF", control_message) + + try: + stem.response.convert("GETCONF", control_message) + except stem.socket.InvalidArguments, exc: + self.assertEqual(exc.arguments, ["yellowbrickroad"])

- def test_invalid_multiline_content(self): + def test_invalid_content(self): """ Parses a malformed GETCONF reply that contains an invalid response code. This is a proper controller message, but malformed according to the