Hi,
the attached patch fixes an issue found by Jan P. in the IFP code.
On 12/01/2014 11:07 PM, Jakub Hrozek wrote:
Hi,
the attached patch fixes an issue found by Jan P. in the IFP code.
0001-IFP-Return-group-names-with-the-right-case.patch
From fa7fdf4df1c29bd066fa32b98b079a2ae24ab3fc Mon Sep 17 00:00:00 2001 From: Jakub Hrozekjhrozek@redhat.com Date: Mon, 1 Dec 2014 23:00:06 +0100 Subject: [PATCH] IFP: Return group names with the right case
The IFP code wasn't honoring the case settings of the domain.
src/responder/ifp/ifpsrv_cmd.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/responder/ifp/ifpsrv_cmd.c b/src/responder/ifp/ifpsrv_cmd.c index 3b7442233a5b4d01f430c9ffdbcdc949637d6db9..5e2be20e8ad81e2b651f1e02ce58974bfaa56984 100644 --- a/src/responder/ifp/ifpsrv_cmd.c +++ b/src/responder/ifp/ifpsrv_cmd.c @@ -370,6 +370,7 @@ ifp_user_get_groups_reply(struct sss_domain_info *domain, int i, num; const char *name; const char **groupnames;
const char *tmpstr;
/* one less, the first one is the user entry */ num = res->count - 1;
@@ -388,15 +389,23 @@ ifp_user_get_groups_reply(struct sss_domain_info *domain, }
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) {
tmpstr = sss_replace_space(groupnames, name,
ireq->ifp_ctx->rctx->override_space);
Sure, you can allocate tmpstr on groupnames, I think ireq or tmp_ctx would be better choice.
if (tmpstr == NULL) { DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n", name); continue; } } else {
groupnames[i] = name;
tmpstr = name; }
groupnames[i] = sss_get_cased_name(ireq, tmpstr, domain->case_preserve);
Here, groupnames should be used as talloc context instead of ireq. sss_get_cased_names duplicates the string so tmpstr should be freed.
if (groupnames[i] == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,
"sss_get_cased_name failed, skipping\n");
continue;
}
DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n", groupnames[i]); }
On Wed, Dec 03, 2014 at 04:33:33PM +0100, Pavel Březina wrote:
if (groupnames[i] == NULL) {
tmpstr = sss_replace_space(groupnames, name,
ireq->ifp_ctx->rctx->override_space);
Sure, you can allocate tmpstr on groupnames, I think ireq or tmp_ctx would be better choice.
if (tmpstr == NULL) { DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n", name); continue; } } else {
groupnames[i] = name;
tmpstr = name; }
groupnames[i] = sss_get_cased_name(ireq, tmpstr, domain->case_preserve);
Here, groupnames should be used as talloc context instead of ireq. sss_get_cased_names duplicates the string so tmpstr should be freed.
if (groupnames[i] == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,
"sss_get_cased_name failed, skipping\n");
continue;
}
}DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n", groupnames[i]);
Both suggestions make sense, please see the attached patch. Thank you for the review.
On 12/03/2014 05:58 PM, Jakub Hrozek wrote:
On Wed, Dec 03, 2014 at 04:33:33PM +0100, Pavel Březina wrote:
if (groupnames[i] == NULL) {
tmpstr = sss_replace_space(groupnames, name,
ireq->ifp_ctx->rctx->override_space);
Sure, you can allocate tmpstr on groupnames, I think ireq or tmp_ctx would be better choice.
if (tmpstr == NULL) { DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n", name); continue; } } else {
groupnames[i] = name;
tmpstr = name; }
groupnames[i] = sss_get_cased_name(ireq, tmpstr, domain->case_preserve);
Here, groupnames should be used as talloc context instead of ireq. sss_get_cased_names duplicates the string so tmpstr should be freed.
if (groupnames[i] == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,
"sss_get_cased_name failed, skipping\n");
continue;
}
DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n", groupnames[i]); }
Both suggestions make sense, please see the attached patch. Thank you for the review.
Ack.
On Thu, Dec 04, 2014 at 12:51:04PM +0100, Pavel Březina wrote:
On 12/03/2014 05:58 PM, Jakub Hrozek wrote:
On Wed, Dec 03, 2014 at 04:33:33PM +0100, Pavel Březina wrote:
if (groupnames[i] == NULL) {
tmpstr = sss_replace_space(groupnames, name,
ireq->ifp_ctx->rctx->override_space);
Sure, you can allocate tmpstr on groupnames, I think ireq or tmp_ctx would be better choice.
if (tmpstr == NULL) { DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n", name); continue; } } else {
groupnames[i] = name;
tmpstr = name; }
groupnames[i] = sss_get_cased_name(ireq, tmpstr, domain->case_preserve);
Here, groupnames should be used as talloc context instead of ireq. sss_get_cased_names duplicates the string so tmpstr should be freed.
if (groupnames[i] == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,
"sss_get_cased_name failed, skipping\n");
continue;
}
}DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n", groupnames[i]);
Both suggestions make sense, please see the attached patch. Thank you for the review.
Ack.
* master: 1b4bd7e378deda73a18d86e4b2998bff45883e7b
sssd-devel@lists.fedorahosted.org