[SSSD] [PATCH] Make the replace-space functionality usable for other responders

Lukas Slebodnik lslebodn at redhat.com
Fri Aug 8 08:48:11 UTC 2014


On (07/08/14 19:46), Jakub Hrozek wrote:
>Hi,
>
>the attached patches change the replace-space-in-name functionality
>after discussion on the list. In short:
>    - only space can be substituted
>    - for only one character, not a string
>    - the option is now set in the [sssd] section because it works for
>      any responder
>
>For more details, please see the attached patches.

>From 4ebeea258ec01cf08bebfa4a5a110320c3c0898d Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Tue, 5 Aug 2014 10:12:34 +0200
>Subject: [PATCH 1/6] 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
>---

//snip

>--- 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_string);
>+char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
>+                                 char *orig_name,
>+                                 const char *replace_string);
> 
> #endif /* __SSSD_UTIL_H__ */
>-- 
>1.9.3
>

I don't know why did you change the same lines twice:
    1st time in this patch: functions werere renamed.
    2nt time in next patch "const char *replace_string" was replaced with
    character.
In my opinion, it can be done in one patch.

>From ba2d408f14de29b2b1357998fb3df812e264c453 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Thu, 7 Aug 2014 15:28:11 +0200
>Subject: [PATCH 2/6] Make the space override responder-agnostic
>
>https://fedorahosted.org/sssd/ticket/2397
>
>In order to make the override_space option usable by other responders,
>we need to move the override_space option to the generic responder
>structure.
>---
> src/confdb/confdb.h                     |  2 +-
> src/responder/common/responder.h        |  1 +
> src/responder/common/responder_common.c | 15 +++++++
> src/responder/nss/nsssrv.c              |  5 ---
> src/responder/nss/nsssrv.h              |  1 -
> src/responder/nss/nsssrv_cmd.c          | 17 +++++---
> src/tests/cmocka/test_nss_srv.c         |  1 +
> src/tests/cmocka/test_string_utils.c    | 76 ++++++++++++++++-----------------
> src/util/string_utils.c                 | 12 +++---
> src/util/util.h                         |  4 +-
> 10 files changed, 74 insertions(+), 60 deletions(-)
>
>diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
>index d5f7511d0cfe916242b5bfae506271e028a87edb..9160debd6f4b5a35ac94ec29dc863ed80a0e337e 100644
>--- a/src/confdb/confdb.h
>+++ b/src/confdb/confdb.h
>@@ -68,6 +68,7 @@
> #define CONFDB_MONITOR_TRY_INOTIFY "try_inotify"
> #define CONFDB_MONITOR_KRB5_RCACHEDIR "krb5_rcache_dir"
> #define CONFDB_MONITOR_DEFAULT_DOMAIN "default_domain_suffix"
>+#define CONFDB_MONITOR_OVERRIDE_SPACE "override_default_space"
> 
> /* Both monitor and domains */
> #define CONFDB_NAME_REGEX   "re_expression"
>@@ -99,7 +100,6 @@
> #define CONFDB_MEMCACHE_TIMEOUT "memcache_timeout"
> #define CONFDB_NSS_HOMEDIR_SUBSTRING "homedir_substring"
> #define CONFDB_DEFAULT_HOMEDIR_SUBSTRING "/home"
>-#define CONFDB_NSS_OVERRIDE_SPACE "override_space"
> 
> /* PAM */
> #define CONFDB_PAM_CONF_ENTRY "config/pam"
>diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
>index 167f45cb156a189deb996d28f8165529535607de..3674d13f2303d0ce248f765a638aaa83d0c16cf3 100644
>--- a/src/responder/common/responder.h
>+++ b/src/responder/common/responder.h
>@@ -102,6 +102,7 @@ struct resp_ctx {
>     uid_t *allowed_uids;
> 
>     char *default_domain;
>+    char override_space;
> 
>     void *pvt_ctx;
> 
>diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
>index 2f9db70ce22ef224e744cad80d89639340ec1a79..3afeb3cc6cb3a9fc4bd72864d45a0d0bb0bea469 100644
>--- a/src/responder/common/responder_common.c
>+++ b/src/responder/common/responder_common.c
>@@ -781,6 +781,7 @@ int sss_process_init(TALLOC_CTX *mem_ctx,
>     struct resp_ctx *rctx;
>     struct sss_domain_info *dom;
>     int ret;
>+    char *tmp = NULL;
> 
>     rctx = talloc_zero(mem_ctx, struct resp_ctx);
>     if (!rctx) {
>@@ -844,6 +845,20 @@ int sss_process_init(TALLOC_CTX *mem_ctx,
>         goto fail;
>     }
> 
>+    ret = confdb_get_string(rctx->cdb, rctx, CONFDB_MONITOR_CONF_ENTRY,
>+                            CONFDB_MONITOR_OVERRIDE_SPACE, NULL,
>+                            &tmp);
>+    if (ret != EOK) {
>+        DEBUG(SSSDBG_OP_FAILURE,
>+              "Cannnot get the space substitution character [%d]: %s\n",
>+               ret, strerror(ret));
>+        goto fail;
>+    }
>+
>+    if (tmp != NULL) {
>+        rctx->override_space = tmp[0];
>+    }
>+
It would be good to write debug message if size of string is longer that 1.

>     ret = sss_monitor_init(rctx, rctx->ev, monitor_intf,
>                            svc_name, svc_version, rctx,
>                            &rctx->mon_conn);
>diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
>index 9705878fa80d33d3bddfe7be7fcc97151b44a0f0..84a6b7fedd096a7d4159b7ac6670820c1d8fd941 100644
>--- a/src/responder/nss/nsssrv.c
>+++ b/src/responder/nss/nsssrv.c
>@@ -298,11 +298,6 @@ static int nss_get_config(struct nss_ctx *nctx,
>                             &nctx->homedir_substr);
>     if (ret != EOK) goto done;
> 
>-    ret = confdb_get_string(cdb, nctx, CONFDB_NSS_CONF_ENTRY,
>-                            CONFDB_NSS_OVERRIDE_SPACE, NULL,
>-                            &nctx->override_space);
>-    if (ret != EOK) goto done;
>-
>     ret = 0;
> done:
>     return ret;
>diff --git a/src/responder/nss/nsssrv.h b/src/responder/nss/nsssrv.h
>index f5238fb822ecc376d8dca06c00fb989276949688..a5b946b7e4a38d7d8b35ec5df1b6644d01896470 100644
>--- a/src/responder/nss/nsssrv.h
>+++ b/src/responder/nss/nsssrv.h
>@@ -69,7 +69,6 @@ struct nss_ctx {
>     char **etc_shells;
>     char *shell_fallback;
>     char *default_shell;
>-    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 8e9a7dbaf22af995646738f18ccad6168c10b29e..d92882564e3f59e9927011f1797a1948553f9423 100644
>--- a/src/responder/nss/nsssrv_cmd.c
>+++ b/src/responder/nss/nsssrv_cmd.c
>@@ -372,7 +372,8 @@ static int fill_pwent(struct sss_packet *packet,
>             continue;
>         }
> 
>-        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->override_space);
>+        tmpstr = sss_replace_space(tmp_ctx, tmpstr,
>+                                   nctx->rctx->override_space);
>         if (tmpstr == NULL) {
>             DEBUG(SSSDBG_CRIT_FAILURE,
>                   "sss_replace_space failed, skipping\n");
>@@ -752,7 +753,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx)
>         if (!name) return ENOMEM;
> 
>         name = sss_reverse_replace_space(dctx, name,
>-                                         nctx->override_space);
>+                                         nctx->rctx->override_space);
>         if (name == NULL) {
>             DEBUG(SSSDBG_CRIT_FAILURE,
>                   "sss_reverse_replace_space failed\n");
>@@ -2360,7 +2361,7 @@ static int fill_members(struct sss_packet *packet,
>             continue;
>         }
> 
>-        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->override_space);
>+        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->rctx->override_space);
>         if (tmpstr == NULL) {
>             DEBUG(SSSDBG_CRIT_FAILURE,
>                   "sss_replace_space failed\n");
>@@ -2523,7 +2524,7 @@ static int fill_grent(struct sss_packet *packet,
>             continue;
>         }
> 
>-        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->override_space);
>+        tmpstr = sss_replace_space(tmp_ctx, tmpstr, nctx->rctx->override_space);
>         if (tmpstr == NULL) {
>             DEBUG(SSSDBG_CRIT_FAILURE,
>                   "sss_replace_space failed, skipping\n");
>@@ -2724,7 +2725,8 @@ 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_space(dctx, name, nctx->override_space);
>+        name = sss_reverse_replace_space(dctx, name,
>+                                         nctx->rctx->override_space);
>         if (name == NULL) {
>             DEBUG(SSSDBG_CRIT_FAILURE,
>                   "sss_reverse_replace_space failed\n");
>@@ -3754,7 +3756,8 @@ 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_space(dctx, name, nctx->override_space);
>+        name = sss_reverse_replace_space(dctx, name,
>+                                         nctx->rctx->override_space);
>         if (name == NULL) {
>             DEBUG(SSSDBG_CRIT_FAILURE,
>                   "sss_reverse_replace_space failed\n");
>@@ -3921,7 +3924,7 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx)
>             }
> 
>             name = sss_reverse_replace_space(dctx, name,
>-                                             nctx->override_space);
>+                                             nctx->rctx->override_space);
>             if (name == NULL) {
>                 DEBUG(SSSDBG_CRIT_FAILURE,
>                       "sss_reverse_replace_space failed\n");
>diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
>index 808a7a1854cb80b6ffb3f84f26144083e8599bb5..f5d1f60ec62056b42c52789f27a38eebdf362657 100644
>--- a/src/tests/cmocka/test_nss_srv.c
>+++ b/src/tests/cmocka/test_nss_srv.c
>@@ -889,6 +889,7 @@ void test_nss_setup(struct sss_test_conf_param params[],
>     nss_test_ctx->rctx = mock_rctx(nss_test_ctx, nss_test_ctx->tctx->ev,
>                                    nss_test_ctx->tctx->dom, nss_test_ctx->nctx);
>     assert_non_null(nss_test_ctx->rctx);
>+    nss_test_ctx->nctx->rctx = nss_test_ctx->rctx;
> 
>     /* Create client context */
>     nss_test_ctx->cctx = mock_cctx(nss_test_ctx, nss_test_ctx->rctx);
>diff --git a/src/tests/cmocka/test_string_utils.c b/src/tests/cmocka/test_string_utils.c
>index dda9fac80be32b2dbbee8b365ca2f82c475a3b30..a52b9bb2180e5eb103d0a5cff4d5f939261b5414 100644
>--- a/src/tests/cmocka/test_string_utils.c
>+++ b/src/tests/cmocka/test_string_utils.c
>@@ -31,34 +31,34 @@ void test_replace_whitespaces(void **state)
>     struct {
>         const char *input;
>         const char *output;
>-        const char *replace_string;
>+        const char replace_string;
                             s/string/character/

>     } 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", " " },
>-        { 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_space(mem_ctx, input_str, NULL);
>+    res = sss_replace_space(mem_ctx, input_str, '\0');
>     assert_string_equal(res, input_str);
>     talloc_zfree(res);
> 
>-    res = sss_replace_space(mem_ctx, input_str, "");
>+    res = sss_replace_space(mem_ctx, input_str, '\0');
>     assert_string_equal(res, input_str);
>     talloc_zfree(res);
> 
>@@ -84,36 +84,36 @@ void test_reverse_replace_whitespaces(void **state)
>     struct {
>         const char *input;
>         const char *output;
>-        const char *replace_string;
>+        const char replace_string;
                             s/string/character/

>     } 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 ", " " },
>-        { 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_space(mem_ctx, input_str, NULL);
>+    res = sss_reverse_replace_space(mem_ctx, input_str, '\0');
>     assert_string_equal(res, input_str);
>     talloc_free(res);
> 
>-    res = sss_reverse_replace_space(mem_ctx, input_str, "");
>+    res = sss_reverse_replace_space(mem_ctx, input_str, '\0');
>     assert_string_equal(res, input_str);
>     talloc_free(res);
> 
>diff --git a/src/util/string_utils.c b/src/util/string_utils.c
>index d639c6de97275e53e019b60c8578f33ca22ec37f..ec4cc687e6652ee58e963869f7da3bd5f4cec2da 100644
>--- a/src/util/string_utils.c
>+++ b/src/util/string_utils.c
>@@ -46,20 +46,20 @@ static char *replace_char(TALLOC_CTX *mem_ctx,
> 
> char * sss_replace_space(TALLOC_CTX *mem_ctx,
>                          const char *orig_name,
>-                         const char *replace_string)
>+                         const char subst)
> {
>-    if (replace_string == NULL || replace_string[0] == '\0') {
>+    if (subst == '\0') {
>         return talloc_strdup(mem_ctx, orig_name);
>     }
>-    return replace_char(mem_ctx, orig_name, ' ', replace_string[0]);
>+    return replace_char(mem_ctx, orig_name, ' ', subst);
> }
> 
> char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
>                                  char *orig_name,
>-                                 const char *replace_string)
>+                                 const char subst)
> {
>-    if (replace_string == NULL || replace_string[0] == '\0') {
>+    if (subst == '\0') {
>         return talloc_strdup(mem_ctx, orig_name);
>     }
>-    return replace_char(mem_ctx, orig_name, replace_string[0], ' ');
>+    return replace_char(mem_ctx, orig_name, subst, ' ');
> }
>diff --git a/src/util/util.h b/src/util/util.h
>index c58e3fde62928b0f6b43698b8e20c058fd856df1..ca740d0163939f237dff0f80c5c8ab2ed183d07f 100644
>--- a/src/util/util.h
>+++ b/src/util/util.h
>@@ -568,9 +568,9 @@ errno_t name_to_well_known_sid(const char *dom, const char *name,
> /* from string_utils.c */
> char * sss_replace_space(TALLOC_CTX *mem_ctx,
>                          const char *orig_name,
>-                         const char *replace_string);
>+                         const char replace_char);
> char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
>                                  char *orig_name,
>-                                 const char *replace_string);
>+                                 const char replace_char);
> 
> #endif /* __SSSD_UTIL_H__ */
>-- 
>1.9.3

LS



More information about the sssd-devel mailing list