[tor-commits] [stem/master] Revisions for GETCONF handling
atagar at torproject.org
atagar at torproject.org
Wed Jul 4 21:34:20 UTC 2012
commit c09b80ef912bb590a37e0d4533a34bbba14cabf9
Author: Damian Johnson <atagar at torproject.org>
Date: Wed Jul 4 14:25:32 2012 -0700
Revisions for GETCONF handling
These changes have gone through several review iterations so there wasn't much
to tweak with it. Only substantial twiddling on my part was...
* The super() method is new to me, and looks like it's the preferred method
(especially in Python 3.x when it's syntactically much nicer). Swapped all of
our constructors to use super() rather than Parent.__init__(self, args). This
also forced me to fix some base classes that weren't inheriting from object.
* Moved _case_insensitive_lookup() helper into controller module rather than
having a separate util.
* Removed the "GETCONF reply doesn't match the parameters that we requested."
check since this'll always produce an error if tor adds new context sensitive
options (my bad).
* The get_conf_map() method wouldn't accept context sensitive options like
HiddenServiceDir.
* The get_conf_map()'s default value is what we should return if the lookup
fails, not the mapped values (ie, get_conf_map("blarg", {}) should give an
empty map).
* Integ test was missing a requirement check that we could make a control
socket, causing the RUN_NONE target to fail.
---
stem/connection.py | 12 ++--
stem/control.py | 147 ++++++++++++++++++------------
stem/descriptor/__init__.py | 2 +-
stem/descriptor/extrainfo_descriptor.py | 6 +-
stem/descriptor/reader.py | 10 +-
stem/descriptor/server_descriptor.py | 7 +-
stem/response/__init__.py | 9 +-
stem/response/getconf.py | 5 +-
stem/socket.py | 6 +-
stem/util/__init__.py | 2 +-
stem/util/control.py | 24 -----
stem/util/log.py | 2 +-
test/integ/control/controller.py | 13 ++--
13 files changed, 127 insertions(+), 118 deletions(-)
diff --git a/stem/connection.py b/stem/connection.py
index e22a2d0..52037a3 100644
--- a/stem/connection.py
+++ b/stem/connection.py
@@ -799,7 +799,7 @@ class AuthenticationFailure(Exception):
"""
def __init__(self, message, auth_response = None):
- Exception.__init__(self, message)
+ super(AuthenticationFailure, self).__init__(message)
self.auth_response = auth_response
class UnrecognizedAuthMethods(AuthenticationFailure):
@@ -810,7 +810,7 @@ class UnrecognizedAuthMethods(AuthenticationFailure):
"""
def __init__(self, message, unknown_auth_methods):
- AuthenticationFailure.__init__(self, message)
+ super(UnrecognizedAuthMethods, self).__init__(message)
self.unknown_auth_methods = unknown_auth_methods
class IncorrectSocketType(AuthenticationFailure):
@@ -844,7 +844,7 @@ class CookieAuthFailed(AuthenticationFailure):
"""
def __init__(self, message, cookie_path, is_safecookie, auth_response = None):
- AuthenticationFailure.__init__(self, message, auth_response)
+ super(CookieAuthFailed, self).__init__(message, auth_response)
self.is_safecookie = is_safecookie
self.cookie_path = cookie_path
@@ -866,7 +866,7 @@ class AuthChallengeFailed(CookieAuthFailed):
"""
def __init__(self, message, cookie_path):
- CookieAuthFailed.__init__(self, message, cookie_path, True)
+ super(AuthChallengeFailed, self).__init__(message, cookie_path, True)
class AuthChallengeUnsupported(AuthChallengeFailed):
"""
@@ -881,7 +881,7 @@ class UnrecognizedAuthChallengeMethod(AuthChallengeFailed):
"""
def __init__(self, message, cookie_path, authchallenge_method):
- AuthChallengeFailed.__init__(self, message, cookie_path)
+ super(UnrecognizedAuthChallengeMethod, self).__init__(message, cookie_path)
self.authchallenge_method = authchallenge_method
class AuthSecurityFailure(AuthChallengeFailed):
@@ -907,7 +907,7 @@ class NoAuthCookie(MissingAuthInfo):
"""
def __init__(self, message, is_safecookie):
- AuthenticationFailure.__init__(self, message)
+ super(NoAuthCookie, self).__init__(message)
self.is_safecookie = is_safecookie
# authentication exceptions ordered as per the authenticate function's pydocs
diff --git a/stem/control.py b/stem/control.py
index 61dd453..1b830a5 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -31,7 +31,6 @@ interacting at a higher level.
from __future__ import with_statement
-import re
import time
import Queue
import threading
@@ -56,6 +55,17 @@ State = stem.util.enum.Enum("INIT", "RESET", "CLOSED")
UNDEFINED = "<Undefined_ >"
+# Configuration options that are fetched by a special key. The keys are
+# lowercase to make case insensetive lookups easier.
+
+MAPPED_CONFIG_KEYS = {
+ "hiddenservicedir": "HiddenServiceOptions",
+ "hiddenserviceport": "HiddenServiceOptions",
+ "hiddenserviceversion": "HiddenServiceOptions",
+ "hiddenserviceauthorizeclient": "HiddenServiceOptions",
+ "hiddenserviceoptions": "HiddenServiceOptions"
+}
+
# TODO: The Thread's isAlive() method and theading's currentThread() was
# changed to the more conventional is_alive() and current_thread() in python
# 2.6 and above. We should use that when dropping python 2.5 compatability.
@@ -406,14 +416,6 @@ class Controller(BaseController):
BaseController and provides a more user friendly API for library users.
"""
- _mapped_config_keys = {
- "hiddenservicedir": "hiddenserviceoptions",
- "hiddenserviceport": "hiddenserviceoptions",
- "hiddenserviceversion": "hiddenserviceoptions",
- "hiddenserviceauthorizeclient": "hiddenserviceoptions",
- "hiddenserviceoptions": "hiddenserviceoptions"
- }
-
def from_port(control_addr = "127.0.0.1", control_port = 9051):
"""
Constructs a ControlPort based Controller.
@@ -464,7 +466,7 @@ class Controller(BaseController):
* default if one was provided and our call failed
:raises:
- :class:`stem.socket.ControllerError` if the call fails, and we weren't provided a default response
+ :class:`stem.socket.ControllerError` if the call fails and we weren't provided a default response
:class:`stem.socket.InvalidArguments` if the 'param' requested was invalid
"""
@@ -549,12 +551,11 @@ class Controller(BaseController):
provided a default then that's returned as if the GETCONF option is undefined
or if the call fails for any reason (invalid configuration option, error
response, control port closed, initiated, etc). If the configuration key
- consists of whitespace only, None is returned, unless a default value is given.
+ consists of whitespace only, None is returned unless a default value is given.
- :param str param: GETCONF option to be queried
+ :param str param: configuration option to be queried
:param object default: response if the query fails
- :param bool multiple: if True, the value(s) provided are lists of all returned values,
- otherwise this just provides the first value
+ :param bool multiple: if True, the value(s) provided are lists of all returned values, otherwise this just provides the first value
:returns:
Response depends upon how we were called as follows...
@@ -568,25 +569,27 @@ class Controller(BaseController):
:class:`stem.socket.InvalidArguments` if the configuration option requested was invalid
"""
- if re.match("^\s*$", param):
- if default != UNDEFINED: return default
- else: return
- # automagically change the requested parameter if it's context sensitive
- # and cannot be returned on it's own.
- param = param.lower()
- entries = self.get_conf_map(self._mapped_config_keys.get(param, param), default, multiple)
- if param in self._mapped_config_keys:
- return entries[stem.util.control.case_insensitive_lookup(entries, param)]
- else: return entries[param]
+ # Config options are case insensitive and don't contain whitespace. Using
+ # strip so the following check will catch whitespace-only params.
+
+ param = param.lower().strip()
+
+ if not param:
+ return default if default != UNDEFINED else None
+
+ entries = self.get_conf_map(param, default, multiple)
+
+ if entries == default: return default
+ else: return _case_insensitive_lookup(entries, param)
def get_conf_map(self, param, default = UNDEFINED, multiple = True):
"""
- Queries the control socket for the values of given configuration options and
- provides a mapping of the keys to the values. If provided a default then
- that's returned as if the GETCONF option is undefined or if the call fails
- for any reason (invalid configuration option, error response, control port
- closed, initiated, etc). Configuration keys that contain only whitespace are
- ignored.
+ Queries the control socket for the values of given configuration options
+ and provides a mapping of the keys to the values. If provided a default
+ then that's returned if the GETCONF option is undefined or if the call
+ fails for any reason (invalid configuration option, error response, control
+ port closed, initiated, etc). Configuration keys that are empty or contain
+ only whitespace are ignored.
There's three use cases for GETCONF:
- a single value is provided
@@ -603,17 +606,18 @@ class Controller(BaseController):
The get_conf() and get_conf_map() functions both try to account for these
special mappings, so queried like get_conf("HiddenServicePort") should
- behave as you'd expect.
+ behave as you'd expect. This method, however, simply returns whatever Tor
+ provides so get_conf_map("HiddenServicePort") will give the same response
+ as get_conf_map("HiddenServiceOptions").
- :param str,list param: GETCONF option(s) to be queried
+ :param str,list param: configuration option(s) to be queried
:param object default: response if the query fails
- :param bool multiple: if True, the value(s) provided are lists of all returned values,
- otherwise this just provides the first value
+ :param bool multiple: if True, the value(s) provided are lists of all returned values,otherwise this just provides the first value
:returns:
Response depends upon how we were called as follows...
- * dict of 'config key => value' mappings, the value is a list if 'multiple' is True and otherwise is a str of just the first value
+ * dict of 'config key => value' mappings, the value is a list if 'multiple' is True and a str of just the first value otherwise
* default if one was provided and our call failed
:raises:
@@ -626,37 +630,62 @@ class Controller(BaseController):
try:
# remove strings which contain only whitespace
- param = filter(lambda x: not re.match("^\s*$", x), param)
+ param = filter(lambda entry: entry.strip(), param)
if param == []: return {}
- response = self.msg("GETCONF %s" % ' '.join(param))
- stem.response.convert("GETCONF", response)
+ # translate context sensitive options
+ lookup_param = set([MAPPED_CONFIG_KEYS.get(entry, entry) for entry in param])
- lookup = stem.util.control.case_insensitive_lookup
- # map the entries back to the parameters given so the capitalization
- # matches. Note: when the same configuration key is provided multiple
- # times, this uses the first one and ignores the rest.
- for key in response.entries:
- if not key.lower() in self._mapped_config_keys:
- response.entries[lookup(param, key)] = response.entries[key]
- if key != lookup(param, key): del response.entries[key]
+ response = self.msg("GETCONF %s" % ' '.join(lookup_param))
+ stem.response.convert("GETCONF", response)
- requested_params = set(param)
- reply_params = set(response.entries.keys())
+ # Maps the entries back to the parameters that the user requested so the
+ # capitalization matches (ie, if they request "exitpolicy" then that
+ # should be the key rather than "ExitPolicy"). When the same
+ # configuration key is provided multiple times this determines the case
+ # based on the first and ignores the rest.
+ #
+ # This retains the tor provided camel casing of MAPPED_CONFIG_KEYS
+ # entries since the user didn't request those by their key, so we can't
+ # be sure what they wanted.
- # if none of the requested parameters are context sensitive and if the
- # parameters received don't match the parameters requested
- if not set(self._mapped_config_keys.values()) & requested_params and requested_params != reply_params:
- requested_label = ", ".join(requested_params)
- reply_label = ", ".join(reply_params)
-
- raise stem.socket.ProtocolError("GETCONF reply doesn't match the parameters that we requested. Queried '%s' but got '%s'." % (requested_label, reply_label))
+ for key in response.entries:
+ if not key.lower() in MAPPED_CONFIG_KEYS.values():
+ user_expected_key = _case_insensitive_lookup(param, key)
+
+ if key != user_expected_key:
+ response.entries[user_expected_key] = response.entries[key]
+ del response.entries[key]
- if not multiple:
+ if multiple:
+ return response.entries
+ else:
return dict([(entry[0], entry[1][0]) for entry in response.entries.items()])
- return response.entries
-
except stem.socket.ControllerError, exc:
- if default != UNDEFINED: return dict([(p, default) for p in param])
+ if default != UNDEFINED: return default
else: raise exc
+def _case_insensitive_lookup(entries, key):
+ """
+ Makes a case insensitive lookup within a list or dictionary, providing the
+ first matching entry that we come across.
+
+ :param list,dict entries: list or dictionary to be searched
+ :param str key: entry or key value to look up
+
+ :returns: case insensitive match
+
+ :raises: ValueError if no such value exists
+ """
+
+ if isinstance(entries, dict):
+ for k, v in entries.items():
+ if k.lower() == key.lower():
+ return v
+ else:
+ for entry in entries:
+ if entry.lower() == key.lower():
+ return entry
+
+ raise ValueError("key '%s' doesn't exist in dict: %s" % (key, entries))
+
diff --git a/stem/descriptor/__init__.py b/stem/descriptor/__init__.py
index 21dad37..4bddbc5 100644
--- a/stem/descriptor/__init__.py
+++ b/stem/descriptor/__init__.py
@@ -95,7 +95,7 @@ def parse_file(path, descriptor_file):
raise TypeError("Unable to determine the descriptor's type. filename: '%s', first line: '%s'" % (filename, first_line))
-class Descriptor:
+class Descriptor(object):
"""
Common parent for all types of descriptors.
"""
diff --git a/stem/descriptor/extrainfo_descriptor.py b/stem/descriptor/extrainfo_descriptor.py
index 979c679..b1c7b09 100644
--- a/stem/descriptor/extrainfo_descriptor.py
+++ b/stem/descriptor/extrainfo_descriptor.py
@@ -281,7 +281,7 @@ class ExtraInfoDescriptor(stem.descriptor.Descriptor):
:raises: ValueError if the contents is malformed and validate is True
"""
- stem.descriptor.Descriptor.__init__(self, raw_contents)
+ super(ExtraInfoDescriptor, self).__init__(raw_contents)
self.nickname = None
self.fingerprint = None
@@ -709,7 +709,7 @@ class RelayExtraInfoDescriptor(ExtraInfoDescriptor):
self.signature = None
self._digest = None
- ExtraInfoDescriptor.__init__(self, raw_contents, validate)
+ super(RelayExtraInfoDescriptor, self).__init__(raw_contents, validate)
def digest(self):
if self._digest is None:
@@ -750,7 +750,7 @@ class BridgeExtraInfoDescriptor(ExtraInfoDescriptor):
self._digest = None
self.transport = None
- ExtraInfoDescriptor.__init__(self, raw_contents, validate)
+ super(BridgeExtraInfoDescriptor, self).__init__(raw_contents, validate)
def digest(self):
return self._digest
diff --git a/stem/descriptor/reader.py b/stem/descriptor/reader.py
index 8d4a462..ed07c88 100644
--- a/stem/descriptor/reader.py
+++ b/stem/descriptor/reader.py
@@ -97,7 +97,7 @@ class AlreadyRead(FileSkipped):
"Already read a file with this 'last modified' timestamp or later."
def __init__(self, last_modified, last_modified_when_read):
- FileSkipped.__init__(self)
+ super(AlreadyRead, self).__init__()
self.last_modified = last_modified
self.last_modified_when_read = last_modified_when_read
@@ -105,7 +105,7 @@ class ParsingFailure(FileSkipped):
"File contents could not be parsed as descriptor data."
def __init__(self, parsing_exception):
- FileSkipped.__init__(self)
+ super(ParsingFailure, self).__init__()
self.exception = parsing_exception
class UnrecognizedType(FileSkipped):
@@ -115,21 +115,21 @@ class UnrecognizedType(FileSkipped):
"""
def __init__(self, mime_type):
- FileSkipped.__init__(self)
+ super(UnrecognizedType, self).__init__()
self.mime_type = mime_type
class ReadFailed(FileSkipped):
"An IOError occured while trying to read the file."
def __init__(self, read_exception):
- FileSkipped.__init__(self)
+ super(ReadFailed, self).__init__()
self.exception = read_exception
class FileMissing(ReadFailed):
"File does not exist."
def __init__(self):
- ReadFailed.__init__(self, None)
+ super(FileMissing, self).__init__(None)
def load_processed_files(path):
"""
diff --git a/stem/descriptor/server_descriptor.py b/stem/descriptor/server_descriptor.py
index 765a5ba..fc15a3d 100644
--- a/stem/descriptor/server_descriptor.py
+++ b/stem/descriptor/server_descriptor.py
@@ -188,7 +188,7 @@ class ServerDescriptor(stem.descriptor.Descriptor):
:raises: ValueError if the contents is malformed and validate is True
"""
- stem.descriptor.Descriptor.__init__(self, raw_contents)
+ super(ServerDescriptor, self).__init__(raw_contents)
self.nickname = None
self.fingerprint = None
@@ -561,7 +561,7 @@ class RelayDescriptor(ServerDescriptor):
self.signature = None
self._digest = None
- ServerDescriptor.__init__(self, raw_contents, validate, annotations)
+ super(RelayDescriptor, self).__init__(raw_contents, validate, annotations)
# if we have a fingerprint then checks that our fingerprint is a hash of
# our signing key
@@ -648,7 +648,8 @@ class BridgeDescriptor(ServerDescriptor):
self.address_alt = []
self._digest = None
self._scrubbing_issues = None
- ServerDescriptor.__init__(self, raw_contents, validate, annotations)
+
+ super(BridgeDescriptor, self).__init__(raw_contents, validate, annotations)
def digest(self):
return self._digest
diff --git a/stem/response/__init__.py b/stem/response/__init__.py
index 192e313..155f262 100644
--- a/stem/response/__init__.py
+++ b/stem/response/__init__.py
@@ -48,12 +48,12 @@ def convert(response_type, message, **kwargs):
an in-place conversion of the message from being a ControlMessage to a
subclass for its response type. Recognized types include...
- * GETINFO
- * GETCONF
+ * **\*** GETINFO
+ * **\*** GETCONF
* PROTOCOLINFO
* AUTHCHALLENGE
- If the response_type isn't recognized then this is leaves it alone.
+ **\*** can raise a :class:`stem.socket.InvalidArguments` exception
:param str response_type: type of tor response to convert to
:param stem.response.ControlMessage message: message to be converted
@@ -61,8 +61,7 @@ def convert(response_type, message, **kwargs):
:raises:
* :class:`stem.socket.ProtocolError` the message isn't a proper response of that type
- * :class:`stem.socket.InvalidArguments` the arguments given as input are
- invalid. Raised when converting GETINFO or GETCONF requests
+ * :class:`stem.socket.InvalidArguments` the arguments given as input 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 30b7f68..627e7bc 100644
--- a/stem/response/getconf.py
+++ b/stem/response/getconf.py
@@ -8,7 +8,7 @@ class GetConfResponse(stem.response.ControlMessage):
Note that configuration parameters won't match what we queried for if it's one
of the special mapping options (ex. "HiddenServiceOptions").
- :var dict entries: mapping between the config parameter (string) and their values (list of strings)
+ :var dict entries: mapping between the config parameter (str) and their values (list of str)
"""
def _parse_message(self):
@@ -41,6 +41,9 @@ class GetConfResponse(stem.response.ControlMessage):
if line.is_next_mapping(quoted = False):
key, value = line.split("=", 1) # TODO: make this part of the ControlLine?
elif line.is_next_mapping(quoted = True):
+ # TODO: doesn't seem to occure yet in practice...
+ # https://trac.torproject.org/6172
+
key, value = line.pop_mapping(True).items()[0]
else:
key, value = (line.pop(), None)
diff --git a/stem/socket.py b/stem/socket.py
index 62312a8..7d40056 100644
--- a/stem/socket.py
+++ b/stem/socket.py
@@ -45,7 +45,7 @@ import stem.response
import stem.util.enum
import stem.util.log as log
-class ControlSocket:
+class ControlSocket(object):
"""
Wrapper for a socket connection that speaks the Tor control protocol. To the
better part this transparently handles the formatting for sending and
@@ -278,7 +278,7 @@ class ControlPort(ControlSocket):
:raises: :class:`stem.socket.SocketError` if connect is True and we're unable to establish a connection
"""
- ControlSocket.__init__(self)
+ super(ControlPort, self).__init__()
self._control_addr = control_addr
self._control_port = control_port
@@ -326,7 +326,7 @@ class ControlSocketFile(ControlSocket):
:raises: :class:`stem.socket.SocketError` if connect is True and we're unable to establish a connection
"""
- ControlSocket.__init__(self)
+ super(ControlSocketFile, self).__init__()
self._socket_path = socket_path
if connect: self.connect()
diff --git a/stem/util/__init__.py b/stem/util/__init__.py
index 75a2da4..5f6bb24 100644
--- a/stem/util/__init__.py
+++ b/stem/util/__init__.py
@@ -2,5 +2,5 @@
Utility functions used by the stem library.
"""
-__all__ = ["conf", "connection", "control", "enum", "log", "proc", "system", "term", "tor_tools"]
+__all__ = ["conf", "connection", "enum", "log", "proc", "system", "term", "tor_tools"]
diff --git a/stem/util/control.py b/stem/util/control.py
deleted file mode 100644
index 5ea2efe..0000000
--- a/stem/util/control.py
+++ /dev/null
@@ -1,24 +0,0 @@
-"""
-Helper functions utilized by the controller classes.
-
-**Module Overview:**
-
-::
- case_insensitive_lookup - does case insensitive lookups on python dictionaries
-"""
-
-def case_insensitive_lookup(lst, key):
- """
- Returns the first value equal to key in lst while ignoring case.
-
- :param list lst: list of strings
- :param str key: string to be looked up
-
- :returns: case-insensitive equivalent of key in lst
-
- :raises: ValueError if such a key doesn't exist
- """
-
- for i in lst:
- if i.lower() == key.lower():
- return i
diff --git a/stem/util/log.py b/stem/util/log.py
index c2eb817..38815ba 100644
--- a/stem/util/log.py
+++ b/stem/util/log.py
@@ -152,7 +152,7 @@ class LogBuffer(logging.Handler):
"""
def __init__(self, runlevel):
- logging.Handler.__init__(self, level = logging_level(runlevel))
+ super(LogBuffer, self).__init__(level = logging_level(runlevel))
self.formatter = logging.Formatter(
fmt = '%(asctime)s [%(levelname)s] %(message)s',
datefmt = '%D %H:%M:%S')
diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py
index a9eee1b..55f7210 100644
--- a/test/integ/control/controller.py
+++ b/test/integ/control/controller.py
@@ -140,6 +140,8 @@ class TestController(unittest.TestCase):
Exercises GETCONF with valid and invalid queries.
"""
+ if test.runner.require_control(self): return
+
runner = test.runner.get_runner()
with runner.get_tor_controller() as controller:
@@ -160,19 +162,18 @@ class TestController(unittest.TestCase):
self.assertEqual(expected, controller.get_conf_map([config_key]))
self.assertEqual(expected, controller.get_conf_map([config_key], "la-di-dah"))
- getconf_params = set(["ControlPORT", "dirport", "datadirectory"])
- self.assertEqual(getconf_params, set(controller.get_conf_map(["ControlPORT",
- "dirport", "datadirectory"], multiple=False)))
+ request_params = ["ControlPORT", "dirport", "datadirectory"]
+ reply_params = controller.get_conf_map(request_params, multiple=False).keys()
+ self.assertEqual(set(request_params), set(reply_params))
# non-existant option(s)
self.assertRaises(stem.socket.InvalidArguments, controller.get_conf, "blarg")
self.assertEqual("la-di-dah", controller.get_conf("blarg", "la-di-dah"))
self.assertRaises(stem.socket.InvalidArguments, controller.get_conf_map, "blarg")
- self.assertEqual({"blarg": "la-di-dah"}, controller.get_conf_map("blarg", "la-di-dah"))
+ self.assertEqual("la-di-dah", controller.get_conf_map("blarg", "la-di-dah"))
self.assertRaises(stem.socket.InvalidRequest, controller.get_conf_map, ["blarg", "huadf"], multiple = True)
- self.assertEqual({"erfusdj": "la-di-dah", "afiafj": "la-di-dah"},
- controller.get_conf_map(["erfusdj", "afiafj"], "la-di-dah", multiple = True))
+ self.assertEqual("la-di-dah", controller.get_conf_map(["erfusdj", "afiafj"], "la-di-dah", multiple = True))
# multivalue configuration keys
nodefamilies = [("abc", "xyz", "pqrs"), ("mno", "tuv", "wxyz")]
More information about the tor-commits
mailing list