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 <sbose(a)redhat.com> -----
>
> Date: Wed, 6 Jul 2016 11:13:48 +0200
> From: Sumit Bose <sbose(a)redhat.com>
> To: sssd-devel(a)lists.fedorahosted.org
> Subject: Re: [SSSD] [PATCH] LDAP: Lookup services by all protocols unless a
> protocol is specified
> Message-ID: <20160706091348.GD29143(a)p.Speedport_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 <jhrozek(a)redhat.com>
>> 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.