failover: Protect against empty host names

Added new parameter to split_on_separator that allows to skip
empty values.

The whole function was rewritten. Unit test case was added to
check the new implementation.

https://fedorahosted.org/sssd/ticket/1484

11 files changed, 174 insertions, 102 deletions

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 3707f18b..600d423d 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -599,7 +599,7 @@ int confdb_get_string_as_list(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
         goto done;
     }
 
-    ret = split_on_separator(ctx, values[0], ',', true, result, NULL);
+    ret = split_on_separator(ctx, values[0], ',', true, true, result, NULL);
 
 done:
     talloc_free(values);
diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 8600dab2..dff1071d 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -156,7 +156,7 @@ ad_servers_init(TALLOC_CTX *mem_ctx,
     if (!tmp_ctx) return ENOMEM;
 
     /* Split the server list */
-    ret = split_on_separator(tmp_ctx, servers, ',', true, &list, NULL);
+    ret = split_on_separator(tmp_ctx, servers, ',', true, true, &list, NULL);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
         goto done;
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index eb384a1f..be1bd1d2 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -770,7 +770,7 @@ errno_t ipa_servers_init(struct be_ctx *ctx,
     }
 
     /* split server parm into a list */
-    ret = split_on_separator(tmp_ctx, servers, ',', true, &list, NULL);
+    ret = split_on_separator(tmp_ctx, servers, ',', true, true, &list, NULL);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
         goto done;
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index ed2fffae..c6865c09 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -486,7 +486,7 @@ errno_t krb5_servers_init(struct be_ctx *ctx,
         return ENOMEM;
     }
 
-    ret = split_on_separator(tmp_ctx, servers, ',', true, &list, NULL);
+    ret = split_on_separator(tmp_ctx, servers, ',', true, true, &list, NULL);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
         goto done;
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index f8b921ad..a97dc34e 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -561,7 +561,7 @@ errno_t common_parse_search_base(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = split_on_separator(tmp_ctx, unparsed_base, '?', false,
+    ret = split_on_separator(tmp_ctx, unparsed_base, '?', false, false,
                              &split_bases, &count);
     if (ret != EOK) goto done;
 
@@ -1214,7 +1214,7 @@ errno_t sdap_urls_init(struct be_ctx *ctx,
 
 
     /* split server parm into a list */
-    ret = split_on_separator(tmp_ctx, urls, ',', true, &list, NULL);
+    ret = split_on_separator(tmp_ctx, urls, ',', true, true, &list, NULL);
     if (ret != EOK) {
         DEBUG(1, ("Failed to parse server list!\n"));
         goto done;
diff --git a/src/providers/ldap/ldap_init.c b/src/providers/ldap/ldap_init.c
index 52bd233f..807526b8 100644
--- a/src/providers/ldap/ldap_init.c
+++ b/src/providers/ldap/ldap_init.c
@@ -294,7 +294,7 @@ int sssm_ldap_access_init(struct be_ctx *bectx,
         order = "filter";
     }
 
-    ret = split_on_separator(access_ctx, order, ',', true,
+    ret = split_on_separator(access_ctx, order, ',', true, true,
                              &order_list, &order_list_len);
     if (ret != EOK) {
         DEBUG(1, ("split_on_separator failed.\n"));
diff --git a/src/providers/ldap/sdap_async_sudo_hostinfo.c b/src/providers/ldap/sdap_async_sudo_hostinfo.c
index 0a695cdb..f47e9865 100644
--- a/src/providers/ldap/sdap_async_sudo_hostinfo.c
+++ b/src/providers/ldap/sdap_async_sudo_hostinfo.c
@@ -89,7 +89,7 @@ struct tevent_req * sdap_sudo_get_hostinfo_send(TALLOC_CTX *mem_ctx,
     conf_ip_addr = dp_opt_get_string(opts->basic, SDAP_SUDO_IP);
 
     if (conf_hostnames != NULL) {
-        ret = split_on_separator(state, conf_hostnames, ' ', true,
+        ret = split_on_separator(state, conf_hostnames, ' ', true, true,
                                  &state->hostnames, NULL);
         if (ret != EOK) {
             DEBUG(SSSDBG_MINOR_FAILURE,
@@ -102,7 +102,7 @@ struct tevent_req * sdap_sudo_get_hostinfo_send(TALLOC_CTX *mem_ctx,
     }
 
     if (conf_ip_addr != NULL) {
-        ret = split_on_separator(state, conf_ip_addr, ' ', true,
+        ret = split_on_separator(state, conf_ip_addr, ' ', true, true,
                                  &state->ip_addr, NULL);
         if (ret != EOK) {
             DEBUG(SSSDBG_MINOR_FAILURE,
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index c5d7577b..35381be8 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -167,7 +167,8 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
     char *endptr;
     struct passwd *pwd;
 
-    ret = split_on_separator(mem_ctx, cvs_string, ',', true, &list, &list_size);
+    ret = split_on_separator(mem_ctx, cvs_string, ',', true, false,
+                             &list, &list_size);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("split_on_separator failed [%d][%s].\n",
                                   ret, strerror(ret)));
diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
index ff809a5a..e7e04e65 100644
--- a/src/tests/util-tests.c
+++ b/src/tests/util-tests.c
@@ -718,6 +718,96 @@ START_TEST(test_atomicio_read_from_empty_file)
 }
 END_TEST
 
+struct split_data {
+    const char *input;
+    const char **expected_list;
+    bool trim;
+    bool skip_empty;
+    int expected_size;
+    int expected_ret;
+};
+
+START_TEST(test_split_on_separator)
+{
+    TALLOC_CTX *mem = global_talloc_context;
+    errno_t ret;
+    char **list = NULL;
+    int size;
+    const char *str_ref;
+    const char *str_out;
+    int i;
+    int a;
+    int num_of_tests;
+    struct split_data sts[] = {
+        {
+            "one,two,three", /* input string */
+            (const char *[]){"one", "two", "three", NULL}, /* expec. output list */
+            false, false, /* trim, skip_empty */
+            3, 0 /* expec. size, expec. retval */
+        },
+        {
+            "one,two,three",
+            (const char *[]){"one", "two", "three", NULL},
+            true, true,
+            3, 0
+        },
+        {
+            "  one,  two   ,three ",
+            (const char*[]){"one", "two", "three", NULL},
+            true, true,
+            3, 0
+        },
+        {
+            /* If skip empty is false, single comma means "empty,empty" */
+            ",",
+            (const char*[]){"", "", NULL, NULL},
+            false, false,
+            2, 0
+        },
+        {
+            "one,  ,",
+            (const char*[]){"one", "  ", "NULL", "NULL"},
+            false, true,
+            2, 0
+        },
+        {
+            ", ,,",
+            (const char*[]){NULL},
+            true, true,
+            0, 0
+        },
+        {
+            NULL,
+            NULL,
+            false, false,
+            0, EINVAL
+        },
+    };
+    num_of_tests = sizeof(sts) / sizeof(struct split_data);
+
+    for (a = 0; a < num_of_tests; a++) {
+        ret = split_on_separator(mem, sts[a].input, ',', sts[a].trim,
+                                 sts[a].skip_empty, &list, &size);
+
+        fail_unless(ret == sts[a].expected_ret,
+                    "split_on_separator failed [%d]: %s\n", ret,
+                    strerror(ret));
+        if (ret) {
+            continue;
+        }
+        fail_unless(size == sts[a].expected_size, "Returned wrong size %d "
+                    "(expected %d).\n", size, sts[a].expected_size);
+
+        for (i = 0; str_ref = sts[a].expected_list[i], str_out = list[i]; i++) {
+            fail_unless(strcmp(str_ref, str_out) == 0,
+                        "Expected:%s Got:%s\n", str_ref, str_out);
+        }
+        talloc_free(list);
+        list = NULL;
+    }
+}
+END_TEST
+
 Suite *util_suite(void)
 {
     Suite *s = suite_create("util");
@@ -733,6 +823,7 @@ Suite *util_suite(void)
     tcase_add_test (tc_util, test_parse_args);
     tcase_add_test (tc_util, test_add_string_to_list);
     tcase_add_test (tc_util, test_string_in_list);
+    tcase_add_test (tc_util, test_split_on_separator);
     tcase_set_timeout(tc_util, 60);
 
     TCase *tc_utf8 = tcase_create("utf8");
diff --git a/src/util/util.c b/src/util/util.c
index ab980775..b035e231 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -27,118 +27,97 @@
 #include "util/sss_utf8.h"
 #include "dhash.h"
 
-/* split a string into an allocated array of strings.
- * the separator is a string, and is case-sensitive.
- * optionally single values can be trimmed of of spaces and tabs */
 int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
-                       const char sep, bool trim, char ***_list, int *size)
+                       const char sep, bool trim, bool skip_empty,
+                       char ***_list, int *size)
 {
-    const char *t, *p, *n;
-    size_t l, len;
-    char **list, **r;
-    const char sep_str[2] = { sep, '\0'};
-
-    if (!str || !*str || !_list) return EINVAL;
-
-    t = str;
+    int ret;
+    const char *substr_end = str;
+    const char *substr_begin = str;
+    const char *sep_pos = NULL;
+    size_t substr_len;
+    char **list = NULL;
+    int num_strings = 0;
+    TALLOC_CTX *tmp_ctx = NULL;
+
+    if (str == NULL || *str == '\0' || _list == NULL) {
+        return EINVAL;
+    }
 
-    list = NULL;
-    l = 0;
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
 
-    /* trim leading whitespace */
-    if (trim)
-        while (isspace(*t)) t++;
+    do {
+        substr_len = 0;
 
-    /* find substrings separated by the separator */
-    while (t && (p = strpbrk(t, sep_str))) {
-        len = p - t;
-        n = p + 1; /* save next string starting point */
-        if (trim) {
-            /* strip whitespace after the separator
-             * so it's not in the next token */
-            while (isspace(*t)) {
-                t++;
-                len--;
-                if (len == 0) break;
-            }
-            p--;
-            /* strip whitespace before the separator
-             * so it's not in the current token */
-            while (len > 0 && (isspace(*p))) {
-                len--;
-                p--;
-            }
+        /* If this is not the first substring, then move from the separator. */
+        if (sep_pos != NULL) {
+            substr_end = sep_pos + 1;
+            substr_begin = sep_pos + 1;
         }
 
-        /* Add the token to the array, +2 b/c of the trailing NULL */
-        r = talloc_realloc(mem_ctx, list, char *, l + 2);
-        if (!r) {
-            talloc_free(list);
-            return ENOMEM;
-        } else {
-            list = r;
+        /* Find end of the first substring */
+        while (*substr_end != sep && *substr_end != '\0') {
+            substr_end++;
+            substr_len++;
         }
 
-        if (len == 0) {
-            list[l] = talloc_strdup(list, "");
-        } else {
-            list[l] = talloc_strndup(list, t, len);
-        }
-        if (!list[l]) {
-            talloc_free(list);
-            return ENOMEM;
-        }
-        l++;
+        sep_pos = substr_end;
 
-        t = n; /* move to next string */
-    }
+        if (trim) {
+            /* Trim leading whitespace */
+            while (isspace(*substr_begin) && substr_begin < substr_end) {
+                substr_begin++;
+                substr_len--;
+            }
 
-    /* Handle the last remaining token */
-    if (t) {
-        r = talloc_realloc(mem_ctx, list, char *, l + 2);
-        if (!r) {
-            talloc_free(list);
-            return ENOMEM;
-        } else {
-            list = r;
+            /* Trim trailing whitespace */
+            while (substr_end - 1 > substr_begin && isspace(*(substr_end-1))) {
+                substr_end--;
+                substr_len--;
+            }
         }
 
-        if (trim) {
-            /* trim leading whitespace */
-            len = strlen(t);
-            while (isspace(*t)) {
-                t++;
-                len--;
-                if (len == 0) break;
-            }
-            /* trim trailing whitespace */
-            p = t + len - 1;
-            while (len > 0 && (isspace(*p))) {
-                len--;
-                p--;
+        /* Copy the substring to the output list of strings */
+        if (skip_empty == false || substr_len > 0) {
+            list = talloc_realloc(tmp_ctx, list, char*, num_strings + 2);
+            if (list == NULL) {
+                ret = ENOMEM;
+                goto done;
             }
 
-            if (len == 0) {
-                list[l] = talloc_strdup(list, "");
-            } else {
-                list[l] = talloc_strndup(list, t, len);
+            /* empty string is stored for substr_len == 0 */
+            list[num_strings] = talloc_strndup(list, substr_begin, substr_len);
+            if (list[num_strings] == NULL) {
+                ret = ENOMEM;
+                goto done;
             }
-        } else {
-            list[l] = talloc_strdup(list, t);
+            num_strings++;
         }
-        if (!list[l]) {
-            talloc_free(list);
-            return ENOMEM;
+
+    } while (*sep_pos != '\0');
+
+    if (list == NULL) {
+        /* No allocations were done, make space for the NULL */
+        list = talloc(tmp_ctx, char *);
+        if (list == NULL) {
+            ret = ENOMEM;
+            goto done;
         }
-        l++;
     }
+    list[num_strings] = NULL;
 
-    list[l] = NULL; /* terminate list */
-
-    if (size) *size = l;
-    *_list = list;
+    if (size) {
+        *size = num_strings;
+    }
 
-    return EOK;
+    *_list = talloc_steal(mem_ctx, list);
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
 }
 
 static void free_args(char **args)
diff --git a/src/util/util.h b/src/util/util.h
index c15ca668..e4cb1a86 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -476,7 +476,8 @@ errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid,
 
 /* from util.c */
 int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
-                       const char sep, bool trim, char ***_list, int *size);
+                       const char sep, bool trim, bool skip_empty,
+                       char ***_list, int *size);
 
 char **parse_args(const char *str);