On Tue, Aug 11, 2015 at 1:34 AM, Jan Cholasta <jchol...@redhat.com> wrote:
> On 6.8.2015 21:43, Gabe Alford wrote: > >> Hello, >> >> Updated patch attached. >> >> - Time limit is -1 for unlimited. I found this >> https://www.redhat.com/archives/freeipa-devel/2011-January/msg00330.html >> in reference to keeping the time limit as -1 for unlimited. >> > > This patch does two conflicting things: it coerces time limit of 0 to -1 > and at the same time prohibits the user to use 0 for time limit. We should > do just one of these and IMHO it should be the coercion of 0 to -1. > > Sure enough, testing time limit at 0 did not work for unlimited as well >> as appeared to have negative effects on IPA. >> > > This is because the time limit read from ipa config is not converted to > int in ldap2.find_entries(), so the coercion does not work. Fix this and 0 > will work just fine. > > Also, I believe that >> http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.search_ext_s >> specifies unlimited for time limit as -1. (Please correct me if I am >> wrong.) >> > > python-ldap is layers below our API and should not determine what we use > for unlimited time limit. I would prefer if we were self-consistent and use > 0 for both time limit and size limit. > A misunderstanding on my part as I thought it was higher up in the API for some reason. Updated patch attached. Thanks, Gabe
From b1b173486e3ed4f6fd5bf05d93e52e2783ab1a1e Mon Sep 17 00:00:00 2001 From: Gabe <redhatri...@gmail.com> Date: Wed, 12 Aug 2015 09:12:37 -0600 Subject: [PATCH] Standardize minvalue for ipasearchrecordlimit and ipasesarchsizelimit for unlimited minvalue https://fedorahosted.org/freeipa/ticket/4023 --- install/ui/test/data/ipa_init_commands.json | 6 +++--- install/ui/test/data/ipa_init_objects.json | 6 +++--- install/ui/test/data/json_metadata.json | 4 ++-- ipalib/plugins/baseldap.py | 4 ++-- ipalib/plugins/config.py | 21 +++++++++++++-------- ipaserver/plugins/ldap2.py | 4 ++-- 6 files changed, 25 insertions(+), 20 deletions(-) diff --git a/install/ui/test/data/ipa_init_commands.json b/install/ui/test/data/ipa_init_commands.json index 743f508e2a733b766008bdd21838454ef7df8c21..1e0a8088f2b769d382d9c8532d20ca2b356580de 100644 --- a/install/ui/test/data/ipa_init_commands.json +++ b/install/ui/test/data/ipa_init_commands.json @@ -2446,7 +2446,7 @@ "attribute": true, "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum amount of time (seconds) for a search (> 0, or -1 for unlimited)", + "doc": "Maximum amount of time (seconds) for a search (0 is unlimited)", "flags": [ "nonempty" ], @@ -2460,7 +2460,7 @@ "attribute": true, "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum number of records to search (-1 is unlimited)", + "doc": "Maximum number of records to search (0 is unlimited)", "flags": [ "nonempty" ], @@ -24018,4 +24018,4 @@ "methods": {}, "objects": {} } -} \ No newline at end of file +} diff --git a/install/ui/test/data/ipa_init_objects.json b/install/ui/test/data/ipa_init_objects.json index c8c836926d94dd4c1903aa9a62fa91c11a238e75..9cbebaf099b4de937ae107e1be17c65bdeddedff 100644 --- a/install/ui/test/data/ipa_init_objects.json +++ b/install/ui/test/data/ipa_init_objects.json @@ -498,7 +498,7 @@ { "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum amount of time (seconds) for a search (> 0, or -1 for unlimited)", + "doc": "Maximum amount of time (seconds) for a search (0 is unlimited)", "flags": [], "label": "Search time limit", "maxvalue": 2147483647, @@ -510,7 +510,7 @@ { "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum number of records to search (-1 is unlimited)", + "doc": "Maximum number of records to search (0 is unlimited)", "flags": [], "label": "Search size limit", "maxvalue": 2147483647, @@ -9158,4 +9158,4 @@ } } } -} \ No newline at end of file +} diff --git a/install/ui/test/data/json_metadata.json b/install/ui/test/data/json_metadata.json index a9e66230f94875f7da7efa73141e244b8201a5cc..ff15fe793b48b76d9f8b3dfdb6df31f6f9226529 100644 --- a/install/ui/test/data/json_metadata.json +++ b/install/ui/test/data/json_metadata.json @@ -2596,7 +2596,7 @@ "cli_name": "searchtimelimit", "cli_short_name": null, "default": null, - "doc": "Max. amount of time (sec.) for a search (> 0, or -1 for unlimited)", + "doc": "Max. amount of time (sec.) for a search (0 is unlimited)", "exclude": null, "flags": [], "hint": null, @@ -2619,7 +2619,7 @@ "cli_name": "searchrecordslimit", "cli_short_name": null, "default": null, - "doc": "Max. number of records to search (-1 is unlimited)", + "doc": "Max. number of records to search (0 is unlimited)", "exclude": null, "flags": [], "hint": null, diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 36a5d5f4918828a97b0b30d4613819f777722de8..231f5036df0db00456752b3356a15e55b85e1b4e 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -1917,14 +1917,14 @@ class LDAPSearch(BaseLDAPCommand, crud.Search): takes_options = ( Int('timelimit?', label=_('Time Limit'), - doc=_('Time limit of search in seconds'), + doc=_('Time limit of search in seconds (0 is unlimited)'), flags=['no_display'], minvalue=0, autofill=False, ), Int('sizelimit?', label=_('Size Limit'), - doc=_('Maximum number of entries returned'), + doc=_('Maximum number of entries returned (0 is unlimited)'), flags=['no_display'], minvalue=0, autofill=False, diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py index 6267313d5e9af2d97f45f987115de143d7aa7915..1f6e6f4ebcbd7d7327d250d6b03b129d19c5dc31 100644 --- a/ipalib/plugins/config.py +++ b/ipalib/plugins/config.py @@ -78,11 +78,6 @@ EXAMPLES: register = Registry() -def validate_searchtimelimit(ugettext, limit): - if limit == 0: - raise ValidationError(name='ipasearchtimelimit', error=_('searchtimelimit must be -1 or > 1.')) - return None - @register() class config(LDAPObject): """ @@ -150,16 +145,16 @@ class config(LDAPObject): label=_('Default e-mail domain'), doc=_('Default e-mail domain'), ), - Int('ipasearchtimelimit', validate_searchtimelimit, + Int('ipasearchtimelimit', cli_name='searchtimelimit', label=_('Search time limit'), - doc=_('Maximum amount of time (seconds) for a search (> 0, or -1 for unlimited)'), + doc=_('Maximum amount of time (seconds) for a search (0 is unlimited)'), minvalue=-1, ), Int('ipasearchrecordslimit', cli_name='searchrecordslimit', label=_('Search size limit'), - doc=_('Maximum number of records to search (-1 is unlimited)'), + doc=_('Maximum number of records to search (0 is unlimited)'), minvalue=-1, ), IA5Str('ipausersearchfields', @@ -267,6 +262,16 @@ class config_mod(LDAPUpdate): name=k, error=_('attribute "%s" not allowed') % a ) + # Set ipasearchrecordslimit to 0 if -1 is used + if 'ipasearchrecordslimit' in entry_attrs: + if entry_attrs['ipasearchrecordslimit'] < 0: + entry_attrs['ipasearchrecordslimit'] = 0 + + # Set ipasearchtimelimit to 0 if -1 is used + if 'ipasearchtimelimit' in entry_attrs: + if entry_attrs['ipasearchtimelimit'] < 0: + entry_attrs['ipasearchtimelimit'] = 0 + for (attr, obj) in (('ipauserobjectclasses', 'user'), ('ipagroupobjectclasses', 'group')): if attr in entry_attrs: diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index 68feee4f09eb12e50867dfbe3c482a359838aa82..c12191cf049e93913bf4573504544873b0e6fbc2 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -182,8 +182,8 @@ class ldap2(CrudBackend, LDAPClient): """Get configured global limits, caching them for more calls""" if not _lims: config = self.get_ipa_config() - _lims['time'] = config.get('ipasearchtimelimit', [None])[0] - _lims['size'] = config.get('ipasearchrecordslimit', [None])[0] + _lims['time'] = int(config.get('ipasearchtimelimit', [None])[0]) + _lims['size'] = int(config.get('ipasearchrecordslimit', [None])[0]) return _lims _lims = {} -- 1.8.3.1
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
