[Freeipa-devel] [PATCH 0143-0144] different errors/warnings for different LDAP limit type exceeded

[Rob Crittenden]

Martin Babinsky wrote:
> On 03/21/2016 12:25 PM, Jan Cholasta wrote:
>> On 21.3.2016 10:17, Petr Spacek wrote:
>>> On 18.3.2016 13:49, Rob Crittenden wrote:
>>>> Martin Babinsky wrote:
>>>>> These patches implement behavior agreed upon during discussion of
>>>>> https://fedorahosted.org/freeipa/ticket/5677
>>>>>
>>>>> However I'm not sure if we want to push them into 4-3 branch (the
>>>>> ticket
>>>>> is triaged into 4.3.2 milestone) since they modify the framework
>>>>> behavior quite a bit.
>>>>>
>>>>> If there is no need to have it there (CC'ing Milan since he is the
>>>>> reporter), I would retriage it into 4.4 milestone.
>>>>
>>>>
>>>> + desc="while getting entries (search base: '{}',"
>>>> + "filter: {})".format(base_dn, filter))
>>>>
>>>> This is going to expose parts of the DIT in an error message to
>>>> users. We have
>>>> tried in the past to hide the implementation. I'd propose logging the
>>>> error
>>>> and making the exception less verbose.
>>
>> I agree with Rob here, we shouldn't expose internal stuff in error
>> messages for users.
>>
>> In this particular case, even if we included internal stuff in the error
>> message, it should be the error message returned by the server rather
>> than this ad-hoc message.
>>
>>>
>>> IMHO it actually helps to print the DN. At very least the user can see
>>> if the
>>> error is happening always with the same DN or if it keeps changing.
>>>
>>> In other words, for user it is not that important to understand
>>> meaning of the
>>> DN but it might be important to see if it is the same or not.
>>
>> I can't imagine a situation where it would actually be useful for the
>> user (as opposed to the admin, who has access to logs) to know the base
>> DN of some arbitrary LDAP search operation. Could you give an example?
>>
> Right, attaching updated patches.

I may have suggested debug logging the detailed error. I was wrong. This 
should log at the error level so it always appears in the logs. This may 
be a spurious error and having the user turn on debug logging to capture 
the reasons would be asking a lot.

rob