Re: (ITS#3877) Enhancement: openldapACIValidate implementation

[ando@sys-net.it]

> Full_Name: Nikita Shulga
> Version: HEAD
> OS: Linux
> URL: ftp://ftp.openldap.org/incoming/nikita-shulga-050722.patch
> Submission from: (NULL) (80.73.161.132)
>
>
> In attached patch openldapACIValidate function is implemented.
> It checks that openldapACI entry are syntaxicaly valid,
> although it doesn't check sanity of this aci - for example,
> aci_list_get_rights allows more then one grant|deny clause in aci, so
> validator
> allows it to, despite the fact that aclmodel draft prohibits more then one
> grant|deny entry, etc...
> Also, subject are yet not validated, since it validation depends of
> subject
> type
>
> I'm not sure that this is needed, but just in case:
>
> I, Nikita Shulga, hereby place the following modifications to OpenLDAP
> Software
> (and only these modifications) into the public domain. Hence, these
> modifications may be freely used and/or redistributed for any purpose with
> or
> without attribution and/or other notice.
>

Nikita,

the patch looks fine. I'd suggest:
1) for the sake of code confinement, to move it to acl.c; in that case we
might take advantage of this chance to rationalize the pletora of constant
ACL string definitions in acl_bv_*...
2) why in bertok() don't you just use strchr()?
3) beware of escaping the delimiter char; there was recently an issue with
ACIs that didn't parse correctly a DN containing a "#"; it is now fixed
(in 2.3 for sure; not sure about 2.2), but I didn't check if your patch
takes care of it.
4) I'd also see room for an ACI normalization function that takes care of
normalizing the DN in ACIs, so that we don't need to re-normalize them all
times the ACIs are invoked (see all the occurences of dnNormalize() in
aci_mask()).

I'm not sure about the need for a copyright and rights release statement;
in case, I'd suggest you simply state that you put your code under the
OpenLDAP License.

p.

-- 
Pierangelo Masarati
mailto:pierangelo.masarati@sys-net.it


    SysNet - via Dossi,8 27100 Pavia Tel: +390382573859 Fax: +390382476497