Only replace space with the specified substitution

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

- make sss_replace_whitespaces only replace space (' ') not any
  whitespace
- make sss_replace_whitespaces only replace a single char, not the whole
  string
- rename CONFDB_NSS_OVERRIDE_DEFAULT_WHITESPACE to
  CONFDB_NSS_OVERRIDE_DEFAULT_SPACE
- rename the override_default_whitespace option to override_space
- rename sss_replace_whitespaces() to sss_replace_space()
- rename sss_reverse_replace_whitespaces() to sss_reverse_replace_space()
- rename nctx->override_default_wsp_str to nctx->override_space
- make the return value of sss_replace_space non-const to avoid freeing
  the result without compilation warnings

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>

11 files changed, 110 insertions, 218 deletions

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index b7395c1f3..d5f7511d0 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -99,7 +99,7 @@
 #define CONFDB_MEMCACHE_TIMEOUT "memcache_timeout"
 #define CONFDB_NSS_HOMEDIR_SUBSTRING "homedir_substring"
 #define CONFDB_DEFAULT_HOMEDIR_SUBSTRING "/home"
-#define CONFDB_NSS_OVERRIDE_DEFAULT_WHITESPACE "override_default_whitespace"
+#define CONFDB_NSS_OVERRIDE_SPACE "override_space"
 
 /* PAM */
 #define CONFDB_PAM_CONF_ENTRY "config/pam"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index c3482b3a4..38111a86f 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -73,7 +73,7 @@ option_strings = {
     'shell_fallback' : _('If a shell stored in central directory is allowed but not available, use this fallback'),
     'default_shell': _('Shell to use if the provider does not list one'),
     'memcache_timeout': _('How long will be in-memory cache records valid'),
-    'override_default_whitespace': _('All white spaces in group or user names will be replaced with this string'),
+    'override_space': _('All spaces in group or user names will be replaced with this character'),
 
     # [pam]
     'offline_credentials_expiration' : _('How long to allow cached logins between online logins (days)'),
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 1db9e2207..a3398c8d5 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -44,7 +44,7 @@ shell_fallback = str, None, false
 default_shell = str, None, false
 get_domains_timeout = int, None, false
 memcache_timeout = int, None, false
-override_default_whitespace = str, None, false
+override_space = str, None, false
 
 [pam]
 # Authentication service
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 625ac20a2..99d91f062 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -664,19 +664,19 @@ fallback_homedir = /home/%u
                     </listitem>
                 </varlistentry>
                 <varlistentry>
-                    <term>override_default_whitespace (string)</term>
+                    <term>override_space (string)</term>
                     <listitem>
                         <para>
-                            This parameter will replace white spaces (space bar)
-                            with the given string for user and group names.
+                            This parameter will replace spaces (space bar)
+                            with the given character for user and group names.
                             e.g. (_). User name &quot;john doe&quot; will
                             be &quot;john_doe&quot; This feature was added to
                             help compatibility with shell scripts that have
-                            difficulty handling white spaces, due to the
+                            difficulty handling spaces, due to the
                             default field separator in the shell.
                         </para>
                         <para>
-                            Default: not set (whitespaces will not be replaced)
+                            Default: not set (spaces will not be replaced)
                         </para>
                     </listitem>
                 </varlistentry>
diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
index cf4525a71..9705878fa 100644
--- a/src/responder/nss/nsssrv.c
+++ b/src/responder/nss/nsssrv.c
@@ -299,8 +299,8 @@ static int nss_get_config(struct nss_ctx *nctx,
     if (ret != EOK) goto done;
 
     ret = confdb_get_string(cdb, nctx, CONFDB_NSS_CONF_ENTRY,
-                            CONFDB_NSS_OVERRIDE_DEFAULT_WHITESPACE, NULL,
-                            &nctx->override_default_wsp_str);
+                            CONFDB_NSS_OVERRIDE_SPACE, NULL,
+                            &nctx->override_space);
     if (ret != EOK) goto done;
 
     ret = 0;
diff --git a/src/responder/nss/nsssrv.h b/src/responder/nss/nsssrv.h
index 07443027e..f5238fb82 100644
--- a/src/responder/nss/nsssrv.h
+++ b/src/responder/nss/nsssrv.h
@@ -69,7 +69,7 @@ struct nss_ctx {
     char **etc_shells;
     char *shell_fallback;
     char *default_shell;
-    char *override_default_wsp_str;
+    char *override_space;
 
     struct sss_mc_ctx *pwd_mc_ctx;
     struct sss_mc_ctx *grp_mc_ctx;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 3e1b470e5..10e84e52c 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -372,11 +372,10 @@ static int fill_pwent(struct sss_packet *packet,
             continue;
         }
 
-        tmpstr = sss_replace_whitespaces(tmp_ctx, tmpstr,
-                                         nctx->override_default_wsp_str);
+        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->override_space[0]);
         if (tmpstr == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
-                  "sss_replace_whitespaces failed, skipping\n");
+                  "sss_replace_space failed, skipping\n");
             continue;
         }
 
@@ -752,11 +751,11 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx)
         name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive);
         if (!name) return ENOMEM;
 
-        name = sss_reverse_replace_whitespaces(dctx, name,
-                                               nctx->override_default_wsp_str);
+        name = sss_reverse_replace_space(dctx, name,
+                                         nctx->override_space[0]);
         if (name == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
-                  "sss_reverse_replace_whitespaces failed\n");
+                  "sss_reverse_replace_space failed\n");
             return ENOMEM;
         }
 
@@ -2361,11 +2360,10 @@ static int fill_members(struct sss_packet *packet,
             continue;
         }
 
-        tmpstr = sss_replace_whitespaces(tmp_ctx, tmpstr,
-                                         nctx->override_default_wsp_str);
+        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->override_space[0]);
         if (tmpstr == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
-                  "sss_replace_whitespaces failed\n");
+                  "sss_replace_space failed\n");
             ret = ENOMEM;
             goto done;
         }
@@ -2525,11 +2523,10 @@ static int fill_grent(struct sss_packet *packet,
             continue;
         }
 
-        tmpstr = sss_replace_whitespaces(tmp_ctx, tmpstr,
-                                         nctx->override_default_wsp_str);
+        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->override_space[0]);
         if (tmpstr == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
-                  "sss_replace_whitespaces failed, skipping\n");
+                  "sss_replace_space failed, skipping\n");
             continue;
         }
 
@@ -2727,11 +2724,10 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx)
         name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive);
         if (!name) return ENOMEM;
 
-        name = sss_reverse_replace_whitespaces(dctx, name,
-                                               nctx->override_default_wsp_str);
+        name = sss_reverse_replace_space(dctx, name, nctx->override_space[0]);
         if (name == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
-                  "sss_reverse_replace_whitespaces failed\n");
+                  "sss_reverse_replace_space failed\n");
             return ENOMEM;
         }
 
@@ -3758,11 +3754,10 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx)
         name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive);
         if (!name) return ENOMEM;
 
-        name = sss_reverse_replace_whitespaces(dctx, name,
-                                               nctx->override_default_wsp_str);
+        name = sss_reverse_replace_space(dctx, name, nctx->override_space[0]);
         if (name == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE,
-                  "sss_reverse_replace_whitespaces failed\n");
+                  "sss_reverse_replace_space failed\n");
             return ENOMEM;
         }
 
@@ -3925,11 +3920,11 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx)
                 goto done;
             }
 
-            name = sss_reverse_replace_whitespaces(dctx, name,
-                                               nctx->override_default_wsp_str);
+            name = sss_reverse_replace_space(dctx, name,
+                                             nctx->override_space[0]);
             if (name == NULL) {
                 DEBUG(SSSDBG_CRIT_FAILURE,
-                      "sss_reverse_replace_whitespaces failed\n");
+                      "sss_reverse_replace_space failed\n");
                 ret = ENOMEM;
                 goto done;
             }
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 808a7a185..dba03ddb6 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -74,6 +74,7 @@ mock_nctx(TALLOC_CTX *mem_ctx)
     }
     nctx->neg_timeout = 10;
     nctx->pwfield = discard_const("*");
+    nctx->override_space = discard_const("\0");
 
     err = sss_idmap_init(sss_idmap_talloc, nctx, sss_idmap_talloc_free,
                          &nctx->idmap_ctx);
diff --git a/src/tests/cmocka/test_string_utils.c b/src/tests/cmocka/test_string_utils.c
index d10c3be47..93d7d716f 100644
--- a/src/tests/cmocka/test_string_utils.c
+++ b/src/tests/cmocka/test_string_utils.c
@@ -31,56 +31,40 @@ void test_replace_whitespaces(void **state)
     struct {
         const char *input;
         const char *output;
-        const char *replace_string;
+        const char replace_char;
     } data_set[] = {
-        { "", "", "-" },
-        { " ", "-", "-" },
-        { "\t", "-", "-" },
-        { "\n", "-", "-" },
-        { " \t\n ", "----", "-" },
-        { "abcd", "abcd", "-" },
-        { "a b c d", "a-b-c-d", "-" },
-        { " a b c d ", "-a-b-c-d-", "-" },
-        { " ", "^", "^" },
-        { "\t", "^", "^" },
-        { "\n", "^", "^" },
-        { " \t\n ", "^^^^", "^" },
-        { "abcd", "abcd", "^" },
-        { "a b c d", "a^b^c^d", "^" },
-        { " a b c d ", "^a^b^c^d^", "^" },
-        { " ", "^", "^" },
-        { "\t", ";-)", ";-)" },
-        { "\n", ";-)", ";-)" },
-        { " \t\n ", ";-);-);-);-)", ";-)" },
-        { "abcd", "abcd", ";-)" },
-        { "a b c d", "a;-)b;-)c;-)d", ";-)" },
-        { " a b c d ", ";-)a;-)b;-)c;-)d;-)", ";-)" },
-        { " ", " ", " " },
-        { "    ", "    ", " " },
-        { "abcd", "abcd", " " },
-        { "a b c d", "a b c d", " " },
-        { " a b c d ", " a b c d ", " " },
-        { " ", " \t", " \t" },
-        { "    ", " \t \t \t \t", " \t" },
-        { "abcd", "abcd", " \t" },
-        { "a b c d", "a \tb \tc \td", " \t" },
-        { " a b c d ", " \ta \tb \tc \td \t", " \t" },
-        { NULL, NULL, NULL },
+        { "", "", '-' },
+        { " ", "-", '-' },
+        { "abcd", "abcd", '-' },
+        { "a b c d", "a-b-c-d", '-' },
+        { " a b c d ", "-a-b-c-d-", '-' },
+        { " ", "^", '^' },
+        { "abcd", "abcd", '^' },
+        { "a b c d", "a^b^c^d", '^' },
+        { " a b c d ", "^a^b^c^d^", '^' },
+        { " ", "^", '^' },
+        { " ", " ", ' ' },
+        { "    ", "    ", ' ' },
+        { "abcd", "abcd", ' ' },
+        { "a b c d", "a b c d", ' ' },
+        { NULL, NULL, '\0' },
     };
 
     mem_ctx = talloc_new(NULL);
     assert_non_null(mem_ctx);
     check_leaks_push(mem_ctx);
 
-    res = sss_replace_whitespaces(mem_ctx, input_str, NULL);
-    assert_true(res == input_str);
+    res = sss_replace_space(mem_ctx, input_str, '\0');
+    assert_string_equal(res, input_str);
+    talloc_zfree(res);
 
-    res = sss_replace_whitespaces(mem_ctx, input_str, "");
-    assert_true(res == input_str);
+    res = sss_replace_space(mem_ctx, input_str, '\0');
+    assert_string_equal(res, input_str);
+    talloc_zfree(res);
 
     for (i=0; data_set[i].input != NULL; ++i) {
-        res = sss_replace_whitespaces(mem_ctx, data_set[i].input,
-                                      data_set[i].replace_string);
+        res = sss_replace_space(mem_ctx, data_set[i].input,
+                                data_set[i].replace_char);
         assert_non_null(res);
         assert_string_equal(res, data_set[i].output);
         talloc_zfree(res);
@@ -100,46 +84,43 @@ void test_reverse_replace_whitespaces(void **state)
     struct {
         const char *input;
         const char *output;
-        const char *replace_string;
+        const char replace_char;
     } data_set[] = {
-        { "", "", "-" },
-        { "-", " ", "-" },
-        { "----", "    ", "-" },
-        { "abcd", "abcd", "-" },
-        { "a-b-c-d", "a b c d", "-" },
-        { "-a-b-c-d-", " a b c d ", "-" },
-        { "^", " ", "^" },
-        { "^^^^", "    ", "^" },
-        { "abcd", "abcd", "^" },
-        { "a^b^c^d", "a b c d", "^" },
-        { "^a^b^c^d^", " a b c d ", "^" },
-        { ";-)", " ", ";-)" },
-        { ";-);-);-);-)", "    ", ";-)" },
-        { "abcd", "abcd", ";-)" },
-        { "a;-)b;-)c;-)d", "a b c d", ";-)" },
-        { ";-)a;-)b;-)c;-)d;-)", " a b c d ", ";-)" },
-        { " ", " ", " " },
-        { "    ", "    ", " " },
-        { "abcd", "abcd", " " },
-        { "a b c d", "a b c d", " " },
-        { " a b c d ", " a b c d ", " " },
-        { NULL, NULL, NULL },
+        { "", "", '-' },
+        { "-", " ", '-' },
+        { "----", "    ", '-' },
+        { "abcd", "abcd", '-' },
+        { "a-b-c-d", "a b c d", '-' },
+        { "-a-b-c-d-", " a b c d ", '-' },
+        { "^", " ", '^' },
+        { "^^^^", "    ", '^' },
+        { "abcd", "abcd", '^' },
+        { "a^b^c^d", "a b c d", '^' },
+        { "^a^b^c^d^", " a b c d ", '^' },
+        { " ", " ", ' ' },
+        { "    ", "    ", ' ' },
+        { "abcd", "abcd", ' ' },
+        { "a b c d", "a b c d", ' ' },
+        { " a b c d ", " a b c d ", ' ' },
+        { NULL, NULL, '\0' },
     };
 
     mem_ctx = talloc_new(NULL);
     assert_non_null(mem_ctx);
     check_leaks_push(mem_ctx);
 
-    res = sss_reverse_replace_whitespaces(mem_ctx, input_str, NULL);
-    assert_true(res == input_str);
+    res = sss_reverse_replace_space(mem_ctx, input_str, '\0');
+    assert_string_equal(res, input_str);
+    talloc_free(res);
 
-    res = sss_reverse_replace_whitespaces(mem_ctx, input_str, "");
-    assert_true(res == input_str);
+    res = sss_reverse_replace_space(mem_ctx, input_str, '\0');
+    assert_string_equal(res, input_str);
+    talloc_free(res);
 
     for (i=0; data_set[i].input != NULL; ++i) {
         input_str = discard_const_p(char, data_set[i].input);
-        res = sss_reverse_replace_whitespaces(mem_ctx, input_str,
-                                      data_set[i].replace_string);
+        res = sss_reverse_replace_space(mem_ctx, input_str,
+                                        data_set[i].replace_char);
         assert_non_null(res);
         assert_string_equal(res, data_set[i].output);
         talloc_zfree(res);
diff --git a/src/util/string_utils.c b/src/util/string_utils.c
index 8d878923f..ec4cc687e 100644
--- a/src/util/string_utils.c
+++ b/src/util/string_utils.c
@@ -22,129 +22,44 @@
 
 #include "util/util.h"
 
-const char * sss_replace_whitespaces(TALLOC_CTX *mem_ctx,
-                                     const char *orig_name,
-                                     const char *replace_string)
+static char *replace_char(TALLOC_CTX *mem_ctx,
+                          const char *in,
+                          const char match,
+                          const char sub)
 {
-    char *new_name;
-    const char *ptr;
-    size_t replace_string_len;
-    TALLOC_CTX *tmp_ctx;
+    char *p;
+    char *out;
 
-    if (replace_string == NULL || replace_string[0] == '\0') {
-        return orig_name;
-    }
-
-    replace_string_len = strlen(replace_string);
-    /* faster implementations without multiple allocations */
-    if (replace_string_len == 1) {
-        char *p;
-        new_name = talloc_strdup(mem_ctx, orig_name);
-        if (new_name == NULL) {
-            return NULL;
-        }
-
-        for (p = new_name; *p != '\0'; ++p) {
-            if (isspace(*p)) {
-                *p = replace_string[0];
-            }
-        }
-
-        return new_name;
-    }
-
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
+    out = talloc_strdup(mem_ctx, in);
+    if (out == NULL) {
         return NULL;
     }
 
-    new_name = talloc_strdup(tmp_ctx, "");
-    if (new_name == NULL) {
-        goto done;
-    }
-
-    ptr = orig_name;
-    while (*ptr != '\0') {
-        if (isspace(*ptr)) {
-            new_name = talloc_asprintf_append(new_name, "%s", replace_string);
-        } else {
-            new_name = talloc_asprintf_append(new_name, "%c", *ptr);
-        }
-        if (new_name == NULL) {
-            goto done;;
+    for (p = out; *p != '\0'; ++p) {
+        if (*p == match) {
+            *p = sub;
         }
-
-        ++ptr;
     }
 
-    new_name = talloc_steal(mem_ctx, new_name);
-done:
-    talloc_free(tmp_ctx);
-    return new_name;
+    return out;
 }
 
-char * sss_reverse_replace_whitespaces(TALLOC_CTX *mem_ctx,
-                                       char *orig_name,
-                                       const char *replace_string)
+char * sss_replace_space(TALLOC_CTX *mem_ctx,
+                         const char *orig_name,
+                         const char subst)
 {
-    TALLOC_CTX *tmp_ctx;
-    char *substr;
-    char *new_name;
-    const char *ptr = orig_name;
-    size_t replace_string_len;
-
-    if (replace_string == NULL || replace_string[0] == '\0') {
-        return orig_name;
-    }
-
-    replace_string_len = strlen(replace_string);
-    /* faster implementations without multiple allocations */
-    if (replace_string_len == 1) {
-        char *p;
-        new_name = talloc_strdup(mem_ctx, orig_name);
-        if (new_name == NULL) {
-            return NULL;
-        }
-
-        for (p = new_name; *p != '\0'; ++p) {
-            if (*p == replace_string[0] ) {
-                *p = ' ';
-            }
-        }
-
-        return new_name;
-    }
-
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
-        return NULL;
+    if (subst == '\0') {
+        return talloc_strdup(mem_ctx, orig_name);
     }
+    return replace_char(mem_ctx, orig_name, ' ', subst);
+}
 
-    new_name = talloc_strdup(tmp_ctx, "");
-    if (new_name == NULL) {
-        goto done;
+char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
+                                 char *orig_name,
+                                 const char subst)
+{
+    if (subst == '\0') {
+        return talloc_strdup(mem_ctx, orig_name);
     }
-
-    do {
-        substr = strstr(ptr, replace_string);
-        if (substr != NULL) {
-            new_name = talloc_asprintf_append(new_name, "%.*s ",
-                                              (int)(substr - ptr), ptr);
-            if (new_name == NULL) {
-                goto done;
-            }
-            ptr += substr - ptr;
-            ptr += replace_string_len;
-        } else {
-            new_name = talloc_asprintf_append(new_name, "%s", ptr);
-            if (new_name == NULL) {
-                goto done;
-            }
-        }
-    } while (substr != NULL);
-
-    new_name = talloc_steal(mem_ctx, new_name);
-done:
-    talloc_free(tmp_ctx);
-    return new_name;
+    return replace_char(mem_ctx, orig_name, subst, ' ');
 }
diff --git a/src/util/util.h b/src/util/util.h
index b638df99a..ca740d016 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -566,11 +566,11 @@ errno_t name_to_well_known_sid(const char *dom, const char *name,
                                const char **sid);
 
 /* from string_utils.c */
-const char * sss_replace_whitespaces(TALLOC_CTX *mem_ctx,
-                                     const char *orig_name,
-                                     const char *replace_string);
-char * sss_reverse_replace_whitespaces(TALLOC_CTX *mem_ctx,
-                                       char *orig_name,
-                                       const char *replace_string);
+char * sss_replace_space(TALLOC_CTX *mem_ctx,
+                         const char *orig_name,
+                         const char replace_char);
+char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
+                                 char *orig_name,
+                                 const char replace_char);
 
 #endif /* __SSSD_UTIL_H__ */