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(a)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(a)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