On Thu, Jul 07, 2016 at 01:26:13PM +0200, Pavel Březina wrote: > On 07/07/2016 01:24 PM, Jakub Hrozek wrote: > > On Thu, Jul 07, 2016 at 12:39:28PM +0200, Pavel Březina wrote: > > > On 07/07/2016 12:34 PM, Jakub Hrozek wrote: > > > > On Thu, Jul 07, 2016 at 10:16:03AM +0200, Sumit Bose wrote: > > > > > resend > > > > > ----- Forwarded message from Sumit Bose <[email protected]> ----- > > > > > > > > > > Date: Wed, 6 Jul 2016 11:13:48 +0200 > > > > > From: Sumit Bose <[email protected]> > > > > > To: [email protected] > > > > > Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols > > > > > unless a > > > > > protocol is specified > > > > > Message-ID: > > > > > <[email protected]_W_724V_Typ_A_05011603_00_009> > > > > > References: <20160705103025.GB24232@hendrix> > > > > > MIME-Version: 1.0 > > > > > Content-Type: text/plain; charset=us-ascii > > > > > Content-Disposition: inline > > > > > In-Reply-To: <20160705103025.GB24232@hendrix> > > > > > User-Agent: Mutt/1.6.1 (2016-04-27) > > > > > > > > > > On Tue, Jul 05, 2016 at 12:30:25PM +0200, Jakub Hrozek wrote: > > > > > > Hi, > > > > > > > > > > > > the attached patch makes service lookups great again. > > > > > > > > > > > > To reproduce, just run: > > > > > > getent service -s sss ldap > > > > > > before the patch we would look up ipService="" because DP gives us > > > > > > an > > > > > > empty string after the recent DP patches. > > > > > > > > > > > From ba9834637b3cc0d7d98f704ba70f9dcb6f9a70e9 Mon Sep 17 00:00:00 > > > > > > 2001 > > > > > > From: Jakub Hrozek <[email protected]> > > > > > > Date: Tue, 5 Jul 2016 12:23:23 +0200 > > > > > > Subject: [PATCH] LDAP: Lookup services by all protocols unless a > > > > > > protocol is > > > > > > specified > > > > > > > > > > > > The DP refactoring changed the way we handle strings from sbus. We > > > > > > no > > > > > > longer receive NULL strings, but empty strings instead. > > > > > > --- > > > > > > src/providers/ldap/ldap_id_services.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/src/providers/ldap/ldap_id_services.c > > > > > > b/src/providers/ldap/ldap_id_services.c > > > > > > index > > > > > > 77215127b53297d840eaa4d2f35a75eedb085e43..db47e3fc55eea969371d61c3c5ac7f818196f3d5 > > > > > > 100644 > > > > > > --- a/src/providers/ldap/ldap_id_services.c > > > > > > +++ b/src/providers/ldap/ldap_id_services.c > > > > > > @@ -114,7 +114,7 @@ services_get_send(TALLOC_CTX *mem_ctx, > > > > > > ret = sss_filter_sanitize(state, name, &clean_name); > > > > > > if (ret != EOK) goto error; > > > > > > > > > > > > - if (protocol) { > > > > > > + if (protocol && protocol[0] != '\0') { > > > > > > ret = sss_filter_sanitize(state, protocol, > > > > > > &clean_protocol); > > > > > > if (ret != EOK) goto error; > > > > > > } > > > > > > > > > > since the sysdb calls later on in the request expect protocol==NULL as > > > > > well I would suggest something like > > > > > > > > > > state->sysdb = sdom->dom->sysdb; > > > > > state->name = name; > > > > > state->protocol = protocol; > > > > > + if (state->protocol[0] == '\0') { > > > > > + state->protocol = NULL; > > > > > + } > > > > > state->filter_type = filter_type; > > > > > state->noexist_delete = noexist_delete; > > > > > > > > > > > > > > > in only use state->protocol in the request. > > > > > > > > OK, done (we use state->protocol up to a point, then clean_protocol). I > > > > also added a check to cover the unlikely situation where sbus might give > > > > us a NULL string, just to error and don't crash. > > > > > > Hi, nice catch. I think it may be better to amend check_and_parse_filter() > > > in dp_target_id.c so it assign NULL instead of "" in data->filter_value > > > (and > > > other) using SBUS_SET_STRING(). Though I didn't check if other look up > > > types > > > (user/group/...) don't expect empty string instead. > > > > Yes, this makes sense and a quick git grep shows that the providers > > expect NULL, not empty string (for example for enumeration which we > > already special-case in check_and_parse_filter). But at this point, > > I would prefer to push this patch, release 1.14.0 and then patch > > check_and_parse_filter() and run tests again to make sure we don't break > > some strange corner case with a late sbus fix. > > > > Attached is a new patch that doesn't error out if protocol is NULL. > > Ok, I'm fine with this. The code looks good to me.
Looks like my mail in between got delayed, CI passed http://sssd-ci.duckdns.org/logs/job/47/43/summary.html ACK bye, Sumit > _______________________________________________ > sssd-devel mailing list > [email protected] > https://lists.fedorahosted.org/admin/lists/[email protected] _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
