[PATCH] ldb_debug: Fix for bug 13185
Gary Lockyer
gary at catalyst.net.nz
Fri Feb 23 02:38:05 UTC 2018
Unit tests attached.
Gary
On 22/02/18 20:22, Andreas Schneider via samba-technical wrote:
> On Thursday, 22 February 2018 05:16:25 CET Douglas Bagnall via samba-technical
> wrote:
>> On 22/02/18 16:53, Douglas Bagnall via samba-technical wrote:
>>> On 22/02/18 16:06, Gary Lockyer via samba-technical wrote:
>>>> + /*
>>>> + * Check for duplicates in unique indexes
>>>>
>>>> */
>>>>
>>>> if (list->count > 0 &&
>>>>
>>>> ((a != NULL
>>>>
>>>> && (a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX ||
>>>>
>>>> - (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) ||
>>>> - ldb_attr_cmp(el->name, LTDB_IDXDN) == 0)) {
>>>> + (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))))) {
>>>
>>> This looks a bit wrong. I don't think it actually is wrong, but it
>>> just looks scary having "&" mixing with other operators without tight
>>> parentheses. I had to read random web pages to be sure the precedence
>>> of (a & b || c) works as intended.
>>>
>>> I think something like this is logically the same but a bit clearer:
>>> if (list->count > 0 &&
>>>
>>> a != NULL &&
>>>
>>> ((a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX) ||
>>>
>>> (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) {
>>
>> Well, I spoke with Andrew, and it turns out he meant this:
>>
>> if (list->count > 0 &&
>> ((a != NULL && (a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX)) ||
>> (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) {
>
> Add a unit test for this patch?
>
-------------- next part --------------
From 650a976fe6ca0bf2c067e8e43e871883aebeea11 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 23 Feb 2018 15:03:20 +1300
Subject: [PATCH 1/2] ldb tests: fix null test on incorrect variable
Fix up tests that were performing a null check on the wrong variable
after a call to ldb_msg_new
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
lib/ldb/tests/ldb_mod_op_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index cf2288c..0190936 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -3226,7 +3226,7 @@ static void test_ldb_add_duplicate_value_to_unique_index(void **state)
assert_int_equal(ret, LDB_SUCCESS);
msg02 = ldb_msg_new(tmp_ctx);
- assert_non_null(msg01);
+ assert_non_null(msg02);
msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02");
assert_non_null(msg02->dn);
@@ -3267,7 +3267,7 @@ static void test_ldb_add_to_index_duplicates_allowed(void **state)
assert_int_equal(ret, LDB_SUCCESS);
msg02 = ldb_msg_new(tmp_ctx);
- assert_non_null(msg01);
+ assert_non_null(msg02);
msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02");
assert_non_null(msg02->dn);
--
2.7.4
From c127729dcda1ca21d02cc23a9c758645705ac6f2 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 23 Feb 2018 15:04:36 +1300
Subject: [PATCH 2/2] ldb_debug tests: Fix binary data in debug log
Tests to ensure:
When duplicate objects are added, the GUID was printed in the debug log
are passed through the escape function.
And that duplicate DN's do not generate debug log entries.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13185
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
lib/ldb/tests/ldb_mod_op_test.c | 298 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 298 insertions(+)
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 0190936..b7c389a 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -3308,6 +3308,227 @@ static void test_ldb_add_to_index_unique_values_required(void **state)
assert_int_equal(ret, LDB_SUCCESS);
msg02 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg02);
+
+ msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02");
+ assert_non_null(msg02->dn);
+
+ ret = ldb_msg_add_string(msg02, "cn", "test_unique_index");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg02);
+ assert_int_equal(ret, LDB_ERR_CONSTRAINT_VIOLATION);
+ talloc_free(tmp_ctx);
+}
+
+static void ldb_debug_string(void *context, enum ldb_debug_level level,
+ const char *fmt, va_list ap)
+{
+
+ if (level <= LDB_DEBUG_WARNING) {
+ vasprintf((char**)context, fmt, ap);
+ }
+}
+
+static void test_ldb_unique_index_duplicate_logging(void **state)
+{
+ int ret;
+ struct ldb_message *msg01;
+ struct ldb_message *msg02;
+ struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct ldbtest_ctx);
+ TALLOC_CTX *tmp_ctx;
+ char *debug_string = NULL;
+ char *p = NULL;
+
+ ldb_set_debug(test_ctx->ldb, ldb_debug_string, &debug_string);
+ tmp_ctx = talloc_new(test_ctx);
+ assert_non_null(tmp_ctx);
+
+ msg01 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg01);
+
+ msg01->dn = ldb_dn_new_fmt(msg01, test_ctx->ldb, "dc=test01");
+ assert_non_null(msg01->dn);
+
+ ret = ldb_msg_add_string(msg01, "cn", "test_unique_index");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg01);
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ msg02 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg02);
+
+ msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02");
+ assert_non_null(msg02->dn);
+
+ ret = ldb_msg_add_string(msg02, "cn", "test_unique_index");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg02);
+ assert_int_equal(ret, LDB_ERR_CONSTRAINT_VIOLATION);
+
+ assert_non_null(debug_string);
+ p = strstr(
+ debug_string,
+ "unique index violation on cn "
+ "in dc=test02, conficts with dc=test01 in "
+ "@INDEX:CN:test_unique_index");
+ assert_non_null(p);
+ free(debug_string);
+ talloc_free(tmp_ctx);
+}
+
+static void test_ldb_duplicate_dn_logging(void **state)
+{
+ int ret;
+ struct ldb_message *msg01;
+ struct ldb_message *msg02;
+ struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct ldbtest_ctx);
+ TALLOC_CTX *tmp_ctx;
+ char *debug_string = NULL;
+
+ ldb_set_debug(test_ctx->ldb, ldb_debug_string, &debug_string);
+ tmp_ctx = talloc_new(test_ctx);
+ assert_non_null(tmp_ctx);
+
+ msg01 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg01);
+
+ msg01->dn = ldb_dn_new_fmt(msg01, test_ctx->ldb, "dc=test01");
+ assert_non_null(msg01->dn);
+
+ ret = ldb_msg_add_string(msg01, "cn", "test_unique_index01");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg01);
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ msg02 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg02);
+
+ msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test01");
+ assert_non_null(msg02->dn);
+
+ ret = ldb_msg_add_string(msg02, "cn", "test_unique_index02");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg02);
+ assert_int_equal(ret, LDB_ERR_ENTRY_ALREADY_EXISTS);
+
+ assert_null(debug_string);
+ talloc_free(tmp_ctx);
+}
+
+static int ldb_guid_index_test_setup(void **state)
+{
+ int ret;
+ struct ldb_ldif *ldif;
+ struct ldbtest_ctx *ldb_test_ctx;
+ const char *attrs_ldif = \
+ "dn: @ATTRIBUTES\n"
+ "cn: UNIQUE_INDEX\n"
+ "\n";
+ const char *index_ldif = \
+ "dn: @INDEXLIST\n"
+ "@IDXATTR: cn\n"
+ "@IDXGUID: objectUUID\n"
+ "@IDX_DN_GUID: GUID\n"
+ "\n";
+
+ ldbtest_noconn_setup((void **) &ldb_test_ctx);
+
+
+ ret = ldb_connect(ldb_test_ctx->ldb, ldb_test_ctx->dbpath, 0, NULL);
+ assert_int_equal(ret, 0);
+
+ while ((ldif = ldb_ldif_read_string(ldb_test_ctx->ldb, &attrs_ldif))) {
+ ret = ldb_add(ldb_test_ctx->ldb, ldif->msg);
+ assert_int_equal(ret, LDB_SUCCESS);
+ }
+
+ while ((ldif = ldb_ldif_read_string(ldb_test_ctx->ldb, &index_ldif))) {
+ ret = ldb_add(ldb_test_ctx->ldb, ldif->msg);
+ assert_int_equal(ret, LDB_SUCCESS);
+ }
+
+ *state = ldb_test_ctx;
+ return 0;
+}
+
+static int ldb_guid_index_test_teardown(void **state)
+{
+ int ret;
+ struct ldbtest_ctx *ldb_test_ctx = talloc_get_type_abort(*state,
+ struct ldbtest_ctx);
+ struct ldb_dn *del_dn;
+
+ del_dn = ldb_dn_new_fmt(ldb_test_ctx,
+ ldb_test_ctx->ldb,
+ "@INDEXLIST");
+ assert_non_null(del_dn);
+
+ ret = ldb_delete(ldb_test_ctx->ldb, del_dn);
+ if (ret != LDB_ERR_NO_SUCH_OBJECT) {
+ assert_int_equal(ret, LDB_SUCCESS);
+ }
+
+ assert_dn_doesnt_exist(ldb_test_ctx,
+ "@INDEXLIST");
+
+ TALLOC_FREE(del_dn);
+
+ del_dn = ldb_dn_new_fmt(ldb_test_ctx,
+ ldb_test_ctx->ldb,
+ "@ATTRIBUTES");
+ assert_non_null(del_dn);
+
+ ret = ldb_delete(ldb_test_ctx->ldb, del_dn);
+ if (ret != LDB_ERR_NO_SUCH_OBJECT) {
+ assert_int_equal(ret, LDB_SUCCESS);
+ }
+
+ assert_dn_doesnt_exist(ldb_test_ctx,
+ "@ATTRIBUTES");
+
+ ldbtest_teardown((void **) &ldb_test_ctx);
+ return 0;
+}
+
+
+static void test_ldb_unique_index_duplicate_with_guid(void **state)
+{
+ int ret;
+ struct ldb_message *msg01;
+ struct ldb_message *msg02;
+ struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct ldbtest_ctx);
+ TALLOC_CTX *tmp_ctx;
+ char *debug_string = NULL;
+ char *p = NULL;
+
+ ldb_set_debug(test_ctx->ldb, ldb_debug_string, &debug_string);
+ tmp_ctx = talloc_new(test_ctx);
+ assert_non_null(tmp_ctx);
+
+ msg01 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg01);
+
+ msg01->dn = ldb_dn_new_fmt(msg01, test_ctx->ldb, "dc=test01");
+ assert_non_null(msg01->dn);
+
+ ret = ldb_msg_add_string(msg01, "cn", "test_unique_index");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_msg_add_string(msg01, "objectUUID", "0123456789abcdef");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg01);
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ msg02 = ldb_msg_new(tmp_ctx);
assert_non_null(msg01);
msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02");
@@ -3316,10 +3537,71 @@ static void test_ldb_add_to_index_unique_values_required(void **state)
ret = ldb_msg_add_string(msg02, "cn", "test_unique_index");
assert_int_equal(ret, LDB_SUCCESS);
+ ret = ldb_msg_add_string(msg02, "objectUUID", "0123456789abcde0");
+ assert_int_equal(ret, LDB_SUCCESS);
+
ret = ldb_add(test_ctx->ldb, msg02);
assert_int_equal(ret, LDB_ERR_CONSTRAINT_VIOLATION);
+
+ assert_non_null(debug_string);
+ p = strstr(
+ debug_string,
+ "unique index violation on cn in dc=test02, conficts with "
+ "objectUUID 0123456789abcdef in @INDEX:CN:test_unique_index");
+ assert_non_null(p);
+ free(debug_string);
+ talloc_free(tmp_ctx);
+}
+
+static void test_ldb_guid_index_duplicate_dn_logging(void **state)
+{
+ int ret;
+ struct ldb_message *msg01;
+ struct ldb_message *msg02;
+ struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct ldbtest_ctx);
+ TALLOC_CTX *tmp_ctx;
+ char *debug_string = NULL;
+
+ ldb_set_debug(test_ctx->ldb, ldb_debug_string, &debug_string);
+ tmp_ctx = talloc_new(test_ctx);
+ assert_non_null(tmp_ctx);
+
+ msg01 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg01);
+
+ msg01->dn = ldb_dn_new_fmt(msg01, test_ctx->ldb, "dc=test01");
+ assert_non_null(msg01->dn);
+
+ ret = ldb_msg_add_string(msg01, "cn", "test_unique_index01");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_msg_add_string(msg01, "objectUUID", "0123456789abcdef");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg01);
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ msg02 = ldb_msg_new(tmp_ctx);
+ assert_non_null(msg02);
+
+ msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test01");
+ assert_non_null(msg02->dn);
+
+ ret = ldb_msg_add_string(msg02, "cn", "test_unique_index02");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_msg_add_string(msg02, "objectUUID", "0123456789abcde1");
+ assert_int_equal(ret, LDB_SUCCESS);
+
+ ret = ldb_add(test_ctx->ldb, msg02);
+ assert_int_equal(ret, LDB_ERR_ENTRY_ALREADY_EXISTS);
+
+ assert_null(debug_string);
talloc_free(tmp_ctx);
}
+
+
int main(int argc, const char **argv)
{
const struct CMUnitTest tests[] = {
@@ -3459,6 +3741,22 @@ int main(int argc, const char **argv)
test_ldb_add_to_index_unique_values_required,
ldb_non_unique_index_test_setup,
ldb_non_unique_index_test_teardown),
+ cmocka_unit_test_setup_teardown(
+ test_ldb_unique_index_duplicate_logging,
+ ldb_unique_index_test_setup,
+ ldb_unique_index_test_teardown),
+ cmocka_unit_test_setup_teardown(
+ test_ldb_duplicate_dn_logging,
+ ldb_unique_index_test_setup,
+ ldb_unique_index_test_teardown),
+ cmocka_unit_test_setup_teardown(
+ test_ldb_guid_index_duplicate_dn_logging,
+ ldb_guid_index_test_setup,
+ ldb_guid_index_test_teardown),
+ cmocka_unit_test_setup_teardown(
+ test_ldb_unique_index_duplicate_with_guid,
+ ldb_guid_index_test_setup,
+ ldb_guid_index_test_teardown),
};
return cmocka_run_group_tests(tests, NULL, NULL);
--
2.7.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180223/4c74877d/signature.sig>
More information about the samba-technical
mailing list