Bug 544089 - Referential Integrity Plugin does not take into account the attribute subtypes
Summary: Referential Integrity Plugin does not take into account the attribute subtypes
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Server - Plugins
Version: 1.2.1
Hardware: All
OS: Linux
high
medium
Target Milestone: ---
Assignee: Rich Megginson
QA Contact: Viktor Ashirov
URL:
Whiteboard:
Depends On:
Blocks: 434914 389_1.2.6
TreeView+ depends on / blocked
 
Reported: 2009-12-03 21:44 UTC by Andrey Ivanov
Modified: 2015-12-07 16:52 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-07 16:52:48 UTC


Attachments (Terms of Use)
Proposed patch to referint.c file (6.56 KB, patch)
2009-12-12 16:28 UTC, Andrey Ivanov
no flags Details | Diff
The patched referint.c file (24.40 KB, text/plain)
2009-12-12 16:31 UTC, Andrey Ivanov
no flags Details
Proposed patch to referint.c file (6.59 KB, patch)
2010-01-25 20:06 UTC, Andrey Ivanov
nhosoi: review+
Details | Diff
git patch file containing the fix provided by Andrey. (8.40 KB, patch)
2010-01-25 23:18 UTC, Noriko Hosoi
no flags Details | Diff

Description Andrey Ivanov 2009-12-03 21:44:29 UTC
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)

Referential Integrity Plugin does not change the references in subtyped attributes like "manager;en" or "ou;19"

Reproducible: Always

Steps to Reproduce:
1.Activate the referential integrity plug-in and make it follow the "manager" attribute.
2. Create the following users (the user john.doe contains references to 2 others):

dn: uid=big.chief,ou=Users,dc=example,dc=com
changetype: add
uid: big.chief
sn: Chief
givenName: Big
cn: CHIEF Big (Mr.)
objectClass: inetorgperson
objectClass: organizationalPerson
objectClass: person
objectClass: top

dn: uid=small.chief,ou=Users,dc=example,dc=com
changetype: add
uid: small.chief
sn: Chief
givenName: Small
cn: CHIEF Small (Mr.)
objectClass: inetorgperson
objectClass: organizationalPerson
objectClass: person
objectClass: top

dn: uid=john.doe,ou=Users,dc=example,dc=com
changetype: add
uid: john.doe
sn: Doe
givenName: John
cn: DOE John
objectClass: inetorgperson
objectClass: organizationalPerson
objectClass: person
objectClass: top
manager: uid=small.chief,ou=Users,dc=example,dc=com
manager;10: uid=big.chief,ou=Users,dc=example,dc=com
manager;20: uid=small.chief,ou=Users,dc=example,dc=com


3. Rename the two "bosses" :

dn: uid=small.chief,ou=Users,dc=example,dc=com
changetype: modrdn
newrdn: uid=small.chief.jr
deleteoldrdn: 1
newsuperior: ou=Users,dc=example,dc=com

dn: uid=big.chief,ou=Users,dc=example,dc=com
changetype: modrdn
newrdn: uid=big.chief.jr
deleteoldrdn: 1
newsuperior: ou=Users,dc=example,dc=com
Actual Results:  
The john.doe entry becomes :

uid=john.doe,ou=Users,dc=id,dc=polytechnique,dc=edu
uid: john.doe
sn: Doe
givenName: John
cn: DOE John
objectClass: inetorgperson
objectClass: organizationalPerson
objectClass: person
objectClass: top
manager;10: uid=big.chief,ou=Users,dc=id,dc=polytechnique,dc=edu
manager;20: uid=small.chief,ou=Users,dc=id,dc=polytechnique,dc=edu
manager: uid=small.chief.jr,ou=users,dc=id,dc=polytechnique,dc=edu

Expected Results:  
the plug-in should follow the attribute and all of its subtypes.

We use extensively the attributes subtyping so this bug is quite annoying for us - at each change of, say, the departement name, only the "main" attribute changes, all the subtypes need to be changed manually.

The problem is in the way the function int update_integrity(char **argv, char *origDN, char *newrDN, int logChanges) in referint.c makes the changes. The initial search with the filter ldap_create_filter( filter, filtlen, "(%a=%e)", NULL, NULL, argv[i], origDN, NULL) finds the entries with attributes and with attribute subtypes. But after that when generating the necessary changes (attribute1.mod_type = argv[i] and  attribute2.mod_type = argv[i]) the function takes care only of the "base" attributes listed in the plugin arguments. We should parse each found entry to find all the attribute subtypes with the value concerned and then make changes to them all.

Comment 1 Andrey Ivanov 2009-12-12 16:28:50 UTC
Created attachment 377888 [details]
Proposed patch to referint.c file

Comment 2 Andrey Ivanov 2009-12-12 16:31:13 UTC
Created attachment 377890 [details]
The patched referint.c file

The referint.c file patched with the proposed patch.

Comment 4 Rich Megginson 2010-01-22 15:48:34 UTC
https://bugzilla.redhat.com/attachment.cgi?id=377888&action=diff#referint.c_sec4

at line 392 in the new file - you have this:
		           (slapi_attr_value_find(attr, (struct berval *)slapi_value_get_berval(oldDNslv)) == 0))

is it really necessary to cast the return value of slapi_value_get_berval()?
int slapi_attr_value_find( const Slapi_Attr *a, const struct berval *v );
so it takes a const, and slapi_value_get_berval() returns a const.

Comment 5 Andrey Ivanov 2010-01-22 17:03:32 UTC
No, you're right, it is not necessary. The thing is that the return type of slapi_value_get_berval() is not precisely indicated in RHDS plug-in guide (http://www.redhat.com/docs/manuals/dir-server/8.1/pdf/Plug-in_Guide.pdf). So i've searched an example in the server code. I've found it in ldap/servers/slapd/pw.c (lines 1000-10004) :
if (slapi_attr_value_find(attr, (struct berval *)slapi_value_get_berval(vals[0])) == 0 )

Now that i've looked into slapi-plugin.h the return type is indeed const struct berval *...

Comment 6 Rich Megginson 2010-01-22 17:43:08 UTC
Thanks.  In the future, then plugin guide will be generated directly from slapi-plugin.h with doxygen.  There is some support already, but there is a lot of API to document . . .

Comment 7 Noriko Hosoi 2010-01-25 17:59:08 UTC
Hi Andrey,

I'd like to work on bug 557224 "subtree rename breaks the referential integrity plug-in", which looks requiring to change the code this bug fix affects.

Could you please provide us a new patch including the change suggested by Rich?  We are going to check it.  That helps me a lot to work on the bug...

Thanks in advance,
--noriko

Comment 8 Andrey Ivanov 2010-01-25 20:06:54 UTC
Created attachment 386694 [details]
Proposed patch to referint.c file

Modified patch including the change suggested by Rich

Comment 9 Noriko Hosoi 2010-01-25 20:12:54 UTC
Comment on attachment 386694 [details]
Proposed patch to referint.c file

Thank you, Andrey!

Your new patch looks good to me.

Comment 10 Andrey Ivanov 2010-01-25 20:17:30 UTC
Hi Noriko,

here is the new patch. You are right, it is much simpler to integrate it now. If we try to do that after the modifications necessary to take into account subtree renames it would be a nightmare...

Comment 11 Noriko Hosoi 2010-01-25 23:18:10 UTC
Created attachment 386725 [details]
git patch file containing the fix provided by Andrey.

Thanks to Andrey for providing the patch.

Reviewed by rmeggins@redhat.com and nhosoi@redhat.com.

Pushed to master.

$ git merge work
Updating 0e9fd8b..1a76980
Fast forward
 ldap/servers/plugins/referint/referint.c |  152 ++++++++++++++++-------------
 1 files changed, 84 insertions(+), 68 deletions(-)
$ git push
Counting objects: 13, done.
Delta compression using 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 1.86 KiB, done.
Total 7 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   0e9fd8b..1a76980  master -> master

Comment 12 Jenny Severance 2010-05-13 20:50:20 UTC
verified - RHEL 4 

version:
redhat-ds-base-8.2.0-2010051204.el4dsrv

before modrdn:

# john.doe, People, example.com
dn: uid=john.doe,ou=People,dc=example,dc=com
uid: john.doe
sn: Doe
givenName: John
cn: DOE John
objectClass: inetorgperson
objectClass: organizationalPerson
objectClass: person
objectClass: top
manager: uid=small.chief,ou=People,dc=example,dc=com
manager;10: uid=big.chief,ou=People,dc=example,dc=com
manager;20: uid=small.chief,ou=People,dc=example,dc=com


after modrdn:

# john.doe, People, example.com
dn: uid=john.doe,ou=People,dc=example,dc=com
uid: john.doe
sn: Doe
givenName: John
cn: DOE John
objectClass: inetorgperson
objectClass: organizationalPerson
objectClass: person
objectClass: top
manager: uid=small.chief.jr,ou=people,dc=example,dc=com
manager;20: uid=small.chief.jr,ou=people,dc=example,dc=com
manager;10: uid=big.chief.jr,ou=people,dc=example,dc=com


Note You need to log in before you can comment on or make changes to this bug.