From caec174b203206185b6075c0e822c6f45070dd87 Mon Sep 17 00:00:00 2001

From: Alexander Scheel <ascheel@redhat.com>

Date: Wed, 9 Aug 2017 15:00:26 -0400

Subject: [PATCH] Fix silent crash with duplicate config sections

Signed-off-by: Alexander Scheel <ascheel@redhat.com>

Reviewed-by: Simo Sorce <simo@redhat.com>

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 <linux/limits.h>

+#include <string.h>

+#include <errno.h>

+#include <stdio.h>

+#include <stdlib.h>

+#include <stdint.h>

+#include <ini_configobj.h>

+#include <ini_config.h>

+

+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;

 }