Martin Kosek wrote:
On Mon, 2012-04-16 at 13:51 -0400, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
On 10.4.2012 10:57, Martin Kosek wrote:
Few test hints are attached to the ticket.
---
ldap2 plugin returns NotFound error for find_entries/get_entry
queries when the server did not manage to return an entry
due to time limits. This may be confusing for user when the
entry he searches actually exists.
This patch fixes the behavior in ldap2 plugin to return
LimitsExceeded exception instead. This way, user would know
that his time/size limits are set too low and can amend them to
get correct results.
https://fedorahosted.org/freeipa/ticket/2606
ACK.
Honza
Before pushing I'd like to look at this more. truncated is supposed to
indicate a limits problem. I want to see if the caller should be
responsible for returning a limits error instead.
rob
This is what I had in mind.
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 61341b0..447e738 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -754,7 +754,7 @@ class ldap2(CrudBackend, Encoder):
except _ldap.LDAPError, e:
_handle_errors(e)
- if not res:
+ if not res and not truncated:
raise errors.NotFound(reason='no such entry')
if attrs_list and ('memberindirect' in attrs_list or '*' in
attrs_list)
:
@@ -801,7 +801,10 @@ class ldap2(CrudBackend, Encoder):
if len(entries)> 1:
raise errors.SingleMatchExpected(found=len(entries))
else:
- return entries[0]
+ if truncated:
+ raise errors.LimitsExceeded()
+ else:
+ return entries[0]
def get_entry(self, dn, attrs_list=None, time_limit=None,
size_limit=None, normalize=True):
@@ -811,10 +814,13 @@ class ldap2(CrudBackend, Encoder):
Keyword arguments:
attrs_list - list of attributes to return, all if None
(default None)
"""
- return self.find_entries(
+ (entry, truncated) = self.find_entries(
None, attrs_list, dn, self.SCOPE_BASE, time_limit=time_limit,
size_limit=size_limit, normalize=normalize
- )[0][0]
+ )
+ if truncated:
+ raise errors.LimitsExceeded()
+ return entry[0]
config_defaults = {'ipasearchtimelimit': [2],
'ipasearchrecordslimit': [0]}
def get_ipa_config(self, attrs_list=None):
Thanks for the patch. I had similar patch planned in my mind.
We just need to be more careful, this patch will change an assumption
that ldap2.find_entries will always either raise NotFound error or
return at least one result.
I checked all ldap2.find_entries calls and did few fixes, it should be
OK now. No regression found in unit tests. The updated patch is
attached.
Martin
ACK, pushed to master and ipa-2-2
rob
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
