dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py | 79 +++++++++-
dirsrvtests/tests/suites/dynamic-plugins/stress_tests.py | 22 +-
dirsrvtests/tests/suites/dynamic-plugins/test_dynamic_plugins.py | 3
ldap/servers/plugins/memberof/memberof.c | 59 ++++---
ldap/servers/slapd/plugin_internal_op.c | 27 +--
5 files changed, 143 insertions(+), 47 deletions(-)
New commits:
commit f708c7f5e01c6fb62a2125aa039b2d13f5f66697
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Tue Dec 20 14:59:02 2016 -0500
Ticket 49072 - validate memberof fixup task args
Bug Description: If an invalid base dn, or invalid filter was provided
in the task there was no way to tell thathte task
actually failed.
Fix Description: Log an error, and properly update the task status/exit
code when an error occurs.
Added CI test (also fixed some issues in the dynamic
plugins test suite).
https://fedorahosted.org/389/ticket/49072
Reviewed by: nhosoi(Thanks!)
(cherry picked from commit a79ae70df6b20cd288fca511f784c414e8c52df4)
(cherry picked from commit b0020b73d34bdd630fb5b1a3e4fcebbb4b81f9c9)
diff --git a/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py
b/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py
index 30dfa88..0a74eef 100644
--- a/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py
+++ b/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py
@@ -96,6 +96,7 @@ def test_dependency(inst, plugin):
################################################################################
def wait_for_task(conn, task_dn):
finished = False
+ exitcode = 0
count = 0
while count < 60:
try:
@@ -105,6 +106,7 @@ def wait_for_task(conn, task_dn):
assert False
if task_entry[0].hasAttr('nstaskexitcode'):
# task is done
+ exitcode = task_entry[0].nsTaskExitCode
finished = True
break
except ldap.LDAPError as e:
@@ -117,6 +119,8 @@ def wait_for_task(conn, task_dn):
log.fatal('wait_for_task: Task (%s) did not complete!' % task_dn)
assert False
+ return exitcode
+
################################################################################
#
@@ -1416,9 +1420,82 @@ def test_memberof(inst, args=None):
log.fatal('test_memberof: Search for user1 failed: ' +
e.message['desc'])
assert False
- # Enable the plugin, and run the task
+ # Enable memberof plugin
inst.plugins.enable(name=PLUGIN_MEMBER_OF)
+ #############################################################
+ # Test memberOf fixup arg validation: Test the DN and filter
+ #############################################################
+
+ #
+ # Test bad/nonexistant DN
+ #
+ TASK_DN = 'cn=task-' + str(int(time.time())) + ',' + DN_MBO_TASK
+ try:
+ inst.add_s(Entry((TASK_DN, {
+ 'objectclass': 'top extensibleObject'.split(),
+ 'basedn': DEFAULT_SUFFIX + "bad",
+ 'filter': 'objectclass=top'})))
+ except ldap.LDAPError as e:
+ log.fatal('test_memberof: Failed to add task(bad dn): error ' +
+ e.message['desc'])
+ assert False
+
+ exitcode = wait_for_task(inst, TASK_DN)
+ if exitcode == "0":
+ # We should an error
+ log.fatal('test_memberof: Task with invalid DN still reported success')
+ assert False
+
+ #
+ # Test invalid DN syntax
+ #
+ TASK_DN = 'cn=task-' + str(int(time.time())) + ',' + DN_MBO_TASK
+ try:
+ inst.add_s(Entry((TASK_DN, {
+ 'objectclass': 'top extensibleObject'.split(),
+ 'basedn': "bad",
+ 'filter': 'objectclass=top'})))
+ except ldap.LDAPError as e:
+ log.fatal('test_memberof: Failed to add task(invalid dn syntax): ' +
+ e.message['desc'])
+ assert False
+
+ exitcode = wait_for_task(inst, TASK_DN)
+ if exitcode == "0":
+ # We should an error
+ log.fatal('test_memberof: Task with invalid DN syntax still reported' +
+ ' success')
+ assert False
+
+ #
+ # Test bad filter (missing closing parenthesis)
+ #
+ TASK_DN = 'cn=task-' + str(int(time.time())) + ',' + DN_MBO_TASK
+ try:
+ inst.add_s(Entry((TASK_DN, {
+ 'objectclass': 'top extensibleObject'.split(),
+ 'basedn': DEFAULT_SUFFIX,
+ 'filter': '(objectclass=top'})))
+ except ldap.LDAPError as e:
+ log.fatal('test_memberof: Failed to add task(bad filter: error ' +
+ e.message['desc'])
+ assert False
+
+ exitcode = wait_for_task(inst, TASK_DN)
+ if exitcode == "0":
+ # We should an error
+ log.fatal('test_memberof: Task with invalid filter still reported ' +
+ 'success')
+ assert False
+
+ ####################################################
+ # Test fixup works
+ ####################################################
+
+ #
+ # Run the task and validate that it worked
+ #
TASK_DN = 'cn=task-' + str(int(time.time())) + ',' + DN_MBO_TASK
try:
inst.add_s(Entry((TASK_DN, {
diff --git a/dirsrvtests/tests/suites/dynamic-plugins/stress_tests.py
b/dirsrvtests/tests/suites/dynamic-plugins/stress_tests.py
index 920d3f6..79a8086 100644
--- a/dirsrvtests/tests/suites/dynamic-plugins/stress_tests.py
+++ b/dirsrvtests/tests/suites/dynamic-plugins/stress_tests.py
@@ -88,8 +88,9 @@ class DelUsers(threading.Thread):
try:
conn.delete_s(USER_DN)
except ldap.LDAPError as e:
- log.fatal('DeleteUsers: failed to delete (' + USER_DN + ')
error: ' + e.message['desc'])
- assert False
+ if e == ldap.UNAVAILABLE or e == ldap.SERVER_DOWN:
+ log.fatal('DeleteUsers: failed to delete (' + USER_DN +
') error: ' + e.message['desc'])
+ assert False
idx += 1
@@ -115,11 +116,10 @@ class AddUsers(threading.Thread):
conn.add_s(Entry((GROUP_DN,
{'objectclass': 'top groupOfNames groupOfUniqueNames
extensibleObject'.split(),
'uid': 'user' + str(idx)})))
- except ldap.ALREADY_EXISTS:
- pass
except ldap.LDAPError as e:
- log.fatal('AddUsers: failed to add group (' + USER_DN + ')
error: ' + e.message['desc'])
- assert False
+ if e == ldap.UNAVAILABLE or e == ldap.SERVER_DOWN:
+ log.fatal('AddUsers: failed to add group (' + USER_DN +
') error: ' + e.message['desc'])
+ assert False
log.info('AddUsers - Adding ' + str(NUM_USERS) + ' entries (' +
self.rdnval + ')...')
@@ -129,16 +129,18 @@ class AddUsers(threading.Thread):
conn.add_s(Entry((USER_DN, {'objectclass': 'top
extensibleObject'.split(),
'uid': 'user' + str(idx)})))
except ldap.LDAPError as e:
- log.fatal('AddUsers: failed to add (' + USER_DN + ') error:
' + e.message['desc'])
- assert False
+ if e == ldap.UNAVAILABLE or e == ldap.SERVER_DOWN:
+ log.fatal('AddUsers: failed to add (' + USER_DN + ')
error: ' + e.message['desc'])
+ assert False
if self.addToGroup:
# Add the user to the group
try:
conn.modify_s(GROUP_DN, [(ldap.MOD_ADD, 'uniquemember',
USER_DN)])
except ldap.LDAPError as e:
- log.fatal('AddUsers: Failed to add user' + USER_DN + ' to
group: error ' + e.message['desc'])
- assert False
+ if e == ldap.UNAVAILABLE or e == ldap.SERVER_DOWN:
+ log.fatal('AddUsers: Failed to add user' + USER_DN +
' to group: error ' + e.message['desc'])
+ assert False
idx += 1
diff --git a/dirsrvtests/tests/suites/dynamic-plugins/test_dynamic_plugins.py
b/dirsrvtests/tests/suites/dynamic-plugins/test_dynamic_plugins.py
index c05c402..f67e5b2 100644
--- a/dirsrvtests/tests/suites/dynamic-plugins/test_dynamic_plugins.py
+++ b/dirsrvtests/tests/suites/dynamic-plugins/test_dynamic_plugins.py
@@ -293,7 +293,8 @@ def test_dynamic_plugins(topology):
except:
log.info('Stress test failed!')
- repl_fail(replica_inst)
+ if replication_run:
+ repl_fail(replica_inst)
stress_count += 1
log.info('####################################################################')
diff --git a/ldap/servers/plugins/memberof/memberof.c
b/ldap/servers/plugins/memberof/memberof.c
index aad300f..6ff9c9a 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -65,6 +65,13 @@ typedef struct _memberof_get_groups_data
Slapi_ValueSet **group_norm_vals;
} memberof_get_groups_data;
+typedef struct _task_data
+{
+ char *dn;
+ char *bind_dn;
+ char *filter_str;
+} task_data;
+
/*** function prototypes ***/
/* exported functions */
@@ -142,7 +149,7 @@ static void memberof_task_destructor(Slapi_Task *task);
static const char *fetch_attr(Slapi_Entry *e, const char *attrname,
const char *default_val);
static void memberof_fixup_task_thread(void *arg);
-static int memberof_fix_memberof(MemberOfConfig *config, char *dn, char *filter_str);
+static int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data
*td);
static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data);
static int memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn);
static int memberof_add_objectclass(char *auto_add_oc, const char *dn);
@@ -2625,13 +2632,6 @@ void memberof_unlock()
}
}
-typedef struct _task_data
-{
- char *dn;
- char *bind_dn;
- char *filter_str;
-} task_data;
-
void memberof_fixup_task_thread(void *arg)
{
MemberOfConfig configCopy = {0, 0, 0, 0};
@@ -2646,6 +2646,7 @@ void memberof_fixup_task_thread(void *arg)
slapi_task_inc_refcount(task);
slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
"memberof_fixup_task_thread --> refcount incremented.\n"
);
+
/* Fetch our task data from the task */
td = (task_data *)slapi_task_get_data(task);
@@ -2653,10 +2654,10 @@ void memberof_fixup_task_thread(void *arg)
slapi_td_set_dn(slapi_ch_strdup(td->bind_dn));
slapi_task_begin(task, 1);
- slapi_task_log_notice(task, "Memberof task starts (arg: %s) ...\n",
+ slapi_task_log_notice(task, "Memberof task starts (filter: %s) ...\n",
td->filter_str);
slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,
- "Memberof task starts (arg: %s) ...\n", td->filter_str);
+ "Memberof task starts (filter: \"%s\") ...\n",
td->filter_str);
/* We need to get the config lock first. Trying to get the
* config lock after we already hold the op lock can cause
@@ -2671,7 +2672,7 @@ void memberof_fixup_task_thread(void *arg)
if (usetxn) {
Slapi_DN *sdn = slapi_sdn_new_dn_byref(td->dn);
- Slapi_Backend *be = slapi_be_select(sdn);
+ Slapi_Backend *be = slapi_be_select_exact(sdn);
slapi_sdn_free(&sdn);
if (be) {
fixup_pb = slapi_pblock_new();
@@ -2679,12 +2680,18 @@ void memberof_fixup_task_thread(void *arg)
rc = slapi_back_transaction_begin(fixup_pb);
if (rc) {
slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,
- "memberof_fixup_task_thread: failed to start transaction\n");
+ "memberof_fixup_task_thread: failed to start transaction\n");
+ goto done;
}
} else {
slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,
- "memberof_fixup_task_thread: failed to get be backend from %s\n",
- td->dn);
+ "memberof_fixup_task_thread: failed to get be backend from (%s)\n",
+ td->dn);
+ slapi_task_log_notice(task, "Memberof task - Failed to get be backend from
(%s)\n",
+ td->dn);
+ rc = -1;
+ goto done;
+
}
}
@@ -2692,13 +2699,14 @@ void memberof_fixup_task_thread(void *arg)
memberof_lock();
/* do real work */
- rc = memberof_fix_memberof(&configCopy, td->dn, td->filter_str);
+ rc = memberof_fix_memberof(&configCopy, task, td);
/* release the memberOf operation lock */
memberof_unlock();
+done:
if (usetxn && fixup_pb) {
- if (rc) { /* failes */
+ if (rc) { /* failed */
slapi_back_transaction_abort(fixup_pb);
} else {
slapi_back_transaction_commit(fixup_pb);
@@ -2711,7 +2719,8 @@ void memberof_fixup_task_thread(void *arg)
slapi_task_log_status(task, "Memberof task finished.");
slapi_task_inc_progress(task);
slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,
- "Memberof task finished (arg: %s) ...\n", td->filter_str);
+ "Memberof task finished (filter: %s) result: %d\n",
+ td->filter_str, rc);
/* this will queue the destruction of the task */
slapi_task_finish(task, rc);
@@ -2829,13 +2838,13 @@ memberof_task_destructor(Slapi_Task *task)
"memberof_task_destructor <--\n" );
}
-int memberof_fix_memberof(MemberOfConfig *config, char *dn, char *filter_str)
+int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *td)
{
int rc = 0;
Slapi_PBlock *search_pb = slapi_pblock_new();
- slapi_search_internal_set_pb(search_pb, dn,
- LDAP_SCOPE_SUBTREE, filter_str, 0, 0,
+ slapi_search_internal_set_pb(search_pb, td->dn,
+ LDAP_SCOPE_SUBTREE, td->filter_str, 0, 0,
0, 0,
memberof_get_plugin_id(),
0);
@@ -2844,6 +2853,16 @@ int memberof_fix_memberof(MemberOfConfig *config, char *dn, char
*filter_str)
config,
0, memberof_fix_memberof_callback,
0);
+ if (rc){
+ char *errmsg;
+ int result;
+
+ slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &result);
+ errmsg = ldap_err2string(result);
+ slapi_log_err(SLAPI_LOG_ERR, MEMBEROF_PLUGIN_SUBSYSTEM,
+ "memberof_fix_memberof - Failed (%s)\n", errmsg );
+ slapi_task_log_notice(task, "Memberof task failed (%s)\n", errmsg );
+ }
slapi_pblock_destroy(search_pb);
diff --git a/ldap/servers/slapd/plugin_internal_op.c
b/ldap/servers/slapd/plugin_internal_op.c
index 0f03c76..e203cf5 100644
--- a/ldap/servers/slapd/plugin_internal_op.c
+++ b/ldap/servers/slapd/plugin_internal_op.c
@@ -728,7 +728,7 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data,
if (ifstr == NULL || (scope != LDAP_SCOPE_BASE && scope != LDAP_SCOPE_ONELEVEL
&& scope != LDAP_SCOPE_SUBTREE))
{
- opresult = LDAP_PARAM_ERROR;
+ opresult = LDAP_PARAM_ERROR;
slapi_pblock_set(pb, SLAPI_PLUGIN_INTOP_RESULT, &opresult);
return -1;
}
@@ -745,19 +745,19 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data,
op->o_search_referral_handler = internal_ref_entry_callback;
filter = slapi_str2filter((fstr = slapi_ch_strdup(ifstr)));
- if(scope == LDAP_SCOPE_BASE) {
- filter->f_flags |= (SLAPI_FILTER_LDAPSUBENTRY |
- SLAPI_FILTER_TOMBSTONE | SLAPI_FILTER_RUV);
+ if (NULL == filter) {
+ int result = LDAP_FILTER_ERROR;
+ send_ldap_result(pb, result, NULL, NULL, 0, NULL);
+ slapi_pblock_set(pb, SLAPI_PLUGIN_INTOP_RESULT, &result);
+ rc = -1;
+ goto done;
}
- if (NULL == filter)
- {
- send_ldap_result(pb, LDAP_FILTER_ERROR, NULL, NULL, 0, NULL);
- rc = -1;
- goto done;
+ if (scope == LDAP_SCOPE_BASE) {
+ filter->f_flags |= (SLAPI_FILTER_LDAPSUBENTRY |
+ SLAPI_FILTER_TOMBSTONE | SLAPI_FILTER_RUV);
}
filter_normalize(filter);
-
slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter);
slapi_pblock_set(pb, SLAPI_REQCONTROLS, controls);
@@ -785,11 +785,8 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data,
slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter);
done:
- slapi_ch_free((void **) & fstr);
- if (filter != NULL)
- {
- slapi_filter_free(filter, 1 /* recurse */);
- }
+ slapi_ch_free_string(&fstr);
+ slapi_filter_free(filter, 1 /* recurse */);
slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &tmp_attrs);
slapi_ch_array_free(tmp_attrs);
slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, NULL);