1. Merge branches/likun-value-check to trunk.(for ticket 201: add value/type check for commands sent to cmdctl.)\n 2. Apply the patch for ticket 193:msgq: real type data isn't encoded and decoded properly. \n3. The change had been reviewed by Jelte.

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

src/bin/bindctl/bindcmd.py

@@ -49,7 +49,7 @@ try:
 except ImportError:
     my_readline = sys.stdin.readline
 
-
+CONFIG_MODULE_NAME = 'config'
 CONST_BINDCTL_HELP = """
 usage: <module name> <command name> [param1 = value1 [, param2 = value2]]
 Type Tab character to get the hint of module/command/parameters.
@@ -228,7 +228,8 @@ class BindCmdInterpreter(Cmd):
             for arg in command["command_args"]:
                 param = ParamInfo(name = arg["item_name"],
                                   type = arg["item_type"],
-                                  optional = bool(arg["item_optional"]))
+                                  optional = bool(arg["item_optional"]),
+                                  param_spec = arg)
                 if ("item_default" in arg):
                     param.default = arg["item_default"]
                 cmd.add_param(param)
@@ -301,12 +302,20 @@ class BindCmdInterpreter(Cmd):
                 if not name in params and not param_nr in params:
                     raise CmdMissParamSyntaxError(cmd.module, cmd.command, name)
                 param_nr += 1
+        
+        # Convert parameter value according parameter spec file.
+        # Ignore check for commands belongs to module 'config'
+        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])
 
+    
     def _handle_cmd(self, cmd):
         '''Handle a command entered by the user'''
         if cmd.command == "help" or ("help" in cmd.params.keys()):
             self._handle_help(cmd)
-        elif cmd.module == "config":
+        elif cmd.module == CONFIG_MODULE_NAME:
             self.apply_config_cmd(cmd)
         else:
             self.apply_cmd(cmd)
@@ -357,7 +366,7 @@ class BindCmdInterpreter(Cmd):
                 else:                       
                     hints = self._get_param_startswith(cmd.module, cmd.command,
                                                        text)
-                    if cmd.module == "config":
+                    if cmd.module == CONFIG_MODULE_NAME:
                         # grm text has been stripped of slashes...
                         my_text = self.location + "/" + cur_line.rpartition(" ")[2]
                         list = self.config_data.get_config_item_list(my_text.rpartition("/")[0], True)
@@ -435,6 +444,9 @@ class BindCmdInterpreter(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)
             
             
     def _print_correct_usage(self, ept):        

src/bin/bindctl/bindctl-source.py.in

@@ -29,7 +29,7 @@ __version__ = 'Bindctl'
 
 def prepare_config_commands(tool):
     '''Prepare fixed commands for local configuration editing'''
-    module = ModuleInfo(name = "config", desc = "Configuration commands")
+    module = ModuleInfo(name = CONFIG_MODULE_NAME, desc = "Configuration commands")
     cmd = CommandInfo(name = "show", desc = "Show configuration")
     param = ParamInfo(name = "identifier", type = "string", optional=True)
     cmd.add_param(param)

src/bin/bindctl/moduleinfo.py

@@ -33,17 +33,21 @@ PARAM_NODE_NAME = 'param'
 
 class ParamInfo:
     """One parameter of one command.
-    Each command parameter has four attributes:
-    parameter name, parameter type, parameter value, and parameter description
+    Each command parameter has five attributes:
+    parameter name, parameter type, parameter value,
+    parameter description and paramter's spec(got from
+    module spec file). 
     """
     def __init__(self, name, desc = '', type = STRING_TYPE, 
-                 optional = False, value = '', default_value = ''):
+                 optional = False, value = '', default_value = '', 
+                 param_spec = None):
         self.name = name
         self.type = type
         self.value = value
         self.default_value = default_value                           
         self.desc = desc
         self.is_optional = optional
+        self.param_spec = param_spec
     
     def __str__(self):        
         return str("\t%s <type: %s> \t(%s)" % (self.name, self.type, self.desc))

src/bin/cmdctl/cmdctl.py.in

@@ -263,8 +263,6 @@ class CommandControl():
         Return rcode, dict.
         rcode = 0: dict is the correct returned value.
         rcode > 0: dict is : { 'error' : 'error reason' }
-
-        TODO. add check for parameters.
         '''
 
         # core module ConfigManager does not have a specification file
@@ -273,18 +271,12 @@ class CommandControl():
 
         if module_name not in self.module_spec.keys():
             return 1, {'error' : 'unknown module'}
-
-        cmd_valid = False
-        # todo: make validate_command() in ModuleSpec class
-        commands = self.module_spec[module_name]["commands"]
-        for cmd in commands:
-            if cmd['command_name'] == command_name:
-                cmd_valid = True
-                break
-
-        if not cmd_valid:
-            return 1, {'error' : 'unknown command'}
-
+       
+        spec_obj = isc.config.module_spec.ModuleSpec(self.module_spec[module_name], False)
+        errors = []
+        if not spec_obj.validate_command(command_name, params, errors):
+            return 1, {'error': errors[0]}
+        
         return self.send_command(module_name, command_name, params)
 
     def send_command(self, module_name, command_name, params = None):

src/lib/config/testdata/spec27.spec

@@ -0,0 +1,121 @@
+{
+  "module_spec": {
+    "module_name": "Spec27",
+    "commands": [
+    {
+        'command_name': 'cmd1',
+        "command_description": "command_for_unittest",
+        'command_args': [ 
+          {
+            "item_name": "value1",
+            "item_type": "integer",
+            "item_optional": False,
+            "item_default": 9
+          },
+          { "item_name": "value2",
+            "item_type": "real",
+            "item_optional": False,
+            "item_default": 9.9
+          },
+          { "item_name": "value3",
+            "item_type": "boolean",
+            "item_optional": False,
+            "item_default": False
+          },
+          { "item_name": "value4",
+            "item_type": "string",
+            "item_optional": False,
+            "item_default": "default_string"
+          },
+          { "item_name": "value5",
+            "item_type": "list",
+            "item_optional": False,
+            "item_default": [ "a", "b" ],
+            "list_item_spec": {
+              "item_name": "list_element",
+              "item_type": "integer",
+              "item_optional": False,
+              "item_default": 8
+            }
+          },
+          { "item_name": "value6",
+            "item_type": "map",
+            "item_optional": False,
+            "item_default": {},
+            "map_item_spec": [
+              { "item_name": "v61",
+                "item_type": "string",
+                "item_optional": False,
+                "item_default": "def"
+              },
+              { "item_name": "v62",
+                "item_type": "boolean",
+                "item_optional": False,
+                "item_default": False
+              }
+            ]
+          },
+          { "item_name": "value7",
+            "item_type": "list",
+            "item_optional": True,
+            "item_default": [ ],
+            "list_item_spec": {
+              "item_name": "list_element",
+              "item_type": "any",
+              "item_optional": True
+            }
+          },
+          { "item_name": "value8",
+            "item_type": "list",
+            "item_optional": True,
+            "item_default": [ ],
+            "list_item_spec": {
+              "item_name": "list_element",
+              "item_type": "map",
+              "item_optional": True,
+              "item_default": { "a": "b" },
+              "map_item_spec": [
+                { "item_name": "a",
+                  "item_type": "string",
+                  "item_optional": True,
+                  "item_default": "empty"
+                }
+              ]
+            }
+          },
+          { "item_name": "value9",
+            "item_type": "map",
+            "item_optional": False,
+            "item_default": {},
+            "map_item_spec": [
+              { "item_name": "v91",
+                "item_type": "string",
+                "item_optional": False,
+                "item_default": "def"
+              },
+              { "item_name": "v92",
+                "item_type": "map",
+                "item_optional": False,
+                "item_default": {},
+                "map_item_spec": [
+                  { "item_name": "v92a",
+                    "item_type": "string",
+                    "item_optional": False,
+                    "item_default": "Hello"
+                  } ,
+                  {
+                    "item_name": "v92b",
+                    "item_type": "integer",
+                    "item_optional": False,
+                    "item_default": 47806
+                  }
+                ]
+              }
+            ]
+          }
+        ]
+      }
+    ]
+  }
+}
+

src/lib/python/isc/cc/message.py

@@ -26,6 +26,7 @@ _ITEM_LIST = 0x03
 _ITEM_NULL = 0x04
 _ITEM_BOOL = 0x05
 _ITEM_INT  = 0x06
+_ITEM_REAL = 0x07
 _ITEM_UTF8 = 0x08
 _ITEM_MASK = 0x0f
 
@@ -77,6 +78,10 @@ def _pack_int(item):
     """Pack an integer and its type/length prefix."""
     return (_encode_length_and_type(bytes(str(item), 'utf-8'), _ITEM_INT))
 
+def _pack_real(item):
+    """Pack an integer and its type/length prefix."""
+    return (_encode_length_and_type(bytes(str(item), 'utf-8'), _ITEM_REAL))
+
 def _pack_array(item):
     """Pack a list (array) and its type/length prefix."""
     return (_encode_length_and_type(_encode_array(item), _ITEM_LIST))
@@ -98,6 +103,8 @@ def _encode_item(item):
         return (_pack_bool(item))
     elif type(item) == int:
         return (_pack_int(item))
+    elif type(item) == float:
+        return (_pack_real(item))
     elif type(item) == dict:
         return (_pack_hash(item))
     elif type(item) == list:
@@ -186,6 +193,8 @@ def _decode_item(data):
         value = _decode_bool(item)
     elif item_type == _ITEM_INT:
         value = _decode_int(item)
+    elif item_type == _ITEM_REAL:
+        value = _decode_real(item)
     elif item_type == _ITEM_UTF8:
         value = str(item, 'utf-8')
     elif item_type == _ITEM_HASH:
@@ -205,6 +214,9 @@ def _decode_bool(data):
 def _decode_int(data):
     return int(str(data, 'utf-8'))
 
+def _decode_real(data):
+    return float(str(data, 'utf-8'))
+
 def _decode_hash(data):
     ret = {}
     while len(data) > 0:

src/lib/python/isc/config/config_data.py

@@ -53,6 +53,51 @@ def check_type(spec_part, value):
         # todo: check types of map contents too
         raise isc.cc.data.DataTypeError(str(value) + " is not a map")
 
+def convert_type(spec_part, value):
+    """Convert the give value(type is string) according specification 
+    part relevant for the value. Raises an isc.cc.data.DataTypeError 
+    exception if conversion failed.
+    """
+    if type(spec_part) == dict and 'item_type' in spec_part:
+        data_type = spec_part['item_type']
+    else:
+        raise isc.cc.data.DataTypeError(str("Incorrect specification part for type convering"))
+   
+    try:
+        if data_type == "integer":
+            return int(value)
+        elif data_type == "real":
+            return float(value)
+        elif data_type == "boolean":
+            return str.lower(str(value)) != 'false'
+        elif data_type == "string":
+            return str(value)
+        elif data_type == "list":
+            ret = []
+            if type(value) == list:
+                for item in value:    
+                    ret.append(convert_type(spec_part['list_item_spec'], item))
+            elif type(value) == str:    
+                value = value.split(',')
+                for item in value:    
+                    sub_value = item.split()
+                    for sub_item in sub_value:
+                        ret.append(convert_type(spec_part['list_item_spec'], sub_item))
+
+            if ret == []:
+                raise isc.cc.data.DataTypeError(str(value) + " is not a list")
+
+            return ret
+        elif data_type == "map":
+            return dict(value)
+            # todo: check types of map contents too
+        else:
+            return value
+    except ValueError as err:
+        raise isc.cc.data.DataTypeError(str(err))
+    except TypeError as err:
+        raise isc.cc.data.DataTypeError(str(err))
+
 def find_spec_part(element, identifier):
     """find the data definition for the given identifier
        returns either a map with 'item_name' etc, or a list of those"""

src/lib/python/isc/config/module_spec.py

@@ -83,6 +83,27 @@ class ModuleSpec:
             errors.append("No config_data specification")
             return False
 
+    def validate_command(self, cmd_name, cmd_params, errors = None):
+        '''Check whether the given piece of command conforms to this 
+        command definition. If so, it reutrns True. If not, it will 
+        return False. If errors is given, and is an array, a string
+        describing the error will be appended to it. The current version
+        stops as soon as there is one error.
+           cmd_name is command name to be validated, cmd_params includes 
+        command's parameters needs to be validated. cmd_params must 
+        be a map, with the format like:
+        {param1_name: param1_value, param2_name: param2_value}
+        '''
+        cmd_spec = self.get_commands_spec()
+        if not cmd_spec:
+            return False
+
+        for cmd in cmd_spec:
+            if cmd['command_name'] != cmd_name:
+                continue
+            return _validate_spec_list(cmd['command_args'], True, cmd_params, errors)
+
+        return False
 
     def get_module_name(self):
         """Returns a string containing the name of the module as

src/lib/python/isc/config/tests/config_data_test.py

@@ -95,6 +95,71 @@ class TestConfigData(unittest.TestCase):
 
         self.assertRaises(isc.cc.data.DataTypeError, check_type, config_spec, 1)
 
+    def test_convert_type(self):
+        config_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec22.spec").get_config_spec()
+        spec_part = find_spec_part(config_spec, "value1")
+        self.assertEqual(1, convert_type(spec_part, '1'))
+        self.assertEqual(2, convert_type(spec_part, 2.1))
+        self.assertEqual(2, convert_type(spec_part, '2'))
+        self.assertEqual(3, convert_type(spec_part, '3'))
+        self.assertEqual(1, convert_type(spec_part, True))
+
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ 1, 2 ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+        
+        spec_part = find_spec_part(config_spec, "value2")
+        self.assertEqual(1.1, convert_type(spec_part, '1.1'))
+        self.assertEqual(123.0, convert_type(spec_part, '123'))
+        self.assertEqual(1.0, convert_type(spec_part, True))
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ 1, 2 ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+
+        spec_part = find_spec_part(config_spec, "value3")
+        self.assertEqual(True, convert_type(spec_part, 'True'))
+        self.assertEqual(False, convert_type(spec_part, 'False'))
+        self.assertEqual(True, convert_type(spec_part, 1))
+        self.assertEqual(True, convert_type(spec_part, 1.1))
+        self.assertEqual(True, convert_type(spec_part, 'a'))
+        self.assertEqual(True, convert_type(spec_part, [1, 2]))
+        self.assertEqual(True, convert_type(spec_part, {'a' : 1}))
+
+        spec_part = find_spec_part(config_spec, "value4")
+        self.assertEqual('asdf', convert_type(spec_part, "asdf"))
+        self.assertEqual('1', convert_type(spec_part, 1))
+        self.assertEqual('1.1', convert_type(spec_part, 1.1))
+        self.assertEqual('True', convert_type(spec_part, True))
+        
+        spec_part = find_spec_part(config_spec, "value5")
+        self.assertEqual([1, 2], convert_type(spec_part, '1, 2'))
+        self.assertEqual([1, 2, 3], convert_type(spec_part, '1 2  3'))
+        self.assertEqual([1, 2, 3,4], convert_type(spec_part, '1 2  3, 4'))
+        self.assertEqual([1], convert_type(spec_part, [1,]))
+        self.assertEqual([1,2], convert_type(spec_part, [1,2]))
+        self.assertEqual([1,2], convert_type(spec_part, ['1', '2']))
+
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, 1.1)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, True)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "a", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+
+        spec_part = find_spec_part(config_spec, "value7")
+        self.assertEqual(['1', '2'], convert_type(spec_part, '1, 2'))
+        self.assertEqual(['1', '2', '3'], convert_type(spec_part, '1 2  3'))
+        self.assertEqual(['1', '2', '3','4'], convert_type(spec_part, '1 2  3, 4'))
+        self.assertEqual([1], convert_type(spec_part, [1,]))
+        self.assertEqual([1,2], convert_type(spec_part, [1,2]))
+        self.assertEqual(['1','2'], convert_type(spec_part, ['1', '2']))
+
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, 1.1)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, True)
+        self.assertEqual(['a'], convert_type(spec_part, "a"))
+        self.assertEqual(['a', 'b'], convert_type(spec_part, ["a", "b" ]))
+        self.assertEqual([1, 'b'], convert_type(spec_part, [1, "b" ]))
+
     def test_find_spec_part(self):
         config_spec = self.cd.get_module_spec().get_config_spec()
         spec_part = find_spec_part(config_spec, "item1")

src/lib/python/isc/config/tests/module_spec_test.py

@@ -94,6 +94,24 @@ class TestModuleSpec(unittest.TestCase):
         self.assertEqual(True, self.validate_data("spec22.spec", "data22_7.data"))
         self.assertEqual(False, self.validate_data("spec22.spec", "data22_8.data"))
 
+    def validate_command_params(self, specfile_name, datafile_name, cmd_name):
+        dd = self.read_spec_file(specfile_name);
+        data_file = open(self.spec_file(datafile_name))
+        data_str = data_file.read()
+        params = isc.cc.data.parse_value_str(data_str)
+        return dd.validate_command(cmd_name, params)
+
+    def test_command_validation(self):
+        self.assertEqual(True, self.validate_command_params("spec27.spec", "data22_1.data", 'cmd1'))
+        self.assertEqual(False, self.validate_command_params("spec27.spec", "data22_2.data",'cmd1'))
+        self.assertEqual(False, self.validate_command_params("spec27.spec", "data22_3.data", 'cmd1'))
+        self.assertEqual(False, self.validate_command_params("spec27.spec", "data22_4.data", 'cmd1'))
+        self.assertEqual(False, self.validate_command_params("spec27.spec", "data22_5.data", 'cmd1'))
+        self.assertEqual(True, self.validate_command_params("spec27.spec", "data22_6.data", 'cmd1'))
+        self.assertEqual(True, self.validate_command_params("spec27.spec", "data22_7.data", 'cmd1'))
+        self.assertEqual(False, self.validate_command_params("spec27.spec", "data22_8.data", 'cmd1'))
+        self.assertEqual(False, self.validate_command_params("spec27.spec", "data22_8.data", 'cmd2'))
+
     def test_init(self):
         self.assertRaises(ModuleSpecError, ModuleSpec, 1)
         module_spec = isc.config.module_spec_from_file(self.spec_file("spec1.spec"), False)