#589 [RFE] Support RFC 4527 Read Entry Controls
Closed: wontfix None Opened 11 years ago by nkinder.

The read entry controls specified in RFC 4527 could come in very useful for clients who want to read their writes. Now that 389 DS has converted most of it's plug-ins that perform internal write operations to betxn plug-ins, we would be able to return an entry that was modified by a plug-in with the post-read control. This would be very useful when using plug-ins like DNA or memberOf.


1) This is a very minor request... :)
The controls are registered as follows just with SLAPI_OPERATION_MODDN since SLAPI_OPERATION_MODRDN is identical to SLAPI_OPERATION_MODDN (slapi-plugin.h:#define SLAPI_OPERATION_MODRDN SLAPI_OPERATION_MODDN).
{{{
98 slapi_register_supported_control( LDAP_CONTROL_PRE_READ_ENTRY,
99 SLAPI_OPERATION_DELETE | SLAPI_OPERATION_MODIFY |
100 SLAPI_OPERATION_MODDN );
101 slapi_register_supported_control( LDAP_CONTROL_POST_READ_ENTRY,
102 SLAPI_OPERATION_ADD | SLAPI_OPERATION_MODIFY |
103 SLAPI_OPERATION_MODDN );
}}}
These operation checking code is comparing with LDAP_REQ_MODRDN, as well, although it is identical to LDAP_REQ_MODDN, again. (/usr/include/ldap.h:#define LDAP_REQ_MODRDN LDAP_REQ_MODDN). Could it be possible to change either side to match the number of operations? First, I thought this checking has something not needed. But it is just checking the same value (LDAP_REQ_MODRDN & LDAP_REQ_MODDN) twice.
{{{
612 if(strcmp(oid,LDAP_CONTROL_PRE_READ_ENTRY) == 0){
613 / first verify this is the correct operation for a pre-read entry control /
614 if(op->o_tag == LDAP_REQ_MODIFY || op->o_tag == LDAP_REQ_DELETE ||
615 op->o_tag ==LDAP_REQ_MODRDN || op->o_tag == LDAP_REQ_MODDN){
...
624 } else {
625 / first verify this is the correct operation for a post-read entry control /
626 if(op->o_tag == LDAP_REQ_MODIFY || op->o_tag == LDAP_REQ_ADD ||
627 op->o_tag ==LDAP_REQ_MODRDN || op->o_tag == LDAP_REQ_MODDN){
...
}}}

2) Could there be a case the control is empty (Null req_value could be returned?)
{{{
607 if (slapi_control_present(req_ctls, oid, &req_value, &iscritical))
}}}

Replying to [comment:7 nhosoi]:

1) This is a very minor request... :)
The controls are registered as follows just with SLAPI_OPERATION_MODDN since SLAPI_OPERATION_MODRDN is identical to SLAPI_OPERATION_MODDN (slapi-plugin.h:#define SLAPI_OPERATION_MODRDN SLAPI_OPERATION_MODDN).
{{{
98 slapi_register_supported_control( LDAP_CONTROL_PRE_READ_ENTRY,
99 SLAPI_OPERATION_DELETE | SLAPI_OPERATION_MODIFY |
100 SLAPI_OPERATION_MODDN );
101 slapi_register_supported_control( LDAP_CONTROL_POST_READ_ENTRY,
102 SLAPI_OPERATION_ADD | SLAPI_OPERATION_MODIFY |
103 SLAPI_OPERATION_MODDN );
}}}
These operation checking code is comparing with LDAP_REQ_MODRDN, as well, although it is identical to LDAP_REQ_MODDN, again. (/usr/include/ldap.h:#define LDAP_REQ_MODRDN LDAP_REQ_MODDN). Could it be possible to change either side to match the number of operations? First, I thought this checking has something not needed. But it is just checking the same value (LDAP_REQ_MODRDN & LDAP_REQ_MODDN) twice.
{{{
612 if(strcmp(oid,LDAP_CONTROL_PRE_READ_ENTRY) == 0){
613 / first verify this is the correct operation for a pre-read entry control /
614 if(op->o_tag == LDAP_REQ_MODIFY || op->o_tag == LDAP_REQ_DELETE ||
615 op->o_tag ==LDAP_REQ_MODRDN || op->o_tag == LDAP_REQ_MODDN){
...
624 } else {
625 / first verify this is the correct operation for a post-read entry control /
626 if(op->o_tag == LDAP_REQ_MODIFY || op->o_tag == LDAP_REQ_ADD ||
627 op->o_tag ==LDAP_REQ_MODRDN || op->o_tag == LDAP_REQ_MODDN){
...
}}}

No problem.

2) Could there be a case the control is empty (Null req_value could be returned?)
{{{
607 if (slapi_control_present(req_ctls, oid, &req_value, &iscritical))
}}}

I'm not sure if the req_value would be NULL or bv_val would be NULL in that case. This is "sort of" checked at line 656, but I'll improve it.

656 if((req_value->bv_len != 0) && req_value->bv_val){

Thanks,
Mark

One more thing - if the controls is missing or garbled, the correct response is to ignore it if critical == FALSE, or reject with LDAP_UNAVAILABLE_CRITICAL_EXTENSION if critical == TRUE

Replying to [comment:9 rmeggins]:

One more thing - if the controls is missing or garbled, the correct response is to ignore it if critical == FALSE, or reject with LDAP_UNAVAILABLE_CRITICAL_EXTENSION if critical == TRUE

This is already handled:

result.c

733 done:
if(iscritical){
return rc;
} else {
return 0;
}

Fast-forward
ldap/servers/slapd/control.c | 8 +-
ldap/servers/slapd/result.c | 304 ++++++++++++++++++++++++++++++++++++-
ldap/servers/slapd/slapi-plugin.h | 8 +
3 files changed, 316 insertions(+), 4 deletions(-)

git push origin master
Counting objects: 15, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (8/8), done.
Writing objects: 100% (8/8), 3.77 KiB, done.
Total 8 (delta 6), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
6c26f0b..63f6db7 master -> master

commit 63f6db7
Author: Mark Reynolds mreynolds@redhat.com
Date: Thu May 30 16:05:36 2013 -0400

Hello! Could I expect that this patch will be released (at the latest) with #47388?

I changed Ticket origin to IPA because this feature will be handy for bind-dyndb-ldap project.

This feature is planned for 1.3.2 which is targeted for Fedora 20

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.3.2 - 06/13 (June)

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/589

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata