also prevent unknown data to get into internal systems (by adding a check for unspecified data in validate_config calls)

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac202@3765 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/config/module_spec.cc

@@ -330,13 +330,32 @@ bool
 ModuleSpec::validate_spec_list(ConstElementPtr spec, ConstElementPtr data,
                                const bool full, ElementPtr errors) const
 {
+    bool validated = true;
     std::string cur_item_name;
     BOOST_FOREACH(ConstElementPtr cur_spec_el, spec->listValue()) {
         if (!validate_spec(cur_spec_el, data, full, errors)) {
-            return (false);
+            validated = false;
         }
     }
-    return (true);
+
+    typedef std::pair<std::string, ConstElementPtr> maptype;
+    
+    BOOST_FOREACH(maptype m, data->mapValue()) {
+        bool found = false;
+        BOOST_FOREACH(ConstElementPtr cur_spec_el, spec->listValue()) {
+            if (cur_spec_el->get("item_name")->stringValue().compare(m.first) == 0) {
+                found = true;
+            }
+        }
+        if (!found) {
+            validated = false;
+            if (errors) {
+                errors->add(Element::create("Unknown item " + m.first));
+            }
+        }
+    }
+
+    return (validated);
 }
 
 }

src/lib/config/tests/module_spec_unittests.cc

@@ -166,8 +166,13 @@ TEST(ModuleSpec, DataValidation) {
     EXPECT_TRUE(data_test(dd, "data22_6.data"));
     EXPECT_TRUE(data_test(dd, "data22_7.data"));
     EXPECT_FALSE(data_test(dd, "data22_8.data"));
+    EXPECT_FALSE(data_test(dd, "data22_9.data"));
 
     ElementPtr errors = Element::createList();
     EXPECT_FALSE(data_test_with_errors(dd, "data22_8.data", errors));
     EXPECT_EQ("[ \"Type mismatch\" ]", errors->str());
+
+    errors = Element::createList();
+    EXPECT_FALSE(data_test_with_errors(dd, "data22_9.data", errors));
+    EXPECT_EQ("[ \"Unknown item value_does_not_exist\" ]", errors->str());
 }

src/lib/config/tests/testdata/data22_8.data

@@ -5,5 +5,6 @@
     "value4": "foo",
     "value5": [ 1, 2, 3 ],
     "value6": { "v61": "bar", "v62": true },
-    "value8": [ { "a": "d" }, { "a": 1 } ]
+    "value8": [ { "a": "d" }, { "a": 1 } ],
+    "value9": { "v91": "hi", "v92": { "v92a": "Hi", "v92b": 3 } }
 }

src/lib/config/tests/testdata/data22_9.data

@@ -0,0 +1,11 @@
+{
+    "value1": 1,
+    "value2": 2.3,
+    "value3": true,
+    "value4": "foo",
+    "value5": [ 1, 2, 3 ],
+    "value6": { "v61": "bar", "v62": true },
+    "value8": [ { "a": "d" }, { "a": "e" } ],
+    "value9": { "v91": "hi", "v92": { "v92a": "Hi", "v92b": 3 } },
+    "value_does_not_exist": 1
+}

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

@@ -224,7 +224,7 @@ class ModuleCCSession(ConfigData):
                     if not self._config_handler:
                         answer = create_answer(2, self._module_name + " has no config handler")
                     elif not self.get_module_spec().validate_config(False, new_config, errors):
-                        answer = create_answer(1, " ".join(errors))
+                        answer = create_answer(1, ", ".join(errors))
                     else:
                         isc.cc.data.remove_identical(new_config, self.get_local_config())
                         answer = self._config_handler(new_config)
@@ -422,7 +422,16 @@ class UIModuleCCSession(MultiConfigData):
         """Commit all local changes, send them through b10-cmdctl to
            the configuration manager"""
         if self.get_local_changes():
-            self._conn.send_POST('/ConfigManager/set_config', [ self.get_local_changes() ])
-            # todo: check result
-            self.request_current_config()
-            self.clear_local_changes()
+            response = self._conn.send_POST('/ConfigManager/set_config',
+                                            [ self.get_local_changes() ])
+            answer = isc.cc.data.parse_value_str(response.read().decode())
+            # answer is either an empty dict (on success), or one
+            # containing errors
+            if answer == {}:
+                self.request_current_config()
+                self.clear_local_changes()
+            elif "error" in answer:
+                print("Error: " + answer["error"])
+                print("Configuration not committed")
+            else:
+                raise ModuleCCSessionError("Unknown format of answer in commit(): " + str(answer))

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

@@ -328,7 +328,24 @@ def _validate_spec(spec, full, data, errors):
         return True
 
 def _validate_spec_list(module_spec, full, data, errors):
+    # we do not return immediately, there may be more errors
+    # so we keep a boolean to keep track if we found errors
+    validated = True
+
+    # check if the known items are correct
     for spec_item in module_spec:
         if not _validate_spec(spec_item, full, data, errors):
-            return False
-    return True
+            validated = False
+
+    # check if there are items in our data that are not in the
+    # specification
+    for item_name in data:
+        found = False
+        for spec_item in module_spec:
+            if spec_item["item_name"] == item_name:
+                found = True
+        if not found:
+            if errors != None:
+                errors.append("unknown item " + item_name)
+            validated = False
+    return validated

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

@@ -290,7 +290,7 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_ok)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 2 }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 2 })
         fake_session.group_sendmsg(cmd, 'Spec2')
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
@@ -303,12 +303,12 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_err)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 'aaa' }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 'aaa' })
         fake_session.group_sendmsg(cmd, 'Spec2')
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
         self.assertEqual(len(fake_session.message_queue), 1)
-        self.assertEqual({'result': [1, 'just an error']},
+        self.assertEqual({'result': [1, 'aaa should be an integer']},
                          fake_session.get_message('Spec2', None))
         
     def test_check_command5(self):
@@ -316,12 +316,12 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_exc)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 'aaa' }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 'aaa' })
         fake_session.group_sendmsg(cmd, 'Spec2')
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
         self.assertEqual(len(fake_session.message_queue), 1)
-        self.assertEqual({'result': [1, 'just an exception']},
+        self.assertEqual({'result': [1, 'aaa should be an integer']},
                          fake_session.get_message('Spec2', None))
         
     def test_check_command6(self):
@@ -416,7 +416,7 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = self.create_session("spec2.spec", None, None, fake_session)
         mccs.set_config_handler(self.my_config_handler_ok)
         self.assertEqual(len(fake_session.message_queue), 0)
-        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'Spec2': { 'item1': 2 }})
+        cmd = isc.config.ccsession.create_command(isc.config.ccsession.COMMAND_CONFIG_UPDATE, { 'item1': 2 })
         self.assertEqual(len(fake_session.message_queue), 0)
         env = { 'group':'Spec2', 'from':None }
         mccs.check_command_without_recvmsg(cmd, env)
@@ -560,6 +560,14 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 0)
         
 
+class fakeData:
+    def decode(self):
+        return "{}";
+
+class fakeAnswer:
+    def read(self):
+        return fakeData();
+
 class fakeUIConn():
     def __init__(self):
         self.get_answers = {}
@@ -581,7 +589,7 @@ class fakeUIConn():
         if name in self.post_answers:
             return self.post_answers[name]
         else:
-            return None
+            return fakeAnswer()
     
 
 class TestUIModuleCCSession(unittest.TestCase):

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

@@ -312,6 +312,18 @@ class TestModuleSpec(unittest.TestCase):
         self.assertEqual(False, isc.config.module_spec._validate_spec(spec, True, {}, None))
         self.assertEqual(False, isc.config.module_spec._validate_spec(spec, True, {}, errors))
         self.assertEqual(['non-optional item an_item missing'], errors)
+
+    def test_validate_unknown_items(self):
+        spec = [{ 'item_name': "an_item",
+                 'item_type': "string",
+                 'item_optional': True,
+                 'item_default': "asdf"
+               }]
+
+        errors = []
+        self.assertEqual(False, isc.config.module_spec._validate_spec_list(spec, True, { 'does_not_exist': 1 }, None))
+        self.assertEqual(False, isc.config.module_spec._validate_spec_list(spec, True, { 'does_not_exist': 1 }, errors))
+        self.assertEqual(['unknown item does_not_exist'], errors)