Handle YAML Exceptions

common/config/yaml/config_yaml.cpp

@@ -141,14 +141,12 @@ void SetNonDefault(configmodule_interface_t *cfg, const YAML::Node &node, paramd
   param->paramflags |= PARAMFLAG_PARAMSET;
 }
 
-void GetParams(configmodule_interface_t *cfg, const YAML::Node &node, paramdef_t *params, int num_params)
+void GetParam(configmodule_interface_t *cfg, const YAML::Node &node, paramdef_t *param)
 {
-  for (auto i = 0; i < num_params; i++) {
+  if (node && node[std::string(param->optname)]) {
-    if (node && node[std::string(params[i].optname)]) {
+    SetNonDefault(cfg, node, param);
-      SetNonDefault(cfg, node, &params[i]);
+  } else {
-    } else {
+    config_common_getdefault(cfg, param, nullptr);
-      config_common_getdefault(cfg, &params[i], nullptr);
-    }
   }
 }
 
@@ -163,7 +161,15 @@ extern "C" int config_yaml_init(configmodule_interface_t *cfg)
   pthread_mutex_init(&cfg->memBlocks_mutex, NULL);
   memset(cfg->oneBlock, 0, sizeof(cfg->oneBlock));
 
-  config_yaml::config = new config_yaml::YamlConfig(std::string(cfgP[0]));
+  try {
+    config_yaml::config = new config_yaml::YamlConfig(std::string(cfgP[0]));
+  } catch (const YAML::BadFile &e) {
+    fprintf(stderr, "[CONFIG] Error loading YAML file '%s': %s\n", cfgP[0], e.what());
+    return -1;
+  } catch (const YAML::Exception &e) {
+    fprintf(stderr, "[CONFIG] Error parsing YAML file '%s': %s\n", cfgP[0], e.what());
+    return -1;
+  }
   return 0;
 }
 
@@ -183,14 +189,14 @@ YAML::Node find_node(const std::string &prefix, YAML::Node node)
     if (key.at(0) == '[' && key.back() == ']') {
       // The key is an index to a sequence
       if (!current.IsSequence()) {
-        throw std::invalid_argument("Incorrect yaml file structure");
+        throw std::invalid_argument("Expected a sequence at " + key);
       }
       int index = std::stoi(key.substr(1, key.size() - 2));
       current = current[index];
       continue;
     }
     if (!current.IsMap()) {
-      throw std::invalid_argument("Incorrect yaml file structure");
+      throw std::invalid_argument("Expected a map at " + key);
     }
     if (!current[key]) {
       return config_yaml::invalid_node;
@@ -206,8 +212,14 @@ extern "C" int config_yaml_get(configmodule_interface_t *cfg, paramdef_t *cfgopt
   if (prefix != nullptr) {
     p = std::string(prefix);
   }
-  auto node = find_node(p, YAML::Clone(config_yaml::config->config));
+  YAML::Node node;
-  if (node == config_yaml::invalid_node) {
+  try {
+    node = find_node(p, YAML::Clone(config_yaml::config->config));
+    if (node == config_yaml::invalid_node) {
+      return -1;
+    }
+  } catch (const std::invalid_argument &e) {
+    fprintf(stderr, "[CONFIG] Error finding node '%s': %s\n", p.c_str(), e.what());
     return -1;
   }
   for (auto i = 0; i < numoptions; i++) {
@@ -216,7 +228,14 @@ extern "C" int config_yaml_get(configmodule_interface_t *cfg, paramdef_t *cfgopt
       config_check_valptr(cfg, &cfgoptions[i], sizeof(void *), 1);
     }
   }
-  config_yaml::GetParams(cfg, node, cfgoptions, numoptions);
+  for (int i = 0; i < numoptions; i++) {
+    try {
+      config_yaml::GetParam(cfg, node, &cfgoptions[i]);
+    } catch (const YAML::Exception &e) {
+      fprintf(stderr, "[CONFIG] Config error: prefix: %s, setting: %s, error: %s\n", p.c_str(), cfgoptions[i].optname, e.what());
+      return -1;
+    }
+  }
   return numoptions;
 }
 
@@ -257,7 +276,14 @@ extern "C" int config_yaml_getlist(configmodule_interface_t *cfg,
         }
       }
 
-      config_yaml::GetParams(cfg, node[i], ParamList->paramarray[i], numparams);
+      for (int j = 0; j < numparams; j++) {
+        try {
+          config_yaml::GetParam(cfg, node[i], &ParamList->paramarray[i][j]);
+        } catch (const YAML::Exception &e) {
+          fprintf(stderr, "[CONFIG] Config error: prefix: %s, setting: %s, error: %s\n", path, ParamList->paramarray[i][j].optname, e.what());
+          return -1;
+        }
+      }
     }
   }
 

common/config/yaml/tests/CMakeLists.txt

@@ -11,3 +11,5 @@ configure_file(test_list_of_mappings.yml test_list_of_mappings.yml COPYONLY)
 configure_file(test_int_array.yaml test_int_array.yaml COPYONLY)
 configure_file(test_read_mapping_as_list.yaml test_read_mapping_as_list.yaml COPYONLY)
 configure_file(test_ipv4.yaml test_ipv4.yaml COPYONLY)
+configure_file(test_read_str_as_int.yaml test_read_str_as_int.yaml COPYONLY)
+configure_file(malformed.yaml malformed.yaml COPYONLY)

common/config/yaml/tests/malformed.yaml

@@ -0,0 +1,13 @@
+test:
+  -
+bad_file:
+  - "testvalue1"
+  - "testvalue2"
+  - "testvalue3"
+  - "testvalue4"
+  - "testvalue5"
+  - "testvalue6"
+  - "testvalue7"
+  - "testvalue8"
+  - "testvalue9"
+  - "testvalue10" - yaml error

common/config/yaml/tests/test_read_str_as_int.yaml

@@ -0,0 +1,2 @@
+test:
+  var: "test_var"

common/config/yaml/tests/test_yaml_config.cpp

@@ -78,8 +78,9 @@ TEST(yaml_config, yaml_get_existing_values) {
     uint16_t value;
     p.type = TYPE_UINT16;
     p.u16ptr = &value;
+    p.paramflags = PARAMFLAG_MANDATORY;
     char prefix[] = "test";
-    config_yaml_get(cfg, &p, 1, prefix);
+    EXPECT_EQ(config_yaml_get(cfg, &p, 1, prefix), 1);
     EXPECT_EQ(value, i);
   }
 
@@ -94,7 +95,7 @@ TEST(yaml_config, yaml_get_non_existing_values) {
   cfg->cfgP[0] = strdup("test1.yaml");
   EXPECT_EQ(config_yaml_init(cfg), 0);
 
-  // Testing paremters present in the test node
+  // Testing parameters not present in the test node
   paramdef_t p = {0};
   for (auto i = 4; i <= 5; i++) {
     sprintf(p.optname, "%s%d", "value", i);
@@ -421,6 +422,43 @@ TEST(yaml_config, test_read_ipv4) {
   free(p.defstrval);
 }
 
+TEST(yaml_config, yaml_read_str_as_int) {
+  configmodule_interface_t *cfg = static_cast<configmodule_interface_t*>(calloc(1, sizeof(*cfg)));
+  cfg->cfgP[0] = strdup("test_read_str_as_int.yaml");
+  EXPECT_EQ(config_yaml_init(cfg), 0);
+
+  // Testing paremters present in the test node
+  paramdef_t p = {0};
+  uint16_t value;
+  p.type = TYPE_UINT16;
+  p.u16ptr = &value;
+  p.paramflags = PARAMFLAG_MANDATORY;
+  char prefix[] = "test";
+  sprintf(p.optname, "%s", "var");
+  EXPECT_EQ(config_yaml_get(cfg, &p, 1, prefix), -1);
+
+
+  config_yaml_end(cfg);
+  free(cfg->cfgP[0]);
+  end_configmodule(cfg);
+}
+
+TEST(yaml_config, yaml_open_non_existing_file) {
+  configmodule_interface_t *cfg = static_cast<configmodule_interface_t*>(calloc(1, sizeof(*cfg)));
+  cfg->cfgP[0] = strdup("non_existing_file.yaml");
+  EXPECT_EQ(config_yaml_init(cfg), -1);
+  free(cfg->cfgP[0]);
+  end_configmodule(cfg);
+}
+
+TEST(yaml_config, malformed_file) {
+  configmodule_interface_t *cfg = static_cast<configmodule_interface_t*>(calloc(1, sizeof(*cfg)));
+  cfg->cfgP[0] = strdup("malformed.yaml");
+  EXPECT_EQ(config_yaml_init(cfg), -1);
+  free(cfg->cfgP[0]);
+  end_configmodule(cfg);
+}
+
 int main(int argc, char** argv)
 {
   logInit();