[Freeipa-devel] [PATCH 0058] Make get_entries not ignore its size_limit argument
Stanislav Laznicka
slaznick at redhat.com
Tue Aug 2 11:47:17 UTC 2016
On 07/19/2016 09:20 AM, Jan Cholasta wrote:
> Hi,
>
> On 14.7.2016 14:36, Stanislav Laznicka wrote:
>> Hello,
>>
>> This patch fixes https://fedorahosted.org/freeipa/ticket/5640.
>>
>> With not so much experience with the framework, it raises question in my
>> head whether ipaldap.get_entries is used properly throughout the system
>> - does it always assume that it gets ALL the requested entries or just a
>> few of those as configured by the 'ipaSearchRecordsLimit' attribute of
>> ipaConfig.etc which it actually gets?
>
> That depends. If you call get_entries() on the ldap2 plugin (which is
> usually the case in the framework), then ipaSearchRecordsLimit is
> used. If you call it on some arbitrary LDAPClient instance, the
> hardcoded default (= unlimited) is used.
>
>>
>> One spot that I know the get_entries method was definitely not used
>> properly before this patch is in the
>> baseldap.LDAPObject.get_memberindirect() method:
>>
>> 692 result = self.backend.get_entries(
>> 693 self.api.env.basedn,
>> 694 filter=filter,
>> 695 attrs_list=['member'],
>> 696 size_limit=-1, # paged search will get everything
>> anyway
>> 697 paged_search=True)
>>
>> which to me seems kind of important if the environment size_limit is not
>> set properly :) The patch does not fix the non-propagation of the
>> paged_search, though.
>
> Why do you think size_limit is not used properly here?
AFAIU it is desired that the search is unlimited. However, due to the
fact that neither size_limit nor paged_search are passed from
ldap2.get_entries() to ldap2.find_entries() (methods inherited from
LDAPClient), only the number of records specified by
ipaSearchRecordsLimit is returned. That could eventually cause problems
should ipaSearchRecordsLimit be set to a low value as in the ticket.
>
> Anyway, this ticket is not really easily fixable without more profound
> changes. Often, multiple LDAP searches are done during command
> execution. What do you do with the size limit then? Do you pass the
> same size limit to all the searches? Do you subtract the result size
> from the size limit after each search? Do you do something else with
> it? ... The answer is that it depends on the purpose of each
> individual LDAP search (like in get_memberindirect() above, we have to
> do unlimited search, otherwise the resulting entry would be
> incomplete), and fixing this accross the whole framework is a
> non-trivial task.
>
I do realize that the proposed fix for the permission plugin is not
perfect, it would probably be better to subtract the number of currently
loaded records from the sizelimit, although in the end the number of
returned values will not be higher than the given size_limit. However,
it seems reasonable that if get_entries is passed a size limit, it
should apply it over current ipaSearchRecordsLimit rather than ignoring
it. Then, any use of get_entries could be fixed accordingly if someone
sees fit.
More information about the Freeipa-devel
mailing list