Tk Library Source Code

::ldap::add cannot add entries with multiple values for an 
attribute. 
 
My test program: 
 
----------------------------------------------------- 
#!/usr/local/bin/tclsh8.4 
 
package require ldap 
set ldapfd [::ldap::connect ldap.my.domain] 
 
::ldap::bind $ldapfd "cn=admin,o=test" "mypassword" 
set dn "cn=Test,ou=persons,o=test" 
 
set attrval1 { 
    objectClass person 
    cn          Test 
    sn          Test 
    telephoneNumber {+31415926535 +27182818285} 
} 
 
set attrval2 { 
    objectClass person 
    cn          {Test User} 
    sn          Test 
    telephoneNumber +31415926535 
    telephoneNumber +27182818285 
} 
 
::ldap::add $ldapfd $dn $attrval1 
 
::ldap::disconnect $ldapfd 
----------------------------------------------------- 
 
When $attrval2 is given to ::ldap::add, I get an error 
since telephoneNumber is repeated twice. 
 
When $attrval1 is given to ::ldap::add, there is one 
occurrence of the telephoneNumber with the two 
numbers, which is not the required result. 
 
This may (also) be a documentation problem.

Logged In: YES 
user_id=302287

I applied your patch with small modifications.

File Added - 189010: ldap.diff2

Logged In: YES 
user_id=1268042

Here is the final (I hope) patch to close this PR.

- The "add" function allows multiple tuples with the same 
attribute. This API change maintains compatibility with 
the old one, while allowing multi-valuated attributes. 
However, a additionnel loop makes this function slower.
- A new "addMulti" function extends the API with a list 
where each value in a pair is itself a list of all values 
for this attribute.

So, to summarize:

1. I namespace-imported the asn module

2a. There is no API change. Instead, thanks to Michel 
Schlencker, a new module (ldapx.tcl) will provide a 
higher-level interface based on Snit objects. The current 
ldap.tcl will remains as the low-level interface upon 
which ldapx.tcl is built

2b. We get the two syntaxes at the same price ;-)

3. Some progress is being made. But this is no longer 
related to this PR.

Logged In: YES 
user_id=302287

1. Ok, will consider 'namespace import' or the prefixed
versions..., both would work...

2a. In a major new version the api may change quite a bit,
thats the reason for a major revision, so if you have a
patch or good ideas, go on.

2b. Yes, the spec is buggy, so if it is fixed in a 2.0 it
should be done consistent in ldap search and other places.
There are other things which should be done (use the SASL
package and fix the SSL stuff).

Logged In: YES 
user_id=1268042

1- I don't like "namespace import". IMHO, it obfuscates 
the code.

2.a- Ok for removing api_version command and use a 
different version number (2.0) for ldapx.tcl. May I 
seriously change some APIs? Candidates 
are: ::ldap::connect (take an URI and 
remove ::ldap::secure_connect, ::ldap::bind 
and ::ldap::unbind) and ::ldap::search (use a more 
extensible way of specifying options).

2.b- attr-vallist (attrval1) or duplicate keys (attrval2)?
You seem to prefer attrval2, but this is not compatible 
with ::ldap::search result which should be an objective 
IMHO. The current spec is buggy, and we cannot cope with 
it.

3- I'm playing with some ideas at this time. Follow-up by 
private mail.

Logged In: YES 
user_id=302287

Thanks for the patch.

Some comments:
1. prefixing all the calls with asn:: isn't necessary, this
is a perfect usecase for 'namespace import', so just
throwing out the duplicated code and adding a namespace
import works fine

2. I don't really like the 'api_version' command, such
things should usually be done with a major version change of
the package (see md5 for an example where this happend)

Basically i would just go for fixing the error with multiple
attributes, so that your $arrtval2 should work, but the docs
talk about 'dictionary' which implicitly forbids duplicate
keys in the typical tcl hashtable sense.  

The main problem with using the list approach (like
$arrval1) you choose is the fact that there is no way to
find out if a list or a simple string is passed, so it would
always require listification of all values.

Repeating the name and value for multiple values and letting
the code check for duplicates and create the appropriate
response set would probably be better. This would change the
documentation from the strict "dictionary" to the less
strict "name value list" which would allow duplicate keys.
 
This had the bonus, that it would not break but only extend
the current api, and so the package could stay at api
version 1.x.

3. What kind of functions do you have in mind when you talk
about 'higher level functions to manipulate entries'? Would
be very interested in your ideas there.

File Added - 187435: ldap.diff

Logged In: YES 
user_id=1268042

The attached patch provides a correction:
- addition of a new function ::ldap::api_version, which 
defaults to 1 (compatibility with buggy API)
- when set to 2, ::ldap::add expects multi-valuated values 
for each attribute.

The new function api_version is meant, in the future, to 
be set automatically by new functions to manipulate LDAP 
entries at a higher level.

This patch also removes duplicated code between asn.tcl 
and ldap.tcl (asn* functions are removed from ldap.tcl, 
and some functions from asn.tcl are updated).