[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