commit d3fe6a1ed751a60b395f80aa7b4663ab6f78d230 Author: Damian Johnson atagar@torproject.org Date: Mon May 28 16:38:30 2012 -0700

Cleaning up response classes

General refactoring of the response classes. --- stem/connection.py | 28 ++++++------ stem/response/__init__.py | 2 +- stem/response/getinfo.py | 19 ++++---- stem/response/protocolinfo.py | 81 +++++++++++++++++------------------ test/integ/response/protocolinfo.py | 6 +- test/unit/response/protocolinfo.py | 7 +++- 6 files changed, 72 insertions(+), 71 deletions(-)

diff --git a/stem/connection.py b/stem/connection.py index 34c9c68..c48a7b5 100644 --- a/stem/connection.py +++ b/stem/connection.py @@ -82,21 +82,7 @@ import stem.version import stem.util.enum import stem.util.system import stem.util.log as log - -# Methods by which a controller can authenticate to the control port. Tor gives -# a list of all the authentication methods it will accept in response to -# PROTOCOLINFO queries. -# -# NONE - No authentication required -# PASSWORD - See tor's HashedControlPassword option. Controllers must provide -# the password used to generate the hash. -# COOKIE - See tor's CookieAuthentication option. Controllers need to supply -# the contents of the cookie file. -# UNKNOWN - Tor provided one or more authentication methods that we don't -# recognize. This is probably from a new addition to the control -# protocol. - -AuthMethod = stem.util.enum.Enum("NONE", "PASSWORD", "COOKIE", "UNKNOWN") +from stem.response.protocolinfo import AuthMethod

class AuthenticationFailure(Exception): """ @@ -628,6 +614,13 @@ def get_protocolinfo(controller): the tor process running on it. If the socket is already closed then it is first reconnected.

+ According to the control spec the cookie_file is an absolute path. However, + this often is not the case (especially for the Tor Browser Bundle)... + https://trac.torproject.org/projects/tor/ticket/1101 + + If the path is relative then we'll make an attempt (which may not work) to + correct this. + Arguments: controller (stem.socket.ControlSocket or stem.control.BaseController) - tor controller connection @@ -659,6 +652,11 @@ def get_protocolinfo(controller):

stem.response.convert("PROTOCOLINFO", protocolinfo_response)

+ # attempt to expand relative cookie paths + + if protocolinfo_response.cookie_path: + stem.connection._expand_cookie_path(protocolinfo_response, stem.util.system.get_pid_by_name, "tor") + # attempt to expand relative cookie paths via the control port or socket file

if isinstance(controller, stem.socket.ControlSocket): diff --git a/stem/response/__init__.py b/stem/response/__init__.py index 92b9846..ab38747 100644 --- a/stem/response/__init__.py +++ b/stem/response/__init__.py @@ -1,7 +1,7 @@ """ Parses replies from the control socket.

-converts - translates a ControlMessage into a particular response subclass +convert - translates a ControlMessage into a particular response subclass """

__all__ = ["getinfo", "protocolinfo"] diff --git a/stem/response/getinfo.py b/stem/response/getinfo.py index dabbe12..620ca02 100644 --- a/stem/response/getinfo.py +++ b/stem/response/getinfo.py @@ -22,26 +22,25 @@ class GetInfoResponse(stem.socket.ControlMessage): # 250 OK

self.entries = {} + remaining_lines = list(self)

- lines = list(self) - - if not self.is_ok() or not lines.pop() == "OK": + if not self.is_ok() or not remaining_lines.pop() == "OK": raise stem.socket.ProtocolError("GETINFO response didn't have an OK status:\n%s" % self)

- for line in lines: - if not "=" in line: + while remaining_lines: + try: + key, value = remaining_lines.pop(0).split("=", 1) + except ValueError: raise stem.socket.ProtocolError("GETINFO replies should only contain parameter=value mappings:\n%s" % self)

- key, value = line.split("=", 1) - # if the value is a multiline value then it *must* be of the form # '<key>=\n<value>'

if "\n" in value: - if value.startswith("\n"): - value = value[1:] - else: + if not value.startswith("\n"): raise stem.socket.ProtocolError("GETINFO response contained a multiline value that didn't start with a newline:\n%s" % self) + + value = value[1:]

self.entries[key] = value

diff --git a/stem/response/protocolinfo.py b/stem/response/protocolinfo.py index 97ecfb7..7ead680 100644 --- a/stem/response/protocolinfo.py +++ b/stem/response/protocolinfo.py @@ -1,19 +1,27 @@ -import stem.connection import stem.socket import stem.version +import stem.util.enum import stem.util.log as log

+# Methods by which a controller can authenticate to the control port. Tor gives +# a list of all the authentication methods it will accept in response to +# PROTOCOLINFO queries. +# +# NONE - No authentication required +# PASSWORD - See tor's HashedControlPassword option. Controllers must provide +# the password used to generate the hash. +# COOKIE - See tor's CookieAuthentication option. Controllers need to supply +# the contents of the cookie file. +# UNKNOWN - Tor provided one or more authentication methods that we don't +# recognize. This is probably from a new addition to the control +# protocol. + +AuthMethod = stem.util.enum.Enum("NONE", "PASSWORD", "COOKIE", "UNKNOWN") + class ProtocolInfoResponse(stem.socket.ControlMessage): """ Version one PROTOCOLINFO query response.

- According to the control spec the cookie_file is an absolute path. However, - this often is not the case (especially for the Tor Browser Bundle)... - https://trac.torproject.org/projects/tor/ticket/1101 - - If the path is relative then we'll make an attempt (which may not work) to - correct this. - The protocol_version is the only mandatory data for a valid PROTOCOLINFO response, so all other values are None if undefined or empty if a collection.

@@ -34,20 +42,22 @@ class ProtocolInfoResponse(stem.socket.ControlMessage):

self.protocol_version = None self.tor_version = None + self.auth_methods = () + self.unknown_auth_methods = () self.cookie_path = None

auth_methods, unknown_auth_methods = [], [] - lines = list(self) + remaining_lines = list(self)

- if not self.is_ok() or not lines.pop() == "OK": - raise stem.socket.ProtocolError("GETINFO response didn't have an OK status:\n%s" % self) + if not self.is_ok() or not remaining_lines.pop() == "OK": + raise stem.socket.ProtocolError("PROTOCOLINFO response didn't have an OK status:\n%s" % self)

# sanity check that we're a PROTOCOLINFO response - if not lines[0].startswith("PROTOCOLINFO"): - msg = "Message is not a PROTOCOLINFO response (%s)" % self - raise stem.socket.ProtocolError(msg) + if not remaining_lines[0].startswith("PROTOCOLINFO"): + raise stem.socket.ProtocolError("Message is not a PROTOCOLINFO response:\n%s" % self)

- for line in lines: + while remaining_lines: + line = remaining_lines.pop(0) line_type = line.pop()

if line_type == "PROTOCOLINFO": @@ -56,16 +66,12 @@ class ProtocolInfoResponse(stem.socket.ControlMessage): # PIVERSION = 1*DIGIT

if line.is_empty(): - msg = "PROTOCOLINFO response's initial line is missing the protocol version: %s" % line - raise stem.socket.ProtocolError(msg) + raise stem.socket.ProtocolError("PROTOCOLINFO response's initial line is missing the protocol version: %s" % line)

- piversion = line.pop() - - if not piversion.isdigit(): - msg = "PROTOCOLINFO response version is non-numeric: %s" % line - raise stem.socket.ProtocolError(msg) - - self.protocol_version = int(piversion) + try: + self.protocol_version = int(line.pop()) + except ValueError: + raise stem.socket.ProtocolError("PROTOCOLINFO response version is non-numeric: %s" % line)

# The piversion really should be "1" but, according to the spec, tor # does not necessarily need to provide the PROTOCOLINFO version that we @@ -83,49 +89,42 @@ class ProtocolInfoResponse(stem.socket.ControlMessage):

# parse AuthMethod mapping if not line.is_next_mapping("METHODS"): - msg = "PROTOCOLINFO response's AUTH line is missing its mandatory 'METHODS' mapping: %s" % line - raise stem.socket.ProtocolError(msg) + raise stem.socket.ProtocolError("PROTOCOLINFO response's AUTH line is missing its mandatory 'METHODS' mapping: %s" % line)

for method in line.pop_mapping()[1].split(","): if method == "NULL": - auth_methods.append(stem.connection.AuthMethod.NONE) + auth_methods.append(AuthMethod.NONE) elif method == "HASHEDPASSWORD": - auth_methods.append(stem.connection.AuthMethod.PASSWORD) + auth_methods.append(AuthMethod.PASSWORD) elif method == "COOKIE": - auth_methods.append(stem.connection.AuthMethod.COOKIE) + auth_methods.append(AuthMethod.COOKIE) else: unknown_auth_methods.append(method) - message_id = "stem.connection.unknown_auth_%s" % method + message_id = "stem.response.protocolinfo.unknown_auth_%s" % method log.log_once(message_id, log.INFO, "PROTOCOLINFO response included a type of authentication that we don't recognize: %s" % method)

# our auth_methods should have a single AuthMethod.UNKNOWN entry if # any unknown authentication methods exist - if not stem.connection.AuthMethod.UNKNOWN in auth_methods: - auth_methods.append(stem.connection.AuthMethod.UNKNOWN) + if not AuthMethod.UNKNOWN in auth_methods: + auth_methods.append(AuthMethod.UNKNOWN)

# parse optional COOKIEFILE mapping (quoted and can have escapes) if line.is_next_mapping("COOKIEFILE", True, True): self.cookie_path = line.pop_mapping(True, True)[1] - - # attempt to expand relative cookie paths - stem.connection._expand_cookie_path(self, stem.util.system.get_pid_by_name, "tor") elif line_type == "VERSION": # Line format: # VersionLine = "250-VERSION" SP "Tor=" TorVersion OptArguments CRLF # TorVersion = QuotedString

if not line.is_next_mapping("Tor", True): - msg = "PROTOCOLINFO response's VERSION line is missing its mandatory tor version mapping: %s" % line - raise stem.socket.ProtocolError(msg) - - torversion = line.pop_mapping(True)[1] + raise stem.socket.ProtocolError("PROTOCOLINFO response's VERSION line is missing its mandatory tor version mapping: %s" % line)

try: - self.tor_version = stem.version.Version(torversion) + self.tor_version = stem.version.Version(line.pop_mapping(True)[1]) except ValueError, exc: raise stem.socket.ProtocolError(exc) else: - log.debug("unrecognized PROTOCOLINFO line type '%s', ignoring entry: %s" % (line_type, line)) + log.debug("Unrecognized PROTOCOLINFO line type '%s', ignoring it: %s" % (line_type, line))

self.auth_methods = tuple(auth_methods) self.unknown_auth_methods = tuple(unknown_auth_methods) diff --git a/test/integ/response/protocolinfo.py b/test/integ/response/protocolinfo.py index f5eb518..e8e8c12 100644 --- a/test/integ/response/protocolinfo.py +++ b/test/integ/response/protocolinfo.py @@ -119,7 +119,7 @@ class TestProtocolInfo(unittest.TestCase): auth_methods, auth_cookie_path = [], None

if test.runner.Torrc.COOKIE in tor_options: - auth_methods.append(stem.connection.AuthMethod.COOKIE) + auth_methods.append(stem.response.protocolinfo.AuthMethod.COOKIE) chroot_path = runner.get_chroot() auth_cookie_path = runner.get_auth_cookie_path()

@@ -127,10 +127,10 @@ class TestProtocolInfo(unittest.TestCase): auth_cookie_path = auth_cookie_path[len(chroot_path):]

if test.runner.Torrc.PASSWORD in tor_options: - auth_methods.append(stem.connection.AuthMethod.PASSWORD) + auth_methods.append(stem.response.protocolinfo.AuthMethod.PASSWORD)

if not auth_methods: - auth_methods.append(stem.connection.AuthMethod.NONE) + auth_methods.append(stem.response.protocolinfo.AuthMethod.NONE)

self.assertEqual((), protocolinfo_response.unknown_auth_methods) self.assertEqual(tuple(auth_methods), protocolinfo_response.auth_methods) diff --git a/test/unit/response/protocolinfo.py b/test/unit/response/protocolinfo.py index a17c58c..1d282ea 100644 --- a/test/unit/response/protocolinfo.py +++ b/test/unit/response/protocolinfo.py @@ -4,7 +4,6 @@ Unit tests for the stem.response.protocolinfo.ProtocolInfoResponse class.

import unittest

-from stem.connection import AuthMethod import stem.socket import stem.version import stem.util.proc @@ -12,6 +11,7 @@ import stem.util.system import stem.response import stem.response.protocolinfo import test.mocking as mocking +from stem.response.protocolinfo import AuthMethod

NO_AUTH = """250-PROTOCOLINFO 1 250-AUTH METHODS=NULL @@ -148,6 +148,8 @@ class TestProtocolInfoResponse(unittest.TestCase): succeeds and fails. """

+ # TODO: move into stem.connection unit tests? + # we need to mock both pid and cwd lookups since the general cookie # expanion works by... # - resolving the pid of the "tor" process @@ -165,6 +167,9 @@ class TestProtocolInfoResponse(unittest.TestCase):

control_message = mocking.get_message(RELATIVE_COOKIE_PATH) stem.response.convert("PROTOCOLINFO", control_message) + + stem.connection._expand_cookie_path(control_message, stem.util.system.get_pid_by_name, "tor") + self.assertEquals("/tmp/foo/tor-browser_en-US/Data/control_auth_cookie", control_message.cookie_path)

# exercise cookie expansion where both calls fail (should work, just