ldap/servers/plugins/automember/automember.c | 117 ++++++++++++++++++++-------
ldap/servers/slapd/back-ldbm/ldbm_add.c | 4
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 5 +
ldap/servers/slapd/back-ldbm/ldbm_modify.c | 2
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 3
ldap/servers/slapd/plugin.c | 3
6 files changed, 102 insertions(+), 32 deletions(-)
New commits:
commit 67a076422c44f135048f8eac77c4b1f9fbcce3ee
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Mon Dec 9 16:57:35 2013 -0500
Ticket 47622 - Automember betxnpreoperation - transaction not aborted when group entry
does not exist
Bug Description: If the group defined in the automember plugin does not exist, than
any add operation
that should trigger an update, succeeds even though the automember
update failed.
Fix Description: Return an error if a automember post operation update fails -
previously we always
returned success.
Updated plugin_call_func() to check the result of betxn postop
plugins.
Also added return text to the result message when a betxn plugin
fails. This is
useful for clients to explain why the operation failed.
https://fedorahosted.org/389/ticket/47622
Jenkins: passed
Valgrind: passed
Coverity: passed
Reviewed by: rmeggins(Thanks!)
diff --git a/ldap/servers/plugins/automember/automember.c
b/ldap/servers/plugins/automember/automember.c
index e58eb1e..13ec13e 100644
--- a/ldap/servers/plugins/automember/automember.c
+++ b/ldap/servers/plugins/automember/automember.c
@@ -103,8 +103,8 @@ static struct automemberRegexRule *automember_parse_regex_rule(char
*rule_string
static void automember_free_regex_rule(struct automemberRegexRule *rule);
static int automember_parse_grouping_attr(char *value, char **grouping_attr,
char **grouping_value);
-static void automember_update_membership(struct configEntry *config, Slapi_Entry *e,
PRFileDesc *ldif_fd);
-static void automember_add_member_value(Slapi_Entry *member_e, const char *group_dn,
+static int automember_update_membership(struct configEntry *config, Slapi_Entry *e,
PRFileDesc *ldif_fd);
+static int automember_add_member_value(Slapi_Entry *member_e, const char *group_dn,
char *grouping_attr, char *grouping_value, PRFileDesc *ldif_fd);
const char *fetch_attr(Slapi_Entry *e, const char *attrname, const char *default_val);
@@ -1401,7 +1401,7 @@ automember_parse_grouping_attr(char *value, char **grouping_attr,
char **groupin
* Determines which target groups need to be updated according to
* the rules in config, then performs the updates.
*/
-static void
+static int
automember_update_membership(struct configEntry *config, Slapi_Entry *e, PRFileDesc
*ldif_fd)
{
PRCList *rule = NULL;
@@ -1412,10 +1412,11 @@ automember_update_membership(struct configEntry *config,
Slapi_Entry *e, PRFileD
Slapi_DN *last = NULL;
PRCList *curr_exclusion = NULL;
char **vals = NULL;
+ int rc = 0;
int i = 0;
if (!config || !e) {
- return;
+ return -1;
}
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
@@ -1555,15 +1556,23 @@ automember_update_membership(struct configEntry *config,
Slapi_Entry *e, PRFileD
if (PR_CLIST_IS_EMPTY(&targets)) {
/* Add to each default group. */
for (i = 0; config->default_groups && config->default_groups[i];
i++) {
- automember_add_member_value(e, config->default_groups[i],
- config->grouping_attr,
config->grouping_value, ldif_fd);
+ if(automember_add_member_value(e, config->default_groups[i],
config->grouping_attr,
+ config->grouping_value, ldif_fd))
+ {
+ rc = SLAPI_PLUGIN_FAILURE;
+ goto out;
+ }
}
} else {
/* Update the target groups. */
dnitem = (struct automemberDNListItem *)PR_LIST_HEAD(&targets);
while ((PRCList *)dnitem != &targets) {
- automember_add_member_value(e, slapi_sdn_get_dn(dnitem->dn),
- config->grouping_attr,
config->grouping_value, ldif_fd);
+ if(automember_add_member_value(e,
slapi_sdn_get_dn(dnitem->dn),config->grouping_attr,
+ config->grouping_value, ldif_fd))
+ {
+ rc = SLAPI_PLUGIN_FAILURE;
+ goto out;
+ }
dnitem = (struct automemberDNListItem *)PR_NEXT_LINK((PRCList *)dnitem);
}
}
@@ -1582,6 +1591,9 @@ automember_update_membership(struct configEntry *config, Slapi_Entry
*e, PRFileD
slapi_ch_free((void**)&dnitem);
}
+out:
+
+ return rc;
}
/*
@@ -1589,7 +1601,7 @@ automember_update_membership(struct configEntry *config, Slapi_Entry
*e, PRFileD
*
* Adds a member entry to a group.
*/
-static void
+static int
automember_add_member_value(Slapi_Entry *member_e, const char *group_dn, char
*grouping_attr,
char *grouping_value, PRFileDesc *ldif_fd)
{
@@ -1600,6 +1612,7 @@ automember_add_member_value(Slapi_Entry *member_e, const char
*group_dn, char *g
char *vals[2];
char *member_value = NULL;
int freeit = 0;
+ int rc = 0;
/* If grouping_value is dn, we need to fetch the dn instead. */
if (slapi_attr_type_cmp(grouping_value, "dn", SLAPI_TYPE_CMP_EXACT) == 0)
{
@@ -1649,6 +1662,7 @@ automember_add_member_value(Slapi_Entry *member_e, const char
*group_dn, char *g
"a \"%s\" value to group \"%s\"
(%s).\n",
member_value, grouping_attr, group_dn,
ldap_err2string(result));
+ rc = result;
}
} else {
slapi_log_error(SLAPI_LOG_FATAL, AUTOMEMBER_PLUGIN_SUBSYSTEM,
@@ -1662,8 +1676,9 @@ out:
if (freeit) {
slapi_ch_free_string(&member_value);
}
-
slapi_pblock_destroy(mod_pb);
+
+ return rc;
}
@@ -1833,6 +1848,7 @@ automember_add_post_op(Slapi_PBlock *pb)
Slapi_DN *sdn = NULL;
struct configEntry *config = NULL;
PRCList *list = NULL;
+ int rc = SLAPI_PLUGIN_SUCCESS;
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"--> automember_add_post_op\n");
@@ -1848,8 +1864,9 @@ automember_add_post_op(Slapi_PBlock *pb)
}
} else {
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
- "automember_add_post_op: Error "
- "retrieving dn\n");
+ "automember_add_post_op: Error retrieving dn\n");
+
+ rc = SLAPI_PLUGIN_FAILURE;
goto bail;
}
@@ -1863,12 +1880,11 @@ automember_add_post_op(Slapi_PBlock *pb)
if (e) {
/* If the entry is a tombstone, just bail. */
- Slapi_Value *tombstone =
- slapi_value_new_string(SLAPI_ATTR_VALUE_TOMBSTONE);
- int rc = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS,
- tombstone);
+ Slapi_Value *tombstone = slapi_value_new_string(SLAPI_ATTR_VALUE_TOMBSTONE);
+ int is_tombstone = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS,
+ tombstone);
slapi_value_free(&tombstone);
- if (rc) {
+ if (is_tombstone) {
return SLAPI_PLUGIN_SUCCESS;
}
@@ -1891,7 +1907,10 @@ automember_add_post_op(Slapi_PBlock *pb)
if (slapi_dn_issuffix(slapi_sdn_get_dn(sdn), config->scope)
&&
(slapi_filter_test_simple(e, config->filter) == 0)) {
/* Find out what membership changes are needed and make them. */
- automember_update_membership(config, e, NULL);
+ if(automember_update_membership(config, e, NULL)){
+ rc = SLAPI_PLUGIN_FAILURE;
+ break;
+ }
}
list = PR_NEXT_LINK(list);
@@ -1904,11 +1923,21 @@ automember_add_post_op(Slapi_PBlock *pb)
"automember_add_post_op: Error "
"retrieving post-op entry %s\n",
slapi_sdn_get_dn(sdn));
}
+
bail:
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
- "<-- automember_add_post_op\n");
+ "<-- automember_add_post_op (%d)\n", rc);
- return SLAPI_PLUGIN_SUCCESS;
+ if(rc){
+ char errtxt[SLAPI_DSE_RETURNTEXT_SIZE];
+ int result = LDAP_UNWILLING_TO_PERFORM;
+
+ PR_snprintf(errtxt, SLAPI_DSE_RETURNTEXT_SIZE, "Automember Plugin update
unexpectedly failed.\n");
+ slapi_pblock_set(pb, SLAPI_RESULT_CODE, &result);
+ slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, &errtxt);
+ }
+
+ return rc;
}
/*
@@ -2216,7 +2245,11 @@ void automember_rebuild_task_thread(void *arg){
if (slapi_dn_issuffix(slapi_entry_get_dn(entries[i]), config->scope)
&&
(slapi_filter_test_simple(entries[i], config->filter) == 0))
{
- automember_update_membership(config, entries[i], NULL);
+ if(automember_update_membership(config, entries[i], NULL)){
+ result = SLAPI_PLUGIN_FAILURE;
+ automember_config_unlock();
+ goto out;
+ }
}
list = PR_NEXT_LINK(list);
}
@@ -2417,7 +2450,7 @@ void automember_export_task_thread(void *arg){
/* make sure the plugin is still up, as this loop could run for awhile */
if (!g_plugin_started) {
automember_config_unlock();
- result = -1;
+ result = SLAPI_DSE_CALLBACK_ERROR;
goto out;
}
if (!PR_CLIST_IS_EMPTY(g_automember_config)) {
@@ -2427,7 +2460,11 @@ void automember_export_task_thread(void *arg){
if (slapi_dn_issuffix(slapi_sdn_get_dn(td->base_dn), config->scope)
&&
(slapi_filter_test_simple(entries[i], config->filter) == 0))
{
- automember_update_membership(config, entries[i], ldif_fd);
+ if(automember_update_membership(config, entries[i], ldif_fd)){
+ result = SLAPI_DSE_CALLBACK_ERROR;
+ automember_config_unlock();
+ goto out;
+ }
}
list = PR_NEXT_LINK(list);
}
@@ -2627,7 +2664,13 @@ void automember_map_task_thread(void *arg){
if (slapi_dn_issuffix(slapi_entry_get_dn_const(e), config->scope)
&&
(slapi_filter_test_simple(e, config->filter) == 0))
{
- automember_update_membership(config, e, ldif_fd_out);
+ if(automember_update_membership(config, e, ldif_fd_out)){
+ result = SLAPI_DSE_CALLBACK_ERROR;
+ slapi_entry_free(e);
+ slapi_ch_free_string(&entrystr);
+ automember_config_unlock();
+ goto out;
+ }
}
list = PR_NEXT_LINK(list);
}
@@ -2638,7 +2681,7 @@ void automember_map_task_thread(void *arg){
slapi_task_log_notice(task, "Automember map task, skipping invalid
entry.");
slapi_task_log_status(task, "Automember map task, skipping invalid
entry.");
}
- slapi_ch_free((void **)&entrystr);
+ slapi_ch_free_string(&entrystr);
}
automember_config_unlock();
@@ -2671,6 +2714,7 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
Slapi_DN *new_sdn = NULL;
struct configEntry *config = NULL;
PRCList *list = NULL;
+ int rc = SLAPI_PLUGIN_SUCCESS;
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"--> automember_modrdn_post_op\n");
@@ -2691,7 +2735,7 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"automember_modrdn_post_op: Error "
"retrieving post-op entry\n");
- return SLAPI_PLUGIN_SUCCESS;
+ return SLAPI_PLUGIN_FAILURE;
}
if ((old_sdn = automember_get_sdn(pb))) {
@@ -2702,7 +2746,7 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
slapi_log_error(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,
"automember_modrdn_post_op: Error "
"retrieving dn\n");
- return SLAPI_PLUGIN_SUCCESS;
+ return SLAPI_PLUGIN_FAILURE;
}
/* If replication, just bail. */
@@ -2730,7 +2774,10 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
if (slapi_dn_issuffix(slapi_sdn_get_dn(new_sdn), config->scope)
&&
(slapi_filter_test_simple(post_e, config->filter) == 0)) {
/* Find out what membership changes are needed and make them. */
- automember_update_membership(config, post_e, NULL);
+ if(automember_update_membership(config, post_e, NULL)){
+ rc = SLAPI_PLUGIN_FAILURE;
+ break;
+ }
}
list = PR_NEXT_LINK(list);
@@ -2739,9 +2786,19 @@ automember_modrdn_post_op(Slapi_PBlock *pb)
automember_config_unlock();
+ if(rc){
+ char errtxt[SLAPI_DSE_RETURNTEXT_SIZE];
+ int result = LDAP_UNWILLING_TO_PERFORM;
+
+ PR_snprintf(errtxt, SLAPI_DSE_RETURNTEXT_SIZE, "Automember Plugin update
unexpectedly failed. "
+ "Please see the server errors log for more
information.\n");
+ slapi_pblock_set(pb, SLAPI_RESULT_CODE, &result);
+ slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, &errtxt);
+ }
+
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
- "<-- automember_modrdn_post_op\n");
+ "<-- automember_modrdn_post_op (%d)\n", rc);
- return SLAPI_PLUGIN_SUCCESS;
+ return rc;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c
b/ldap/servers/slapd/back-ldbm/ldbm_add.c
index dbcc914..7834b40 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
@@ -357,7 +357,7 @@ ldbm_back_add( Slapi_PBlock *pb )
/* make sure opreturn is set for the postop plugins */
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ?
&ldap_result_code : &rc);
}
-
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
/*
@@ -795,6 +795,7 @@ ldbm_back_add( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code
: &retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -1046,6 +1047,7 @@ ldbm_back_add( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code :
&retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index 42e26de..ce2a154 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -325,6 +325,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ?
&ldap_result_code : &rc );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
/* the flag could be set in a preop plugin (e.g., USN) */
@@ -354,6 +355,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
ldap_result_code ?
&ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -603,6 +605,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
ldap_result_code ?
&ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
}
@@ -633,6 +636,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
&ldap_result_code : &rc );
}
/* retval is -1 */
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry );
@@ -1105,6 +1109,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index def314e..92692f0 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
@@ -623,6 +623,7 @@ ldbm_back_modify( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code
: &retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -793,6 +794,7 @@ ldbm_back_modify( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ? &ldap_result_code :
&retval);
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index fc66ae6..09f1cea 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -466,6 +466,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ?
&ldap_result_code : &rc );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT,
&ldap_result_message);
goto error_return;
}
/*
@@ -890,6 +891,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ?
&ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
@@ -1130,6 +1132,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
if (!opreturn) {
slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, ldap_result_code ?
&ldap_result_code : &retval );
}
+ slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
goto error_return;
}
diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c
index 66b011b..de9c01b 100644
--- a/ldap/servers/slapd/plugin.c
+++ b/ldap/servers/slapd/plugin.c
@@ -1476,7 +1476,8 @@ plugin_call_func (struct slapdplugin *list, int operation,
Slapi_PBlock *pb, int
}
else if (SLAPI_PLUGIN_BEPREOPERATION == list->plg_type ||
SLAPI_PLUGIN_BETXNPREOPERATION == list->plg_type ||
- SLAPI_PLUGIN_BEPOSTOPERATION == list->plg_type)
+ SLAPI_PLUGIN_BEPOSTOPERATION == list->plg_type ||
+ SLAPI_PLUGIN_BETXNPOSTOPERATION == list->plg_type )
{
/*
* respect fatal error SLAPI_PLUGIN_FAILURE (-1);