[svn] commit: r3765 - in /branches/trac202/src/lib: config/ config/tests/ config/tests/testdata/ python/isc/config/ python/isc/config/tests/

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Dec 8 13:55:27 UTC 2010


Author: jelte
Date: Wed Dec  8 13:55:26 2010
New Revision: 3765

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

Added:
    branches/trac202/src/lib/config/tests/testdata/data22_9.data
Modified:
    branches/trac202/src/lib/config/module_spec.cc
    branches/trac202/src/lib/config/tests/module_spec_unittests.cc
    branches/trac202/src/lib/config/tests/testdata/data22_8.data
    branches/trac202/src/lib/python/isc/config/ccsession.py
    branches/trac202/src/lib/python/isc/config/module_spec.py
    branches/trac202/src/lib/python/isc/config/tests/ccsession_test.py
    branches/trac202/src/lib/python/isc/config/tests/module_spec_test.py

Modified: branches/trac202/src/lib/config/module_spec.cc
==============================================================================
--- branches/trac202/src/lib/config/module_spec.cc (original)
+++ branches/trac202/src/lib/config/module_spec.cc Wed Dec  8 13:55:26 2010
@@ -330,14 +330,33 @@
 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);
-        }
-    }
-    return (true);
-}
-
-}
-}
+            validated = false;
+        }
+    }
+
+    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);
+}
+
+}
+}

Modified: branches/trac202/src/lib/config/tests/module_spec_unittests.cc
==============================================================================
--- branches/trac202/src/lib/config/tests/module_spec_unittests.cc (original)
+++ branches/trac202/src/lib/config/tests/module_spec_unittests.cc Wed Dec  8 13:55:26 2010
@@ -166,8 +166,13 @@
     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());
 }

Modified: branches/trac202/src/lib/config/tests/testdata/data22_8.data
==============================================================================
--- branches/trac202/src/lib/config/tests/testdata/data22_8.data (original)
+++ branches/trac202/src/lib/config/tests/testdata/data22_8.data Wed Dec  8 13:55:26 2010
@@ -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 } }
 }

Modified: branches/trac202/src/lib/python/isc/config/ccsession.py
==============================================================================
--- branches/trac202/src/lib/python/isc/config/ccsession.py (original)
+++ branches/trac202/src/lib/python/isc/config/ccsession.py Wed Dec  8 13:55:26 2010
@@ -224,7 +224,7 @@
                     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 @@
         """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))

Modified: branches/trac202/src/lib/python/isc/config/module_spec.py
==============================================================================
--- branches/trac202/src/lib/python/isc/config/module_spec.py (original)
+++ branches/trac202/src/lib/python/isc/config/module_spec.py Wed Dec  8 13:55:26 2010
@@ -328,7 +328,24 @@
         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

Modified: branches/trac202/src/lib/python/isc/config/tests/ccsession_test.py
==============================================================================
--- branches/trac202/src/lib/python/isc/config/tests/ccsession_test.py (original)
+++ branches/trac202/src/lib/python/isc/config/tests/ccsession_test.py Wed Dec  8 13:55:26 2010
@@ -290,7 +290,7 @@
         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 @@
         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 @@
         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 @@
         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)
@@ -559,6 +559,14 @@
         mccs.check_command()
         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):
@@ -581,7 +589,7 @@
         if name in self.post_answers:
             return self.post_answers[name]
         else:
-            return None
+            return fakeAnswer()
     
 
 class TestUIModuleCCSession(unittest.TestCase):

Modified: branches/trac202/src/lib/python/isc/config/tests/module_spec_test.py
==============================================================================
--- branches/trac202/src/lib/python/isc/config/tests/module_spec_test.py (original)
+++ branches/trac202/src/lib/python/isc/config/tests/module_spec_test.py Wed Dec  8 13:55:26 2010
@@ -312,6 +312,18 @@
         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)
         
 
 




More information about the bind10-changes mailing list