Re: [Freeipa-devel] [PATCH] 248 Raise proper exception when LDAP limits are exceeded
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
From 44e711ac4d1668a887c6e2eebe4f4e0cb343033c Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 17 Apr 2012 09:56:04 +0200
Subject: [PATCH] Raise proper exception when LDAP limits are exceeded
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
1) Return even a zero search results + truncated bool set in
ldap2.find_entries
2) Raise LimitsExceeded in ldap2.get_entry and
ldap2.find_entry_by_attr instead of NotFound error
This changed several assumptions about ldap2.find_entries
results. Several calls accross IPA code base had to be
amended.
https://fedorahosted.org/freeipa/ticket/2606
---
ipalib/plugins/automount.py |2 ++
ipaserver/plugins/ldap2.py | 24 ++--
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index 9df400d2ec87088b9f9f41240bec7226cb5f7a22..366729425f733b74c6c2ffb3efb1545c39afac2d 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -724,6 +724,8 @@ class automountkey(LDAPObject):
basedn, _ldap.SCOPE_ONELEVEL)
if len(entries) 1:
raise errors.NotFound(reason=_('More than one entry with key %(key)s found, use --info to select specific entry.') % dict(key=pkey))
+if truncated:
+raise errors.LimitsExceeded()
dn = entries[0][0]
return dn
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 61341b082b834fb3cc2ab5e3452a563be37cebef..b65e011dd32099f33878e485408fffa966955187 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
Re: [Freeipa-devel] [PATCH] 248 Raise proper exception when LDAP limits are exceeded
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
Re: [Freeipa-devel] [PATCH] 248 Raise proper exception when LDAP limits are exceeded
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):
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 248 Raise proper exception when LDAP limits are exceeded
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 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 248 Raise proper exception when LDAP limits are exceeded
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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 248 Raise proper exception when LDAP limits are exceeded
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 From ee78c421a942cdfb7baaa0e21b9c39a4e5cac272 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Tue, 10 Apr 2012 10:50:51 +0200 Subject: [PATCH] Raise proper exception when LDAP limits are exceeded 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 --- ipaserver/plugins/ldap2.py |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index 61341b082b834fb3cc2ab5e3452a563be37cebef..a96abe0c9bfbc1a098a12a65ac500764b9031c23 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -750,6 +750,10 @@ class ldap2(CrudBackend, Encoder): res.append(res_list[0]) except (_ldap.ADMINLIMIT_EXCEEDED, _ldap.TIMELIMIT_EXCEEDED, _ldap.SIZELIMIT_EXCEEDED), e: +if not res: +# server returned no result +# return rather LimitsExceeded error than NotFound error +raise errors.LimitsExceeded() truncated = True except _ldap.LDAPError, e: _handle_errors(e) -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
