On (10/08/14 19:11), Jakub Hrozek wrote:
On Fri, Aug 08, 2014 at 10:48:11AM +0200, Lukas Slebodnik wrote:
> 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.
Done as well as all the other changes you requested. Please see the
attached patches.
From 830aff639d7cfe859b977486ca41baa0ae613e0c Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Fri, 1 Aug 2014 07:31:39 +0200
Subject: [PATCH 4/6] IFP: Use the override_default_space option
https://fedorahosted.org/sssd/ticket/2397
The input of the InfoPipe responder substitutes the configured character
for space and the GetUserAttrs and GetUserGroups functions substitute
space for the configured character in their output.
In my opinion, it is not necessary to replace spaces in infopipe responder.
dbus utilities already returned quoted strings.
# gdbus call --system --dest org.freedesktop.sssd.infopipe --object-path
/org/freedesktop/sssd/infopipe --method
org.freedesktop.sssd.infopipe.GetUserGroups 'usersssd25'
(['biggroup 1', 'small group 1', 'big group 2'],)
There will not be such problem with parsing output as with other
commands: id, groups ...
---
src/responder/ifp/ifpsrv_cmd.c | 53 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 51 insertions(+), 2 deletions(-)
diff --git a/src/responder/ifp/ifpsrv_cmd.c b/src/responder/ifp/ifpsrv_cmd.c
index 50483933188cfe9cf1a855bf3505f7e24a91b975..15ecaef65fb865b98d16566aa35c79c4b29ea96c
100644
--- a/src/responder/ifp/ifpsrv_cmd.c
+++ b/src/responder/ifp/ifpsrv_cmd.c
@@ -174,6 +174,26 @@ static void ifp_user_get_attr_process(struct tevent_req *req)
}
static errno_t
+ifp_user_get_attr_replace_space(TALLOC_CTX *mem_ctx,
+ struct ldb_message_element *el,
+ const char sub)
+{
+ int i;
+
+ for (i = 0; i < el->num_values; i++) {
+ el->values[i].data = (uint8_t *) sss_replace_space(mem_ctx,
+ (const char *) el->values[i].data,
+ sub);
+ if (el->values[i].data == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "sss_replace_space failed,
skipping\n");
+ return ENOMEM;
+ }
+ }
+
+ return EOK;
+}
+
+static errno_t
ifp_user_get_attr_handle_reply(struct ifp_req *ireq,
const char **attrs, struct ldb_result *res)
{
@@ -214,6 +234,18 @@ ifp_user_get_attr_handle_reply(struct ifp_req *ireq,
continue;
}
+ /* Normalize white space in user names */
+ if (ireq->ifp_ctx->rctx->override_space != '\0' &&
+ strcmp(attrs[ai], SYSDB_NAME) == 0) {
+ ret = ifp_user_get_attr_replace_space(ireq, el,
+ ireq->ifp_ctx->rctx->override_space);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n",
+ attrs[ai]);
+ continue;
+ }
+ }
+
ret = ifp_add_ldb_el_to_dict(&iter_dict, el);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE,
@@ -343,8 +375,17 @@ ifp_user_get_groups_reply(struct ifp_req *ireq, struct ldb_result
*res)
continue;
}
- DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n", name);
- groupnames[i] = name;
+ if (ireq->ifp_ctx->rctx->override_space != '\0') {
+ groupnames[i] = sss_replace_space(groupnames, name,
+ ireq->ifp_ctx->rctx->override_space);
+ if (groupnames[i] == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n", name);
+ continue;
+ }
+ } else {
+ groupnames[i] = name;
+ }
+ DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n", groupnames[i]);
}
return infopipe_iface_GetUserGroups_finish(ireq->dbus_req,
@@ -495,6 +536,14 @@ static errno_t ifp_user_get_attr_search(struct tevent_req *req)
name = sss_get_cased_name(state, state->name, dom->case_sensitive);
if (!name) return ENOMEM;
+ state->name = sss_reverse_replace_space(state, name,
+ state->rctx->override_space);
^^^^^^^^^^^
sanitized string is sored in variable state->name, but variable 'name'
is tested for NULL and used further incode.
+ if (name == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "sss_reverse_replace_space failed\n");
+ return ENOMEM;
+ }
+
/* verify this user has not yet been negatively cached,
* or has been permanently filtered */
ret = sss_ncache_check_user(state->ncache,
It is your decision. You can:
a) fix problem in function ifp_user_get_attr_search
b) remove changes in IFP responder.
LS