[pmg-devel] [PATCH pmg-api] fix #2661: reintroduce LDAPCache->ldap_connect

Thomas Lamprecht t.lamprecht at proxmox.com
Fri Apr 3 14:12:56 CEST 2020


On 4/3/20 2:05 PM, Dominik Csapak wrote:
>>> +    if ($scheme eq 'ldaps') {
>>> +    $opts->{verify} = 'require' if $self->{verify};
>>> +    if ($self->{cafile}) {
>>> +        $opts->{cafile} = $self->{cafile};
>>> +    } else {
>>> +        $opts->{capath} = '/etc/ssl/certs/';
>>> +    }
>>> +    } elsif ($self->{mode} eq 'ldap+starttls') {
>>> +    $opts->{verify} = $self->{verify} ? 'require' : 'none';
>>> +
>>> +    if ($self->{cafile}) {
>>> +        $opts->{cafile} = $self->{cafile};
>>> +    } else {
>>> +        $opts->{capath} = '/etc/ssl/certs/';
>>> +    }
>>> +    }
>> not introduced by your patch - but why not make the condition
>> ($scheme eq 'ldaps' || $scheme eq 'ldap+starttls')
>> set the common parameters and only set the verify individually?
>> (more a question of curiosity than a suggestion for changing it)
> 
> good question, also why did i use $scheme one time and $self-{mode} the other?
> i guess i did rewrite the code a few times and at the end
> the branches were this similar, but i did not notice ^^
> 
> @Thomas, should i send a v2 or do you prefer it to have a follow up?
> (so that the 'fix' commit is separate)

Followup is fine to me, just hoping that I don't forget it ^^




More information about the pmg-devel mailing list