diff --git a/common/config/yaml/config_yaml.cpp b/common/config/yaml/config_yaml.cpp index 0da2d4bf56..0b9139c407 100644 --- a/common/config/yaml/config_yaml.cpp +++ b/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(params[i].optname)]) { - SetNonDefault(cfg, node, ¶ms[i]); - } else { - config_common_getdefault(cfg, ¶ms[i], nullptr); - } + if (node && node[std::string(param->optname)]) { + SetNonDefault(cfg, node, param); + } else { + config_common_getdefault(cfg, param, 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)); - if (node == config_yaml::invalid_node) { + YAML::Node 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; + } + } } } diff --git a/common/config/yaml/tests/CMakeLists.txt b/common/config/yaml/tests/CMakeLists.txt index d33fa57658..e3f1002d79 100644 --- a/common/config/yaml/tests/CMakeLists.txt +++ b/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) diff --git a/common/config/yaml/tests/malformed.yaml b/common/config/yaml/tests/malformed.yaml new file mode 100644 index 0000000000..c7f5fff55b --- /dev/null +++ b/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 diff --git a/common/config/yaml/tests/test_read_str_as_int.yaml b/common/config/yaml/tests/test_read_str_as_int.yaml new file mode 100644 index 0000000000..a055e855d5 --- /dev/null +++ b/common/config/yaml/tests/test_read_str_as_int.yaml @@ -0,0 +1,2 @@ +test: + var: "test_var" diff --git a/common/config/yaml/tests/test_yaml_config.cpp b/common/config/yaml/tests/test_yaml_config.cpp index 384a1f406c..814e3a3635 100644 --- a/common/config/yaml/tests/test_yaml_config.cpp +++ b/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(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(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(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();