BIND 10 trac2595, updated. 64dd4df0a88c7007cdc22249118fbc0d3d59cd70 [2595] Address more review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 23 15:58:41 UTC 2013
The branch, trac2595 has been updated
via 64dd4df0a88c7007cdc22249118fbc0d3d59cd70 (commit)
from e34d32533ce85b2ae710f6ab45ebd187cb3520b7 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 64dd4df0a88c7007cdc22249118fbc0d3d59cd70
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Jan 23 16:56:54 2013 +0100
[2595] Address more review comments
- small updates to doc and comments
- changed __try_login to _try_login
- replaced direct calls to print() with self._print(), to override in tests
(and more easily check output)
- added some more test cases
- made cmdctl tests fail is environment vars aren't set
-----------------------------------------------------------------------
Summary of changes:
src/bin/bindctl/bindcmd.py | 140 ++++++++++++++++++---------------
src/bin/bindctl/tests/bindctl_test.py | 66 ++++++++++------
src/bin/cmdctl/cmdctl.py.in | 17 ++--
src/bin/cmdctl/tests/cmdctl_test.py | 24 ++++--
4 files changed, 145 insertions(+), 102 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/bindctl/bindcmd.py b/src/bin/bindctl/bindcmd.py
index f6727d7..738b128 100644
--- a/src/bin/bindctl/bindcmd.py
+++ b/src/bin/bindctl/bindcmd.py
@@ -124,6 +124,11 @@ class BindCmdInterpreter(Cmd):
self.csv_file_dir = pwd.getpwnam(getpass.getuser()).pw_dir + \
os.sep + '.bind10' + os.sep
+ def _print(self, *args):
+ '''Simple wrapper around calls to print that can be overridden in
+ unit tests.'''
+ print(*args)
+
def _get_session_id(self):
'''Generate one session id for the connection. '''
rand = os.urandom(16)
@@ -151,19 +156,19 @@ WARNING: Python readline module isn't available, so the command line editor
return 1
self.cmdloop()
- print('\nExit from bindctl')
+ self._print('\nExit from bindctl')
return 0
except FailToLogin as err:
# error already printed when this was raised, ignoring
return 1
except KeyboardInterrupt:
- print('\nExit from bindctl')
+ self._print('\nExit from bindctl')
return 0
except socket.error as err:
- print('Failed to send request, the connection is closed')
+ self._print('Failed to send request, the connection is closed')
return 1
except http.client.CannotSendRequest:
- print('Can not send request, the connection is busy')
+ self._print('Can not send request, the connection is busy')
return 1
def _get_saved_user_info(self, dir, file_name):
@@ -182,7 +187,8 @@ WARNING: Python readline module isn't available, so the command line editor
for row in users_info:
users.append([row[0], row[1]])
except (IOError, IndexError) as err:
- print("Error reading saved username and password from %s%s: %s" % (dir, file_name, err))
+ self._print("Error reading saved username and password "
+ "from %s%s: %s" % (dir, file_name, err))
finally:
if csvfile:
csvfile.close()
@@ -202,20 +208,22 @@ WARNING: Python readline module isn't available, so the command line editor
writer.writerow([username, passwd])
csvfile.close()
except IOError as err:
- print("Error saving user information:", err)
- print("user info file name: %s%s" % (dir, file_name))
+ self._print("Error saving user information:", err)
+ self._print("user info file name: %s%s" % (dir, file_name))
return False
return True
- def __try_login(self, username, password):
+ def _try_login(self, username, password):
'''
- Attempts to log in to bindctl by sending a POST with
+ Attempts to log in to cmdctl by sending a POST with
the given username and password.
On success of the POST (mind, not the login, only the network
operation), returns a tuple (response, data).
On failure, raises a FailToLogin exception, and prints some
information on the failure.
+ This call is essentially 'private', but made 'protected' for
+ easier testing.
'''
param = {'username': username, 'password' : password}
try:
@@ -223,11 +231,12 @@ WARNING: Python readline module isn't available, so the command line editor
data = response.read().decode()
return (response, data)
except socket.error as err:
- print("Socket error while sending login information: ", err)
+ self._print("Socket error while sending login information: ", err)
if err.errno == errno.ECONNRESET:
- print("Please check the logs of b10-cmdctl, there may be a "
- "problem accepting SSL connections, such as a "
- "permission problem on the server certificate file.")
+ self._print("Please check the logs of b10-cmdctl, there may "
+ "be a problem accepting SSL connections, such "
+ "as a permission problem on the server "
+ "certificate file.")
raise FailToLogin()
def login_to_cmdctl(self):
@@ -240,30 +249,30 @@ WARNING: Python readline module isn't available, so the command line editor
# Look at existing username/password combinations and try to log in
users = self._get_saved_user_info(self.csv_file_dir, CSV_FILE_NAME)
for row in users:
- response, data = self.__try_login(row[0], row[1])
+ response, data = self._try_login(row[0], row[1])
if response.status == http.client.OK:
# Is interactive?
if sys.stdin.isatty():
- print(data + ' login as ' + row[0])
+ self._print(data + ' login as ' + row[0])
return True
# No valid logins were found, prompt the user for a username/password
count = 0
- print('No stored password file found, please see sections '
+ self._print('No stored password file found, please see sections '
'"Configuration specification for b10-cmdctl" and "bindctl '
'command-line options" of the BIND 10 guide.')
while True:
count = count + 1
if count > 3:
- print("Too many authentication failures")
+ self._print("Too many authentication failures")
return False
username = input("Username: ")
passwd = getpass.getpass()
- response, data = self.__try_login(username, passwd)
- print(data)
+ response, data = self._try_login(username, passwd)
+ self._print(data)
if response.status == http.client.OK:
self._save_user_info(username, passwd, self.csv_file_dir,
@@ -461,25 +470,26 @@ WARNING: Python readline module isn't available, so the command line editor
pass
def do_help(self, name):
- print(CONST_BINDCTL_HELP)
+ self._print(CONST_BINDCTL_HELP)
for k in self.modules.values():
n = k.get_name()
if len(n) >= CONST_BINDCTL_HELP_INDENT_WIDTH:
- print(" %s" % n)
- print(textwrap.fill(k.get_desc(),
- initial_indent=" ",
- subsequent_indent=" " +
- " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
- width=70))
+ self._print(" %s" % n)
+ self._print(textwrap.fill(k.get_desc(),
+ initial_indent=" ",
+ subsequent_indent=" " +
+ " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+ width=70))
else:
- print(textwrap.fill("%s%s%s" %
- (k.get_name(),
- " "*(CONST_BINDCTL_HELP_INDENT_WIDTH - len(k.get_name())),
- k.get_desc()),
- initial_indent=" ",
- subsequent_indent=" " +
- " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
- width=70))
+ self._print(textwrap.fill("%s%s%s" %
+ (k.get_name(),
+ " "*(CONST_BINDCTL_HELP_INDENT_WIDTH -
+ len(k.get_name())),
+ k.get_desc()),
+ initial_indent=" ",
+ subsequent_indent=" " +
+ " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+ width=70))
def onecmd(self, line):
if line == 'EOF' or line.lower() == "quit":
@@ -654,20 +664,20 @@ WARNING: Python readline module isn't available, so the command line editor
self._validate_cmd(cmd)
self._handle_cmd(cmd)
except (IOError, http.client.HTTPException) as err:
- print('Error: ', err)
+ self._print('Error: ', err)
except BindCtlException as err:
- print("Error! ", err)
+ self._print("Error! ", err)
self._print_correct_usage(err)
except isc.cc.data.DataTypeError as err:
- print("Error! ", err)
+ self._print("Error! ", err)
except isc.cc.data.DataTypeError as dte:
- print("Error: " + str(dte))
+ self._print("Error: " + str(dte))
except isc.cc.data.DataNotFoundError as dnfe:
- print("Error: " + str(dnfe))
+ self._print("Error: " + str(dnfe))
except isc.cc.data.DataAlreadyPresentError as dape:
- print("Error: " + str(dape))
+ self._print("Error: " + str(dape))
except KeyError as ke:
- print("Error: missing " + str(ke))
+ self._print("Error: missing " + str(ke))
def _print_correct_usage(self, ept):
if isinstance(ept, CmdUnknownModuleSyntaxError):
@@ -716,7 +726,8 @@ WARNING: Python readline module isn't available, so the command line editor
module_name = identifier.split('/')[1]
if module_name != "" and (self.config_data is None or \
not self.config_data.have_specification(module_name)):
- print("Error: Module '" + module_name + "' unknown or not running")
+ self._print("Error: Module '" + module_name +
+ "' unknown or not running")
return
if cmd.command == "show":
@@ -730,7 +741,9 @@ WARNING: Python readline module isn't available, so the command line editor
#identifier
identifier += cmd.params['argument']
else:
- print("Error: unknown argument " + cmd.params['argument'] + ", or multiple identifiers given")
+ self._print("Error: unknown argument " +
+ cmd.params['argument'] +
+ ", or multiple identifiers given")
return
values = self.config_data.get_value_maps(identifier, show_all)
for value_map in values:
@@ -758,13 +771,14 @@ WARNING: Python readline module isn't available, so the command line editor
line += "(default)"
if value_map['modified']:
line += "(modified)"
- print(line)
+ self._print(line)
elif cmd.command == "show_json":
if identifier == "":
- print("Need at least the module to show the configuration in JSON format")
+ self._print("Need at least the module to show the "
+ "configuration in JSON format")
else:
data, default = self.config_data.get_value(identifier)
- print(json.dumps(data))
+ self._print(json.dumps(data))
elif cmd.command == "add":
self.config_data.add_value(identifier,
cmd.params.get('value_or_name'),
@@ -776,7 +790,7 @@ WARNING: Python readline module isn't available, so the command line editor
self.config_data.remove_value(identifier, None)
elif cmd.command == "set":
if 'identifier' not in cmd.params:
- print("Error: missing identifier or value")
+ self._print("Error: missing identifier or value")
else:
parsed_value = None
try:
@@ -793,9 +807,9 @@ WARNING: Python readline module isn't available, so the command line editor
try:
self.config_data.commit()
except isc.config.ModuleCCSessionError as mcse:
- print(str(mcse))
+ self._print(str(mcse))
elif cmd.command == "diff":
- print(self.config_data.get_local_changes())
+ self._print(self.config_data.get_local_changes())
elif cmd.command == "go":
self.go(identifier)
@@ -815,7 +829,7 @@ WARNING: Python readline module isn't available, so the command line editor
# check if exists, if not, revert and error
v,d = self.config_data.get_value(new_location)
if v is None:
- print("Error: " + identifier + " not found")
+ self._print("Error: " + identifier + " not found")
return
self.location = new_location
@@ -830,7 +844,7 @@ WARNING: Python readline module isn't available, so the command line editor
with open(command.params['filename']) as command_file:
commands = command_file.readlines()
except IOError as ioe:
- print("Error: " + str(ioe))
+ self._print("Error: " + str(ioe))
return
elif command_sets.has_command_set(command.command):
commands = command_sets.get_commands(command.command)
@@ -848,7 +862,7 @@ WARNING: Python readline module isn't available, so the command line editor
def __show_execute_commands(self, commands):
'''Prints the command list without executing them'''
for line in commands:
- print(line.strip())
+ self._print(line.strip())
def __apply_execute_commands(self, commands):
'''Applies the configuration commands from the given iterator.
@@ -869,18 +883,19 @@ WARNING: Python readline module isn't available, so the command line editor
for line in commands:
line = line.strip()
if verbose:
- print(line)
+ self._print(line)
if line.startswith('#') or len(line) == 0:
continue
elif line.startswith('!'):
if re.match('^!echo ', line, re.I) and len(line) > 6:
- print(line[6:])
+ self._print(line[6:])
elif re.match('^!verbose\s+on\s*$', line, re.I):
verbose = True
elif re.match('^!verbose\s+off$', line, re.I):
verbose = False
else:
- print("Warning: ignoring unknown directive: " + line)
+ self._print("Warning: ignoring unknown directive: " +
+ line)
else:
cmd = BindCmdParser(line)
self._validate_cmd(cmd)
@@ -891,12 +906,12 @@ WARNING: Python readline module isn't available, so the command line editor
isc.cc.data.DataNotFoundError,
isc.cc.data.DataAlreadyPresentError,
KeyError) as err:
- print('Error: ', err)
- print()
- print('Depending on the contents of the script, and which')
- print('commands it has called, there can be committed and')
- print('local changes. It is advised to check your settings,')
- print('and revert local changes with "config revert".')
+ self._print('Error: ', err)
+ self._print()
+ self._print('Depending on the contents of the script, and which')
+ self._print('commands it has called, there can be committed and')
+ self._print('local changes. It is advised to check your settings')
+ self._print(', and revert local changes with "config revert".')
def apply_cmd(self, cmd):
'''Handles a general module command'''
@@ -910,6 +925,7 @@ WARNING: Python readline module isn't available, so the command line editor
# The reply is a string containing JSON data,
# parse it, then prettyprint
if data != "" and data != "{}":
- print(json.dumps(json.loads(data), sort_keys=True, indent=4))
+ self._print(json.dumps(json.loads(data), sort_keys=True,
+ indent=4))
diff --git a/src/bin/bindctl/tests/bindctl_test.py b/src/bin/bindctl/tests/bindctl_test.py
index a7e9205..7c98439 100644
--- a/src/bin/bindctl/tests/bindctl_test.py
+++ b/src/bin/bindctl/tests/bindctl_test.py
@@ -335,6 +335,8 @@ class TestConfigCommands(unittest.TestCase):
self.tool.add_module_info(mod_info)
self.tool.config_data = FakeCCSession()
self.stdout_backup = sys.stdout
+ self.printed_messages = []
+ self.tool._print = self.store_print
def test_precmd(self):
def update_all_modules_info():
@@ -347,6 +349,12 @@ class TestConfigCommands(unittest.TestCase):
precmd('EOF')
self.assertRaises(socket.error, precmd, 'continue')
+ def store_print(self, *args):
+ '''Method to override _print in BindCmdInterpreter.
+ Instead of printing the values, appends the argument tuple
+ to the list in self.printed_messages'''
+ self.printed_messages.append(" ".join(map(str, args)))
+
def test_try_login(self):
# Make sure __try_login raises the correct exception
# upon failure of either send_POST or the read() on the
@@ -358,20 +366,35 @@ class TestConfigCommands(unittest.TestCase):
raise socket.error("test error")
self.tool.send_POST = send_POST_raiseImmediately
- self.assertRaises(FailToLogin,
- self.tool._BindCmdInterpreter__try_login,
- "foo", "bar")
-
- def send_POST_raiseOnRead(self, params):
- class MyResponse:
- def read(self):
- raise socket.error("read error")
- return MyResponse()
-
- self.tool.send_POST = send_POST_raiseOnRead
- self.assertRaises(FailToLogin,
- self.tool._BindCmdInterpreter__try_login,
- "foo", "bar")
+ self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
+ self.assertIn('Socket error while sending login information: test error',
+ self.printed_messages)
+ self.assertEqual(1, len(self.printed_messages))
+
+ def create_send_POST_raiseOnRead(exception):
+ '''Create a replacement send_POST() method that raises
+ the given exception when read() is called on the value
+ returned from send_POST()'''
+ def send_POST_raiseOnRead(self, params):
+ class MyResponse:
+ def read(self):
+ raise exception
+ return MyResponse()
+ return send_POST_raiseOnRead
+
+ self.tool.send_POST =\
+ create_send_POST_raiseOnRead(socket.error("read error"))
+ self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
+ self.assertIn('Socket error while sending login information: read error',
+ self.printed_messages)
+ self.assertEqual(2, len(self.printed_messages))
+
+ # any other exception should be passed through
+ self.tool.send_POST =\
+ create_send_POST_raiseOnRead(ImportError())
+ self.assertRaises(ImportError, self.tool._try_login, "foo", "bar")
+ self.assertEqual(2, len(self.printed_messages))
+
finally:
self.tool.send_POST = orig_send_POST
@@ -388,29 +411,22 @@ class TestConfigCommands(unittest.TestCase):
self.tool.conn.sock = FakeSocket()
self.tool.conn.sock.close()
- # validate log message for socket.err
- socket_err_output = io.StringIO()
- sys.stdout = socket_err_output
self.assertEqual(1, self.tool.run())
# First few lines may be some kind of heading, or a warning that
# Python readline is unavailable, so we do a sub-string check.
self.assertIn("Failed to send request, the connection is closed",
- socket_err_output.getvalue())
-
- socket_err_output.close()
+ self.printed_messages)
+ self.assertEqual(1, len(self.printed_messages))
# validate log message for http.client.CannotSendRequest
- cannot_send_output = io.StringIO()
- sys.stdout = cannot_send_output
self.assertEqual(1, self.tool.run())
# First few lines may be some kind of heading, or a warning that
# Python readline is unavailable, so we do a sub-string check.
self.assertIn("Can not send request, the connection is busy",
- cannot_send_output.getvalue())
-
- cannot_send_output.close()
+ self.printed_messages)
+ self.assertEqual(2, len(self.printed_messages))
def test_apply_cfg_command_int(self):
self.tool.location = '/'
diff --git a/src/bin/cmdctl/cmdctl.py.in b/src/bin/cmdctl/cmdctl.py.in
index 8e4a73a..11bc63b 100755
--- a/src/bin/cmdctl/cmdctl.py.in
+++ b/src/bin/cmdctl/cmdctl.py.in
@@ -289,10 +289,9 @@ class CommandControl():
if key == 'version':
continue
elif key in ['key_file', 'cert_file']:
- # TODO, only check whether the file exist, is a
- # file, and is readable
- # further check need to be done: eg. whether
- # the private/certificate is valid.
+ # TODO: we only check whether the file exist, is a
+ # file, and is readable; but further check need to be done:
+ # eg. whether the private/certificate is valid.
path = new_config[key]
try:
check_file(path)
@@ -549,16 +548,16 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
certfile = cert,
keyfile = key,
ssl_version = ssl.PROTOCOL_SSLv23)
+ # Return here (if control leaves this blocks it will raise an
+ # error)
return ssl_sock
except ssl.SSLError as err:
logger.error(CMDCTL_SSL_SETUP_FAILURE_USER_DENIED, err)
- self.close_request(sock)
- # raise socket error to finish the request
- raise socket.error
except (CmdctlException, IOError) as cce:
logger.error(CMDCTL_SSL_SETUP_FAILURE_READING_CERT, cce)
- # raise socket error to finish the request
- raise socket.error
+ self.close_request(sock)
+ # raise socket error to finish the request
+ raise socket.error
def get_request(self):
'''Get client request socket and wrap it in SSL context. '''
diff --git a/src/bin/cmdctl/tests/cmdctl_test.py b/src/bin/cmdctl/tests/cmdctl_test.py
index f660bf1..cab89f8 100644
--- a/src/bin/cmdctl/tests/cmdctl_test.py
+++ b/src/bin/cmdctl/tests/cmdctl_test.py
@@ -22,13 +22,13 @@ import sys
from cmdctl import *
import isc.log
-SRC_FILE_PATH = '..' + os.sep
-if 'CMDCTL_SRC_PATH' in os.environ:
- SRC_FILE_PATH = os.environ['CMDCTL_SRC_PATH'] + os.sep
+assert 'CMDCTL_SRC_PATH' in os.environ,\
+ "Please run this test with 'make check'"
+SRC_FILE_PATH = os.environ['CMDCTL_SRC_PATH'] + os.sep
-BUILD_FILE_PATH = '..' + os.sep
-if 'CMDCTL_BUILD_PATH' in os.environ:
- BUILD_FILE_PATH = os.environ['CMDCTL_BUILD_PATH'] + os.sep
+assert 'CMDCTL_BUILD_PATH' in os.environ,\
+ "Please run this test with 'make check'"
+BUILD_FILE_PATH = os.environ['CMDCTL_BUILD_PATH'] + os.sep
# Rewrite the class for unittest.
class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
@@ -535,6 +535,18 @@ class TestSecureHTTPServer(unittest.TestCase):
BUILD_FILE_PATH + 'cmdctl-certfile.pem')
self.assertIsInstance(ssl_sock, ssl.SSLSocket)
+ # wrap_socket can also raise IOError, which should be caught and
+ # handled like the other errors.
+ # Force this by temporarily disabling our own file checks
+ orig_check_func = self.server._check_key_and_cert
+ try:
+ self.server._check_key_and_cert = lambda x,y: None
+ self.assertRaises(socket.error,
+ self.server._wrap_socket_in_ssl_context,
+ sock,
+ 'no_such_file', 'no_such_file')
+ finally:
+ self.server._check_key_and_cert = orig_check_func
class TestFuncNotInClass(unittest.TestCase):
def test_check_port(self):
More information about the bind10-changes
mailing list