On (24/11/14 20:50), Michal Židek wrote:
> On 11/22/2014 05:48 PM, Jakub Hrozek wrote:
>> On Fri, Nov 21, 2014 at 10:26:18PM +0100, Michal Židek wrote:
>>> Hi,
>>>
>>> please see attached patch for ticket
>>>
https://fedorahosted.org/sssd/ticket/2487
>>>
>>> Probably in some part of the code we call the
>>> function sss_get_domain_name with FQDN. We use
>>> regular expression to check if it is the case and
>>> if not (and regex is configured not to accept
>>> non FQDN) than this function fails to construct the
>>> requested name in the output.
>>>
>>> Since the sss_parse_name is here only used to detect
>>> if the name contains domain portion, it is safe
>>> to assume it does not contain it if regular
>>> expression has failed to match anything.
>>>
>>> Michal
>>
>>> From 21dc9d402501f6728f743d082f1046ac558195bd Mon Sep 17 00:00:00 2001
>>> From: Michal Zidek <mzidek(a)redhat.com>
>>> Date: Fri, 21 Nov 2014 20:06:32 +0100
>>> Subject: [PATCH 1/2] util: sss_get_domain_name regex mismatch not fatal
>>>
>>> Assume name is not FQDN if sss_parse_name fails to
>>> match domain with regular expression.
>>>
>>> Fixes:
>>>
https://fedorahosted.org/sssd/ticket/2487
>>> ---
>>> src/util/usertools.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/util/usertools.c b/src/util/usertools.c
>>> index a0b914e..4168421 100644
>>> --- a/src/util/usertools.c
>>> +++ b/src/util/usertools.c
>>> @@ -646,7 +646,13 @@ sss_get_domain_name(TALLOC_CTX *mem_ctx,
>>> /* check if the name already contains domain part */
>>> if (dom->names != NULL) {
>>> ret = sss_parse_name(mem_ctx, dom->names, orig_name,
&domain, NULL);
>>> - if (ret != EOK) {
>>> + if (ret == EINVAL) {
>>> + DEBUG(SSSDBG_TRACE_FUNC,
>>> + "sss_parse_name could not parse domain from [%s].
"
>>> + "Assuming it is not FQDN.\n", orig_name);
>>
>> What do you think about returning a specific return code from
>> sss_parse_name instead of EINVAL in all cases?
>
> Ok, I added specific error code for the case when regular expression
> does not match anything. In other cases EINVAL is still used.
>
>>
>> Did you check other callers of sss_parse_name()/sss_get_domain_name() ?
>
> Yes. We do not look explicitly for EINVAL being returned in the
> code after these functions are called, so creating the new ERR
> code should not affect anything.
>
> Michal
I don't think problem was in function sss_parse_name.
The problem is how it this function used (indirectly)
sss_parse_name was called twice.
the 1st time with fully qualified user name user@domain.
and the 2nd time with extracted name(which was not fqdn) from 1st call.
LS
I disagree. You are right that the function was called twice,
once with the FQDN and once with the extracted name. But the
function sss_get_domain_name should not fail on simple check if
the name has/has not domain part (and it uses sss_parse_name
for this check). So the problem indeed was in the way
sss_get_domain_name was behaving.
I do not know when this check was added to this function,
but it makes sense that the function should accept both
FQDN and non-FQDN names (so the check makes sense).
Michal