From caec174b203206185b6075c0e822c6f45070dd87 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Wed, 9 Aug 2017 15:00:26 -0400 Subject: [PATCH] Fix silent crash with duplicate config sections Signed-off-by: Alexander Scheel Reviewed-by: Simo Sorce Resolves: #194 Merges: #202 (cherry picked from commit c0d85387fc38f9554d601ec2ddb111031a694387) --- proxy/configure.ac | 125 ++++++++++++++++++++++++++++++++++++++++++ proxy/src/gp_config.c | 27 ++++----- 2 files changed, 137 insertions(+), 15 deletions(-) diff --git a/proxy/configure.ac b/proxy/configure.ac index c52dbb6..9e01f7d 100644 --- a/proxy/configure.ac +++ b/proxy/configure.ac @@ -107,6 +107,131 @@ fi AC_SUBST(INI_LIBS) AC_SUBST(INI_CFLAGS) +AC_CHECK_LIB(ref_array, ref_array_destroy, [], + [AC_MSG_WARN([ref_array library must support ref_array_destroy])], + [$INI_CONFIG_LIBS]) + +AC_RUN_IFELSE([AC_LANG_SOURCE([[ +/* See: https://pagure.io/SSSD/ding-libs/pull-request/3172 */ +#include +#include +#include +#include +#include +#include +#include +#include + +static int write_to_file(char *path, char *text) +{ + FILE *f = fopen(path, "w"); + int bytes = 0; + if (f == NULL) + return 1; + + bytes = fprintf(f, "%s", text); + if (bytes != strlen(text)) + return 1; + + return fclose(f); +} + +int main(void) +{ + char base_path[PATH_MAX]; + char augment_path[PATH_MAX]; + + char config_base[] = + "[section]\n" + "key1 = first\n" + "key2 = exists\n"; + + char config_augment[] = + "[section]\n" + "key1 = augment\n" + "key3 = exists\n"; + + char *builddir; + + struct ini_cfgobj *in_cfg, *result_cfg; + struct ini_cfgfile *file_ctx; + + uint32_t merge_flags = INI_MS_DETECT | INI_MS_PRESERVE; + + int ret; + + builddir = getenv("builddir"); + if (builddir == NULL) { + builddir = strdup("."); + } + + snprintf(base_path, PATH_MAX, "%s/tmp_augment_base.conf", builddir); + snprintf(augment_path, PATH_MAX, "%s/tmp_augment_augment.conf", builddir); + + ret = write_to_file(base_path, config_base); + if (ret != 0) { + ret = 1; + goto cleanup; + } + + ret = write_to_file(augment_path, config_augment); + if (ret != 0) { + goto cleanup; + } + + /* Match only augment.conf */ + const char *m_patterns[] = { "^tmp_augment_augment.conf$", NULL }; + + /* Match all sections */ + const char *m_sections[] = { ".*", NULL }; + + /* Create config collection */ + ret = ini_config_create(&in_cfg); + if (ret != EOK) + goto cleanup; + + /* Open base.conf */ + ret = ini_config_file_open(base_path, 0, &file_ctx); + if (ret != EOK) + goto cleanup; + + /* Seed in_cfg with base.conf */ + ret = ini_config_parse(file_ctx, 1, 0, 0, in_cfg); + if (ret != EOK) + goto cleanup; + + /* Update base.conf with augment.conf */ + ret = ini_config_augment(in_cfg, + builddir, + m_patterns, + m_sections, + NULL, + INI_STOP_ON_NONE, + 0, + INI_PARSE_NOSPACE|INI_PARSE_NOTAB, + merge_flags, + &result_cfg, + NULL, + NULL); + /* We always expect EEXIST due to DETECT being set. */ + if (ret != EEXIST) + goto cleanup; + + ret = 0; + +cleanup: + remove(base_path); + remove(augment_path); + + /* Per autoconf guidelines */ + if (ret != 0) + ret = 1; + + return ret; +} +]])] +,, [AC_MSG_ERROR(["ini_config library must support extended INI_MS_DETECT. See: https://pagure.io/SSSD/ding-libs/pull-request/3172"])]) + AX_PTHREAD(,[AC_MSG_ERROR([Could not find Pthreads support])]) LIBS="$PTHREAD_LIBS $LIBS" diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c index 07f7c8d..cd057a0 100644 --- a/proxy/src/gp_config.c +++ b/proxy/src/gp_config.c @@ -728,7 +728,7 @@ static int gp_config_from_file(const char *config_file, 0, /* metadata_flags, FIXME */ &file_ctx); if (ret) { - GPDEBUG("Failed to open config file: %d (%s)\n", + GPERROR("Failed to open config file: %d (%s)\n", ret, gp_strerror(ret)); ini_config_destroy(ini_config); return ret; @@ -742,7 +742,7 @@ static int gp_config_from_file(const char *config_file, if (ret) { char **errors = NULL; /* we had a parsing failure */ - GPDEBUG("Failed to parse config file: %d (%s)\n", + GPERROR("Failed to parse config file: %d (%s)\n", ret, gp_strerror(ret)); if (ini_config_error_count(ini_config)) { ini_config_get_errors(ini_config, &errors); @@ -791,26 +791,25 @@ static int gp_config_from_dir(const char *config_dir, INI_STOP_ON_ANY, /* error_level */ collision_flags, INI_PARSE_NOWRAP, - /* do not allow colliding sections with the same - * name in different files */ - INI_MS_ERROR, + /* allow sections with the same name in + * different files, but log warnings */ + INI_MS_DETECT | INI_MS_PRESERVE, &result_cfg, &error_list, NULL); - if (ret) { + if (error_list) { uint32_t len; - - if (!error_list) { - GPAUDIT("Error when reading config directory number: %d\n", ret); - return ret; - } - len = ref_array_len(error_list); for (uint32_t i = 0; i < len; i++) { /* libini has an unfixable bug where error strings are (char **) */ GPAUDIT("Error when reading config directory: %s\n", *(char **)ref_array_get(error_list, i, NULL)); } + ref_array_destroy(error_list); + } + + if (ret && ret != EEXIST) { + GPERROR("Error when reading config directory number: %d\n", ret); ref_array_destroy(error_list); return ret; @@ -821,9 +820,7 @@ static int gp_config_from_dir(const char *config_dir, ini_config_destroy(*ini_config); *ini_config = result_cfg; } - if (error_list) { - ref_array_destroy(error_list); - } + return 0; }