[SSSD] [PATCH] Check for controls before using them

Stephen Gallagher sgallagh at redhat.com
Thu Sep 2 13:44:02 UTC 2010


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/09/2010 05:59 AM, Sumit Bose wrote:
> On Fri, Mar 19, 2010 at 02:15:06PM +0100, Sumit Bose wrote:
>> On Fri, Mar 19, 2010 at 08:48:49AM -0400, Simo Sorce wrote:
>>> On Fri, 19 Mar 2010 12:00:47 +0100
>>> Sumit Bose <sbose at redhat.com> wrote:
>>>
>>>> On Thu, Mar 18, 2010 at 05:51:13PM -0400, Simo Sorce wrote:
>>>>>
>>>>> Some time ago I added code to fetch the rootdse on connection, but
>>>>> didn't publicize it too much.
>>>>>
>>>>> Attached find 2 patches.
>>>>>
>>>>> 1) Rework the way we store data fetched from the rootdse so the it
>>>>> is more useful and is actually attached to the ldap handle.  
>>>>
>>>> I'm in general fine with this one, although I haven't tested it
>>>> throughly yet.
>>>>>
>>>>> 2) Check controls are supported before using them.  
>>>>
>>>> I have two comments here:
>>>>
>>>> - sdap_is_control_supported() should be integrated in
>>>>   sss_ldap_control_create() or a new call should be created which
>>>> calls sdap_is_control_supported() and then sss_ldap_control_create().
>>>> - the debug message 'Server does not support the Password Policy
>>>>   control' should have a much lower level than 7. Because if the
>>>> control is missing the user might not see the expected
>>>> behaviour/performance.
>>>
>>> Sumit,
>>> do you want to take over the second patch ?
>>> I don't have a way to thoroughly test it anyway and I am a bit short on
>>> time.
>>
>> sure
>>
> 
> Hi,
> 
> not even 4 months later here are the revised versions of these patches.
> I had it add two more changes to the first patch to make it build on
> master because of the change to sdap_cli_connect_recv().
> 
> In the second patch if have created a new call sdap_control_create()
> which checks if the control is supported with
> sdap_is_control_supported() and calls sss_ldap_control_create() on
> success. I think this approach is more clean, because it leaves
> sss_ldap_control_create() as a pure replacement function.
> 

Naturally, we only got around to reviewing this two months after this
revision :(

Unfortunately, the changes made to the creation of LDAP operations in
SSSD 1.3.0 conflict madly with these changes. Would it be possible for
you to rebase this patch atop the current master?

If not, I'll add it to my todo list post-1.4.0.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkx/qiIACgkQeiVVYja6o6MFHQCeIhpJQ4Ttk0+76fKo6uAsVHhB
cy8AnRsnD3Fy736/lAJVEc7BioJGpYI8
=KNjZ
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list