Re: (ITS#4868) Binary Attribute Patch(es)

[ando@sys-net.it]

Few preliminary comments follow.

vargok@yahoo.com wrote:

> Version: 2.3.32

You should rather patch HEAD code instead of released, and even not the
latest.  However, given the development rate of back-sql (~0), it
shouldn't really matter

> These address the use of Binary-valued attributes (#3113, #3386):
> 
> For example, inetOrgPerson.userCertificate is usually transferred with the
> ";binary" directive.  ";binary" is not handled by OpenLDAP/Back-SQL.

Well, the solution you propose is not correct, since you are altering
schema data, which is supposed to be read-only.  In any case, the point is:

- if we decide that back-sql should ignore tags, then the solution
consists in using the canonical name of the underlying AttributeType
when looking up data;

- however, this would destroy the possibility to use different storages
for differently encoded data; for example, a column for "cn;lang-en" and
a column for "cn;lang-jp".  I don't know how many users would prefer one
solution over another, but in any case either we find a solution that
preserves both capabilities or we choose one.  I'd vote in favor of
ignoring ";binary" since it's obsolete and related to transport only,
but in favor of honoring language tags.

> As well,
> the data itself, when stored in the database is not properly read out -- all
> data is read as SQL_C_CHAR data.  This supports SQL_C_BINARY-based data.

This is just fine.

> There remains an issue with selecting attributes using, e.g.,
> "userCertificate;binary" -- nothing is returned.  Someone with a better
> understanding of the attribute-processing method would be much more effective in
> terms of finding the correct place to remove the ";binary" from the
> "attribute-name."  (i.e. "userCertificate;binary" is NOT the attribute-name;
> "userCertificate" is the attribute-name, ";binary" is a transport directive (see
> #3113).

In fact, "userCertificate;binary" is the attribute description.  The
attribute name is in ad_type->sat_cname.

> Additionally, I included to the patch to remove the "assert(0)" in Back-SQL's
> verification that a search filter and ldap-data don't mis-match on suffix.

Fine, but already fixed.

> Tags: OpenLDAP, Back-SQL, MySQL, userCertificate, Binary Attributes, Client
> Certificates
> 
> 
> (1)
> ./servers/slapd/back-sql/search.c.patch
>     Addressed ITS#4856 -- I think you already fixed this
> 
> (2)
> ./servers/slapd/back-sql/back-sql.h.patch
> ./servers/slapd/back-sql/entry-id.c.patch
> ./servers/slapd/back-sql/sql-wrap.c.patch
>     Address ITS#3113, ITS#3386
>     Support for binary attribute values
> 
> (3)
> ./servers/slapd/back-sql/schema-map.c.patch
>     Addresses issue with accessing attributes that have been provided with
> ";binary" directive
> 
> note that I have no doubts whatso ever that there are many better ways to do
> (3).

You made an extensive use of C++ style comments, which are non portable.
 Obviously you didn't read the contribution guidelines.

I'm testing (and improving) it.

p.



Ing. Pierangelo Masarati
OpenLDAP Core Team

SysNet s.n.c.
Via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
------------------------------------------
Office:   +39.02.23998309
Mobile:   +39.333.4963172
Email:    pierangelo.masarati@sys-net.it
------------------------------------------