Merge trac220 to trunk: 1. Make bindctl's command parser only do minimal check. Parameter value can be a sequence of non-space characters, or a string surrounded by quotation marks(these marks can be a part of the value string in escaped form). Make error message be more friendly.(if there is some error in parameter's value, the parameter name will provided) 2. Refactor function login_to_cmdctl() in class BindCmdInterpreter. Avoid using Exception to catch all exceptions.

git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@2356 e5f2f494-b856-4b98-b285-d166d9295462

ChangeLog

@@ -1,3 +1,14 @@
+  67.  [func]	    zhanglikun
+	Make bindctl's command parser only do minimal check. Parameter 
+	value can be a sequence of non-space characters, or a string 
+	surrounded by quotation	marks(these marks can be a part of the 
+	value string in escaped form). Make	error message be more 
+	friendly.(if there is some error in parameter's value, the 
+	parameter name will be provided). Refactor function login_to_cmdctl() 
+	in class BindCmdInterpreter: avoid using Exception to catch all 
+	exceptions.
+	(Trac #220, svn r2356)
+
   66.  [bug]        each
 	Check for duplicate RRsets before inserting data into a message
 	section; this, among other things, will prevent multiple copies

src/bin/bindctl/bindcmd.py

@@ -36,6 +36,9 @@ import getpass
 from hashlib import sha1
 import csv
 import ast
+import pwd
+import getpass
+import traceback
 
 try:
     from collections import OrderedDict
@@ -49,6 +52,8 @@ try:
 except ImportError:
     my_readline = sys.stdin.readline
 
+CSV_FILE_NAME = 'default_user.csv'
+FAIL_TO_CONNECT_WITH_CMDCTL = "Fail to connect with b10-cmdctl module, is it running?"
 CONFIG_MODULE_NAME = 'config'
 CONST_BINDCTL_HELP = """
 usage: <module name> <command name> [param1 = value1 [, param2 = value2]]
@@ -96,14 +101,59 @@ class BindCmdInterpreter(Cmd):
         '''Parse commands inputted from user and send them to cmdctl. '''
         try:
             if not self.login_to_cmdctl():
-                return False
+                return 
 
             # Get all module information from cmd-ctrld
             self.config_data = isc.config.UIModuleCCSession(self)
             self._update_commands()
             self.cmdloop()
+        except FailToLogin as err:
+            print(err)
+            print(FAIL_TO_CONNECT_WITH_CMDCTL)
+            traceback.print_exc()
         except KeyboardInterrupt:
-            return True
+            print('\nExit from bindctl')
+
+    def _get_saved_user_info(self, dir, file_name):
+        ''' Read all the available username and password pairs saved in 
+        file(path is "dir + file_name"), Return value is one list of elements
+        ['name', 'password'], If get information failed, empty list will be 
+        returned.'''
+        if (not dir) or (not os.path.exists(dir)):
+            return []
+
+        try:
+            csvfile = None
+            users = []
+            csvfile = open(dir + file_name)
+            users_info = csv.reader(csvfile)
+            for row in users_info:
+                users.append([row[0], row[1]])
+        except (IOError, IndexError) as e:
+            pass
+        finally:
+            if csvfile:
+                csvfile.close()
+            return users
+
+    def _save_user_info(self, username, passwd, dir, file_name):
+        ''' Save username and password in file "dir + file_name"
+        If it's saved properly, return True, or else return False. '''
+        try:
+            if not os.path.exists(dir):
+                os.mkdir(dir, 0o700)
+
+            csvfilepath = dir + file_name 
+            csvfile = open(csvfilepath, 'w')
+            os.chmod(csvfilepath, 0o600)
+            writer = csv.writer(csvfile)
+            writer.writerow([username, passwd])
+            csvfile.close()
+        except Exception as e:
+            print(e, "\nCannot write %s%s; default user is not stored" % (dir, file_name))
+            return False
+
+        return True
 
     def login_to_cmdctl(self):
         '''Login to cmdctl with the username and password inputted 
@@ -112,33 +162,21 @@ class BindCmdInterpreter(Cmd):
         time, username and password saved in 'default_user.csv' will be
         used first.
         '''
-        csvfile = None
-        bsuccess = False
-        try:
-            cvsfilepath = ""
-            if ('HOME' in os.environ):
-                cvsfilepath = os.environ['HOME']
-                cvsfilepath += os.sep + '.bind10' + os.sep
-            cvsfilepath += 'default_user.csv'
-            csvfile = open(cvsfilepath)
-            users = csv.reader(csvfile)
-            for row in users:
-                param = {'username': row[0], 'password' : row[1]}
+        csv_file_dir = pwd.getpwnam(getpass.getuser()).pw_dir
+        csv_file_dir += os.sep + '.bind10' + os.sep
+        users = self._get_saved_user_info(csv_file_dir, CSV_FILE_NAME)
+        for row in users:
+            param = {'username': row[0], 'password' : row[1]}
+            try:
                 response = self.send_POST('/login', param)
                 data = response.read().decode()
-                if response.status == http.client.OK:
-                    print(data + ' login as ' + row[0] )
-                    bsuccess = True
-                    break
-        except IOError as e:
-            pass
-        except Exception as e:
-            print(e)
-        finally:
-            if csvfile:
-                csvfile.close()
-            if bsuccess:
-                return True
+            except socket.error:
+                traceback.print_exc()
+                raise FailToLogin()
+
+            if response.status == http.client.OK:
+                print(data + ' login as ' + row[0] )
+                return True 
 
         count = 0
         print("[TEMP MESSAGE]: username :root  password :bind10")
@@ -151,34 +189,18 @@ class BindCmdInterpreter(Cmd):
             username = input("Username:")
             passwd = getpass.getpass()
             param = {'username': username, 'password' : passwd}
-            response = self.send_POST('/login', param)
-            data = response.read().decode()
-            print(data)
-            
+            try:
+                response = self.send_POST('/login', param)
+                data = response.read().decode()
+                print(data)
+            except socket.error as e:
+                traceback.print_exc()
+                raise FailToLogin()
+
             if response.status == http.client.OK:
-                cvsfilepath = ""
-                try:
-                    if ('HOME' in os.environ):
-                        cvsfilepath = os.environ['HOME']
-                        cvsfilepath += os.sep + '.bind10' + os.sep
-                        if not os.path.exists(cvsfilepath):
-                                os.mkdir(cvsfilepath, 0o700)
-                    else:
-                        print("Cannot determine location of $HOME. Not storing default user")
-                        return True
-                    cvsfilepath += 'default_user.csv'
-                    csvfile = open(cvsfilepath, 'w')
-                    os.chmod(cvsfilepath, 0o600)
-                    writer = csv.writer(csvfile)
-                    writer.writerow([username, passwd])
-                    csvfile.close()
-                except Exception as e:
-                    # just not store it
-                    print("Cannot write ~/.bind10/default_user.csv; default user is not stored")
-                    print(e)
+                self._save_user_info(username, passwd, csv_file_dir, CSV_FILE_NAME)
                 return True
 
-
     def _update_commands(self):
         '''Update the commands of all modules. '''
         for module_name in self.config_data.get_config_item_list():
@@ -308,8 +330,11 @@ class BindCmdInterpreter(Cmd):
         if cmd.module != CONFIG_MODULE_NAME:
             for param_name in cmd.params:
                 param_spec = command_info.get_param_with_name(param_name).param_spec
-                cmd.params[param_name] = isc.config.config_data.convert_type(param_spec, cmd.params[param_name])
-
+                try:
+                    cmd.params[param_name] = isc.config.config_data.convert_type(param_spec, cmd.params[param_name])
+                except isc.cc.data.DataTypeError as e:
+                    raise isc.cc.data.DataTypeError('Invalid parameter value for \"%s\", the type should be \"%s\" \n' 
+                                                     % (param_name, param_spec['item_type']) + str(e))
     
     def _handle_cmd(self, cmd):
         '''Handle a command entered by the user'''
@@ -441,13 +466,14 @@ class BindCmdInterpreter(Cmd):
             cmd = BindCmdParse(line)
             self._validate_cmd(cmd)
             self._handle_cmd(cmd)
-        except BindCtlException as e:
-            print("Error! ", e)
-            self._print_correct_usage(e)
-        except isc.cc.data.DataTypeError as e:
-            print("Error! ", e)
-            self._print_correct_usage(e)
-            
+        except (IOError, http.client.HTTPException) as err:
+            print('Error!', err)
+            print(FAIL_TO_CONNECT_WITH_CMDCTL)
+        except BindCtlException as err:
+            print("Error! ", err)
+            self._print_correct_usage(err)
+        except isc.cc.data.DataTypeError as err:
+            print("Error! ", err)
             
     def _print_correct_usage(self, ept):        
         if isinstance(ept, CmdUnknownModuleSyntaxError):
@@ -556,7 +582,7 @@ class BindCmdInterpreter(Cmd):
         if (len(cmd.params) != 0):
             cmd_params = json.dumps(cmd.params)
 
-        print("send the message to cmd-ctrld")        
+        print("send the command to cmd-ctrld")        
         reply = self.send_POST(url, cmd.params)
         data = reply.read().decode()
         print("received reply:", data)

src/bin/bindctl/cmdparse.py

@@ -24,15 +24,19 @@ except ImportError:
     from bindctl.mycollections import OrderedDict
 
 param_name_str = "^\s*(?P<param_name>[\w]+)\s*=\s*"
-param_value_str = "(?P<param_value>[\w\.:/-]+)"
-param_value_with_quota_str = "[\"\'](?P<param_value>[\w\.:, /-]+)[\"\']"
+
+# The value string can be a sequence without space or comma 
+# characters, or a string surroundedby quotation marks(such marks
+# can be part of string in an escaped form)
+#param_value_str  = "(?P<param_value>[\"\'].+?(?<!\\\)[\"\']|[^\'\"][^, ]+)"
+param_value_str  = "(?P<param_value>[^\'\" ][^, ]+)"
+param_value_with_quota_str  = "[\"\'](?P<param_value>.+?)(?<!\\\)[\"\']"
 next_params_str = "(?P<blank>\s*)(?P<comma>,?)(?P<next_params>.*)$"
 
 PARAM_WITH_QUOTA_PATTERN = re.compile(param_name_str + 
-                                      param_value_with_quota_str +
+                                      param_value_with_quota_str + 
                                       next_params_str)
 PARAM_PATTERN = re.compile(param_name_str + param_value_str + next_params_str)
-                           
 # Used for module and command name
 NAME_PATTERN = re.compile("^\s*(?P<name>[\w]+)(?P<blank>\s*)(?P<others>.*)$")
 
@@ -98,7 +102,6 @@ class BindCmdParse:
                 
             groups = PARAM_PATTERN.match(param_text) or \
                      PARAM_WITH_QUOTA_PATTERN.match(param_text)
-            
             if not groups:
                 # ok, fill in the params in the order entered
                 params = re.findall("([^\" ]+|\".*\")", param_text)

src/bin/bindctl/exception.py

@@ -115,3 +115,10 @@ class CmdMissParamSyntaxError(CmdSyntaxError):
     def __str__(self):
         return str("Parameter '%s' is missed for command '%s' of module '%s'" % 
                    (self.param, self.command, self.module))
+
+
+class FailToLogin(BindCtlException):
+    def __str__(self):
+        return "Fail to login to cmdctl"
+
+

src/bin/bindctl/tests/bindctl_test.py

@@ -16,6 +16,7 @@
 
 import unittest
 import isc.cc.data
+import os
 from bindctl import cmdparse
 from bindctl import bindcmd
 from bindctl.moduleinfo import *
@@ -50,12 +51,31 @@ class TestCmdLex(unittest.TestCase):
             assert cmd.params["zone_name"] == "cnnic.cn"
             assert cmd.params["file"] == "cnnic.cn.file"
             assert cmd.params["master"] == '1.1.1.1'
+
+    def testCommandWithParamters_2(self):
+        '''Test whether the parameters in key=value can be parsed properly.'''
+        cmd = cmdparse.BindCmdParse('zone cmd name = 1:34::2')
+        self.assertEqual(cmd.params['name'], '1:34::2')
+
+        cmd = cmdparse.BindCmdParse('zone cmd name = 1\"\'34**&2 value=44\"\'\"')
+        self.assertEqual(cmd.params['name'], '1\"\'34**&2')
+        self.assertEqual(cmd.params['value'], '44\"\'\"')
+
+        cmd = cmdparse.BindCmdParse('zone cmd name = 1\"\'34**&2 ,value=  44\"\'\"')
+        self.assertEqual(cmd.params['name'], '1\"\'34**&2')
+        self.assertEqual(cmd.params['value'], '44\"\'\"')
             
+        cmd = cmdparse.BindCmdParse('zone cmd name =  1\'34**&2value=44\"\'\" value = \"==============\'')
+        self.assertEqual(cmd.params['name'], '1\'34**&2value=44\"\'\"')
+        self.assertEqual(cmd.params['value'], '==============')
+
+        cmd = cmdparse.BindCmdParse('zone cmd name =    \"1234, 567890 \" value ==&*/')
+        self.assertEqual(cmd.params['name'], '1234, 567890 ')
+        self.assertEqual(cmd.params['value'], '=&*/')
             
     def testCommandWithListParam(self):
-            cmd = cmdparse.BindCmdParse("zone set zone_name='cnnic.cn', master='1.1.1.1, 2.2.2.2'")
-            assert cmd.params["master"] == '1.1.1.1, 2.2.2.2'            
-        
+        cmd = cmdparse.BindCmdParse("zone set zone_name='cnnic.cn', master='1.1.1.1, 2.2.2.2'")
+        assert cmd.params["master"] == '1.1.1.1, 2.2.2.2'            
         
     def testCommandWithHelpParam(self):
         cmd = cmdparse.BindCmdParse("zone add help")
@@ -217,8 +237,32 @@ class TestNameSequence(unittest.TestCase):
             assert self.random_names[i] == cmd_names[i+1]
             assert self.random_names[i] == module_names[i+1]
             i = i + 1
-        
     
+class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
+    def __init__(self):
+        pass
+
+class TestBindCmdInterpreter(unittest.TestCase):
+
+    def _create_invalid_csv_file(self, csvfilename):
+        import csv
+        csvfile = open(csvfilename, 'w')
+        writer = csv.writer(csvfile)
+        writer.writerow(['name1'])
+        writer.writerow(['name2'])
+        csvfile.close()
+
+    def test_get_saved_user_info(self):
+        cmd = FakeBindCmdInterpreter()
+        users = cmd._get_saved_user_info('/notexist', 'cvs_file.cvs')
+        self.assertEqual([], users)
+        
+        csvfilename = 'csv_file.csv'
+        self._create_invalid_csv_file(csvfilename)
+        users = cmd._get_saved_user_info('./', csvfilename)
+        self.assertEqual([], users)
+        os.remove(csvfilename)
+
 if __name__== "__main__":
     unittest.main()