[PATCH] Refuse to commit a faulty reindex

Andrew Bartlett abartlet at samba.org
Tue Apr 3 23:25:44 UTC 2018


The attached patches address a serious issue breaking upgrades to samba
4.8.

The re-index on the upgrade can fail (due to case-wise duplicate
servicePrincipalName values), and if it does then the reindex does not
complete, but the transaction around it can complete. 

This patch set ensures this cannot happen.  A part of the lmdb patch
set is included to avoid making that patch set harder to land.

Please review and push.

CI results are as part of the larger branch here:

https://gitlab.com/catalyst-samba/samba/pipelines/19924329/builds

and this one of just these patches:
https://gitlab.com/catalyst-samba/samba/pipelines/19925284

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From 216b54de513974d6496eb2abed56034913d653cb Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Mar 2018 16:01:13 +1300
Subject: [PATCH 1/3] ldb_tdb: Ensure we can not commit an index that is
 corrupt due to partial re-index

The re-index traverse can abort part-way though and we need to ensure
that the transaction is never committed as that will leave an un-useable db.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13335

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/ldb_tdb/ldb_tdb.c | 30 ++++++++++++++++++++++++++++++
 lib/ldb/ldb_tdb/ldb_tdb.h |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 9c8c77155c4..20796f2162d 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -410,6 +410,10 @@ static int ltdb_modified(struct ldb_module *module, struct ldb_dn *dn)
 		ret = ltdb_cache_reload(module);
 	}
 
+	if (ret != LDB_SUCCESS) {
+		ltdb->reindex_failed = true;
+	}
+
 	return ret;
 }
 
@@ -1443,9 +1447,17 @@ static int ltdb_start_trans(struct ldb_module *module)
 
 	ltdb_index_transaction_start(module);
 
+	ltdb->reindex_failed = false;
+
 	return LDB_SUCCESS;
 }
 
+/*
+ * Forward declaration to allow prepare_commit to in fact abort the
+ * transaction
+ */
+static int ltdb_del_trans(struct ldb_module *module);
+
 static int ltdb_prepare_commit(struct ldb_module *module)
 {
 	int ret;
@@ -1456,6 +1468,24 @@ static int ltdb_prepare_commit(struct ldb_module *module)
 		return LDB_SUCCESS;
 	}
 
+	/*
+	 * Check if the last re-index failed.
+	 *
+	 * This can happen if for example a duplicate value was marked
+	 * unique.  We must not write a partial re-index into the DB.
+	 */
+	if (ltdb->reindex_failed) {
+		/*
+		 * We must instead abort the transaction so we get the
+		 * old values and old index back
+		 */
+		ltdb_del_trans(module);
+		ldb_set_errstring(ldb_module_get_ctx(module),
+				  "Failure during re-index, so "
+				  "transaction must be aborted.");
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
 	ret = ltdb_index_transaction_commit(module);
 	if (ret != LDB_SUCCESS) {
 		ltdb->kv_ops->abort_write(ltdb);
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.h b/lib/ldb/ldb_tdb/ldb_tdb.h
index 2235bd47c98..28dd20915c7 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.h
+++ b/lib/ldb/ldb_tdb/ldb_tdb.h
@@ -65,6 +65,8 @@ struct ltdb_private {
 
 	bool read_only;
 
+	bool reindex_failed;
+
 	const struct ldb_schema_syntax *GUID_index_syntax;
 
 	/*
-- 
2.11.0


From 95c83217f0d5b8a886986cd2ab06493df70a517b Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 6 Mar 2018 09:13:31 +1300
Subject: [PATCH 2/3] lib ldb tests: Prepare to run api and index test on tdb
 and lmdb

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13335

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/tests/python/api.py   | 145 +++++++++++++++++++++++++-----------------
 lib/ldb/tests/python/index.py |  29 ++++++++-
 2 files changed, 114 insertions(+), 60 deletions(-)

diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py
index 85fe1bc360f..51c9c592f78 100755
--- a/lib/ldb/tests/python/api.py
+++ b/lib/ldb/tests/python/api.py
@@ -12,6 +12,9 @@ import shutil
 
 PY3 = sys.version_info > (3, 0)
 
+TDB_PREFIX = "tdb://"
+MDB_PREFIX = "mdb://"
+
 
 def tempdir():
     import tempfile
@@ -44,13 +47,36 @@ class NoContextTests(TestCase):
         encoded2 = ldb.binary_encode('test\\x')
         self.assertEqual(encoded2, encoded)
 
-class SimpleLdb(TestCase):
+
+class LdbBaseTest(TestCase):
+    def setUp(self):
+        super(LdbBaseTest, self).setUp()
+        try:
+            if self.prefix is None:
+                self.prefix = TDB_PREFIX
+        except AttributeError:
+            self.prefix = TDB_PREFIX
+
+    def tearDown(self):
+        super(LdbBaseTest, self).tearDown()
+
+    def url(self):
+        return self.prefix + self.filename
+
+    def flags(self):
+        if self.prefix == MDB_PREFIX:
+            return ldb.FLG_NOSYNC
+        else:
+            return 0
+
+
+class SimpleLdb(LdbBaseTest):
 
     def setUp(self):
         super(SimpleLdb, self).setUp()
         self.testdir = tempdir()
         self.filename = os.path.join(self.testdir, "test.ldb")
-        self.ldb = ldb.Ldb(self.filename)
+        self.ldb = ldb.Ldb(self.url(), flags=self.flags())
 
     def tearDown(self):
         shutil.rmtree(self.testdir)
@@ -58,16 +84,15 @@ class SimpleLdb(TestCase):
         # Ensure the LDB is closed now, so we close the FD
         del(self.ldb)
 
-
     def test_connect(self):
-        ldb.Ldb(self.filename)
+        ldb.Ldb(self.url(), flags=self.flags())
 
     def test_connect_none(self):
         ldb.Ldb()
 
     def test_connect_later(self):
         x = ldb.Ldb()
-        x.connect(self.filename)
+        x.connect(self.url(), flags=self.flags())
 
     def test_repr(self):
         x = ldb.Ldb()
@@ -82,7 +107,7 @@ class SimpleLdb(TestCase):
         self.assertEqual([], x.modules())
 
     def test_modules_tdb(self):
-        x = ldb.Ldb(self.filename)
+        x = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual("[<ldb module 'tdb'>]", repr(x.modules()))
 
     def test_firstmodule_none(self):
@@ -90,53 +115,53 @@ class SimpleLdb(TestCase):
         self.assertEqual(x.firstmodule, None)
 
     def test_firstmodule_tdb(self):
-        x = ldb.Ldb(self.filename)
+        x = ldb.Ldb(self.url(), flags=self.flags())
         mod = x.firstmodule
         self.assertEqual(repr(mod), "<ldb module 'tdb'>")
 
     def test_search(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(len(l.search()), 0)
 
     def test_search_controls(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(len(l.search(controls=["paged_results:0:5"])), 0)
 
     def test_search_attrs(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(len(l.search(ldb.Dn(l, ""), ldb.SCOPE_SUBTREE, "(dc=*)", ["dc"])), 0)
 
     def test_search_string_dn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(len(l.search("", ldb.SCOPE_SUBTREE, "(dc=*)", ["dc"])), 0)
 
     def test_search_attr_string(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertRaises(TypeError, l.search, attrs="dc")
         self.assertRaises(TypeError, l.search, attrs=b"dc")
 
     def test_opaque(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         l.set_opaque("my_opaque", l)
         self.assertTrue(l.get_opaque("my_opaque") is not None)
         self.assertEqual(None, l.get_opaque("unknown"))
 
     def test_search_scope_base_empty_db(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(len(l.search(ldb.Dn(l, "dc=foo1"),
                           ldb.SCOPE_BASE)), 0)
 
     def test_search_scope_onelevel_empty_db(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(len(l.search(ldb.Dn(l, "dc=foo1"),
                           ldb.SCOPE_ONELEVEL)), 0)
 
     def test_delete(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertRaises(ldb.LdbError, lambda: l.delete(ldb.Dn(l, "dc=foo2")))
 
     def test_delete_w_unhandled_ctrl(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=foo1")
         m["b"] = [b"a"]
@@ -145,10 +170,10 @@ class SimpleLdb(TestCase):
         l.delete(m.dn)
 
     def test_contains(self):
-        name = self.filename
-        l = ldb.Ldb(name)
+        name = self.url()
+        l = ldb.Ldb(name, flags=self.flags())
         self.assertFalse(ldb.Dn(l, "dc=foo3") in l)
-        l = ldb.Ldb(name)
+        l = ldb.Ldb(name, flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=foo3")
         m["b"] = ["a"]
@@ -160,23 +185,23 @@ class SimpleLdb(TestCase):
             l.delete(m.dn)
 
     def test_get_config_basedn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(None, l.get_config_basedn())
 
     def test_get_root_basedn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(None, l.get_root_basedn())
 
     def test_get_schema_basedn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(None, l.get_schema_basedn())
 
     def test_get_default_basedn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(None, l.get_default_basedn())
 
     def test_add(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=foo4")
         m["bla"] = b"bla"
@@ -188,7 +213,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=foo4"))
 
     def test_search_iterator(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         s = l.search_iterator()
         s.abandon()
         try:
@@ -288,7 +313,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=foo5"))
 
     def test_add_text(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=foo4")
         m["bla"] = "bla"
@@ -300,7 +325,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=foo4"))
 
     def test_add_w_unhandled_ctrl(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=foo4")
         m["bla"] = b"bla"
@@ -308,7 +333,7 @@ class SimpleLdb(TestCase):
         self.assertRaises(ldb.LdbError, lambda: l.add(m,["search_options:1:2"]))
 
     def test_add_dict(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = {"dn": ldb.Dn(l, "dc=foo5"),
              "bla": b"bla"}
         self.assertEqual(len(l.search()), 0)
@@ -319,7 +344,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=foo5"))
 
     def test_add_dict_text(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = {"dn": ldb.Dn(l, "dc=foo5"),
              "bla": "bla"}
         self.assertEqual(len(l.search()), 0)
@@ -330,7 +355,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=foo5"))
 
     def test_add_dict_string_dn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = {"dn": "dc=foo6", "bla": b"bla"}
         self.assertEqual(len(l.search()), 0)
         l.add(m)
@@ -340,7 +365,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=foo6"))
 
     def test_add_dict_bytes_dn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = {"dn": b"dc=foo6", "bla": b"bla"}
         self.assertEqual(len(l.search()), 0)
         l.add(m)
@@ -350,7 +375,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=foo6"))
 
     def test_rename(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=foo7")
         m["bla"] = b"bla"
@@ -363,7 +388,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=bar"))
 
     def test_rename_string_dns(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=foo8")
         m["bla"] = b"bla"
@@ -377,7 +402,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=bar"))
 
     def test_empty_dn(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertEqual(0, len(l.search()))
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=empty")
@@ -394,7 +419,7 @@ class SimpleLdb(TestCase):
         self.assertEqual(0, len(rm[0]))
 
     def test_modify_delete(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=modifydelete")
         m["bla"] = [b"1234"]
@@ -417,7 +442,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=modifydelete"))
 
     def test_modify_delete_text(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=modifydelete")
         m.text["bla"] = ["1234"]
@@ -440,7 +465,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=modifydelete"))
 
     def test_modify_add(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=add")
         m["bla"] = [b"1234"]
@@ -458,7 +483,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=add"))
 
     def test_modify_add_text(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=add")
         m.text["bla"] = ["1234"]
@@ -476,7 +501,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=add"))
 
     def test_modify_replace(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=modify2")
         m["bla"] = [b"1234", b"456"]
@@ -496,7 +521,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=modify2"))
 
     def test_modify_replace_text(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=modify2")
         m.text["bla"] = ["1234", "456"]
@@ -516,7 +541,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=modify2"))
 
     def test_modify_flags_change(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=add")
         m["bla"] = [b"1234"]
@@ -542,7 +567,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=add"))
 
     def test_modify_flags_change_text(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         m = ldb.Message()
         m.dn = ldb.Dn(l, "dc=add")
         m.text["bla"] = ["1234"]
@@ -568,7 +593,7 @@ class SimpleLdb(TestCase):
             l.delete(ldb.Dn(l, "dc=add"))
 
     def test_transaction_commit(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         l.transaction_start()
         m = ldb.Message(ldb.Dn(l, "dc=foo9"))
         m["foo"] = [b"bar"]
@@ -577,7 +602,7 @@ class SimpleLdb(TestCase):
         l.delete(m.dn)
 
     def test_transaction_cancel(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         l.transaction_start()
         m = ldb.Message(ldb.Dn(l, "dc=foo10"))
         m["foo"] = [b"bar"]
@@ -588,12 +613,12 @@ class SimpleLdb(TestCase):
     def test_set_debug(self):
         def my_report_fn(level, text):
             pass
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         l.set_debug(my_report_fn)
 
     def test_zero_byte_string(self):
         """Testing we do not get trapped in the \0 byte in a property string."""
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         l.add({
             "dn" : b"dc=somedn",
             "objectclass" : b"user",
@@ -605,10 +630,10 @@ class SimpleLdb(TestCase):
         self.assertEqual(b"foo\0bar", res[0]["displayname"][0])
 
     def test_no_crash_broken_expr(self):
-        l = ldb.Ldb(self.filename)
+        l = ldb.Ldb(self.url(), flags=self.flags())
         self.assertRaises(ldb.LdbError,lambda: l.search("", ldb.SCOPE_SUBTREE, "&(dc=*)(dn=*)", ["dc"]))
 
-class SearchTests(TestCase):
+class SearchTests(LdbBaseTest):
     def tearDown(self):
         shutil.rmtree(self.testdir)
         super(SearchTests, self).tearDown()
@@ -621,7 +646,9 @@ class SearchTests(TestCase):
         super(SearchTests, self).setUp()
         self.testdir = tempdir()
         self.filename = os.path.join(self.testdir, "search_test.ldb")
-        self.l = ldb.Ldb(self.filename, options=["modules:rdn_name"])
+        self.l = ldb.Ldb(self.url(),
+                         flags=self.flags(),
+                         options=["modules:rdn_name"])
 
         self.l.add({"dn": "@ATTRIBUTES",
                     "DC": "CASE_INSENSITIVE"})
@@ -1030,7 +1057,6 @@ class SearchTests(TestCase):
         self.assertEqual(len(res11), 1)
 
 
-
 class IndexedSearchTests(SearchTests):
     """Test searches using the index, to ensure the index doesn't
        break things"""
@@ -1091,6 +1117,7 @@ class GUIDIndexedSearchTests(SearchTests):
         self.IDXGUID = True
         self.IDXONE = True
 
+
 class GUIDIndexedDNFilterSearchTests(SearchTests):
     """Test searches using the index, to ensure the index doesn't
        break things"""
@@ -1126,7 +1153,7 @@ class GUIDAndOneLevelIndexedSearchTests(SearchTests):
         self.IDXONE = True
 
 
-class AddModifyTests(TestCase):
+class AddModifyTests(LdbBaseTest):
     def tearDown(self):
         shutil.rmtree(self.testdir)
         super(AddModifyTests, self).tearDown()
@@ -1138,7 +1165,9 @@ class AddModifyTests(TestCase):
         super(AddModifyTests, self).setUp()
         self.testdir = tempdir()
         self.filename = os.path.join(self.testdir, "add_test.ldb")
-        self.l = ldb.Ldb(self.filename, options=["modules:rdn_name"])
+        self.l = ldb.Ldb(self.url(),
+                         flags=self.flags(),
+                         options=["modules:rdn_name"])
         self.l.add({"dn": "DC=SAMBA,DC=ORG",
                     "name": b"samba.org",
                     "objectUUID": b"0123456789abcdef"})
@@ -1266,6 +1295,7 @@ class AddModifyTests(TestCase):
                     "x": "z", "y": "a",
                     "objectUUID": b"0123456789abcde3"})
 
+
 class IndexedAddModifyTests(AddModifyTests):
     """Test searches using the index, to ensure the index doesn't
        break things"""
@@ -1378,7 +1408,6 @@ class TransIndexedAddModifyTests(IndexedAddModifyTests):
         super(TransIndexedAddModifyTests, self).tearDown()
 
 
-
 class DnTests(TestCase):
 
     def setUp(self):
@@ -1989,13 +2018,13 @@ class ModuleTests(TestCase):
         l = ldb.Ldb(self.filename)
         self.assertEqual(["init"], ops)
 
-class LdbResultTests(TestCase):
+class LdbResultTests(LdbBaseTest):
 
     def setUp(self):
         super(LdbResultTests, self).setUp()
         self.testdir = tempdir()
         self.filename = os.path.join(self.testdir, "test.ldb")
-        self.l = ldb.Ldb(self.filename)
+        self.l = ldb.Ldb(self.url(), flags=self.flags())
         self.l.add({"dn": "DC=SAMBA,DC=ORG", "name": b"samba.org"})
         self.l.add({"dn": "OU=ADMIN,DC=SAMBA,DC=ORG", "name": b"Admins"})
         self.l.add({"dn": "OU=USERS,DC=SAMBA,DC=ORG", "name": b"Users"})
@@ -2103,7 +2132,7 @@ class LdbResultTests(TestCase):
             del(self.l)
             gc.collect()
 
-            child_ldb = ldb.Ldb(self.filename)
+            child_ldb = ldb.Ldb(self.url(), flags=self.flags())
             # start a transaction
             child_ldb.transaction_start()
 
@@ -2174,7 +2203,7 @@ class LdbResultTests(TestCase):
             del(self.l)
             gc.collect()
 
-            child_ldb = ldb.Ldb(self.filename)
+            child_ldb = ldb.Ldb(self.url(), flags=self.flags())
             # start a transaction
             child_ldb.transaction_start()
 
diff --git a/lib/ldb/tests/python/index.py b/lib/ldb/tests/python/index.py
index 239b2bff3fd..e9eaaf059e1 100755
--- a/lib/ldb/tests/python/index.py
+++ b/lib/ldb/tests/python/index.py
@@ -32,6 +32,9 @@ import shutil
 
 PY3 = sys.version_info > (3, 0)
 
+TDB_PREFIX = "tdb://"
+MDB_PREFIX = "mdb://"
+
 
 def tempdir():
     import tempfile
@@ -52,7 +55,29 @@ def contains(result, dn):
     return False
 
 
-class MaxIndexKeyLengthTests(TestCase):
+class LdbBaseTest(TestCase):
+    def setUp(self):
+        super(LdbBaseTest, self).setUp()
+        try:
+            if self.prefix is None:
+                self.prefix = TDB_PREFIX
+        except AttributeError:
+            self.prefix = TDB_PREFIX
+
+    def tearDown(self):
+        super(LdbBaseTest, self).tearDown()
+
+    def url(self):
+        return self.prefix + self.filename
+
+    def flags(self):
+        if self.prefix == MDB_PREFIX:
+            return ldb.FLG_NOSYNC
+        else:
+            return 0
+
+
+class MaxIndexKeyLengthTests(LdbBaseTest):
     def checkGuids(self, key, guids):
         #
         # This check relies on the current implementation where the indexes
@@ -89,7 +114,7 @@ class MaxIndexKeyLengthTests(TestCase):
         self.testdir = tempdir()
         self.filename = os.path.join(self.testdir, "key_len_test.ldb")
         # Note that the maximum key length is set to 50
-        self.l = ldb.Ldb(self.filename,
+        self.l = ldb.Ldb(self.url(),
                          options=[
                              "modules:rdn_name",
                              "max_key_len_for_self_test:50"])
-- 
2.11.0


From b4125238ac8bc2f83ef68abf20a8c979562ff29e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 26 Mar 2018 16:07:45 +1300
Subject: [PATCH 3/3] ldb: Add test to show a reindex failure must not leave
 the DB corrupt

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13335

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/tests/python/api.py | 160 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 160 insertions(+)

diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py
index 51c9c592f78..12409a8c991 100755
--- a/lib/ldb/tests/python/api.py
+++ b/lib/ldb/tests/python/api.py
@@ -1408,6 +1408,166 @@ class TransIndexedAddModifyTests(IndexedAddModifyTests):
         super(TransIndexedAddModifyTests, self).tearDown()
 
 
+class BadIndexTests(LdbBaseTest):
+    def setUp(self):
+        super(BadIndexTests, self).setUp()
+        self.testdir = tempdir()
+        self.filename = os.path.join(self.testdir, "test.ldb")
+        self.ldb = ldb.Ldb(self.url(), flags=self.flags())
+        if hasattr(self, 'IDXGUID'):
+            self.ldb.add({"dn": "@INDEXLIST",
+                          "@IDXATTR": [b"x", b"y", b"ou"],
+                          "@IDXGUID": [b"objectUUID"],
+                          "@IDX_DN_GUID": [b"GUID"]})
+        else:
+            self.ldb.add({"dn": "@INDEXLIST",
+                          "@IDXATTR": [b"x", b"y", b"ou"]})
+
+        super(BadIndexTests, self).setUp()
+
+    def test_unique(self):
+        self.ldb.add({"dn": "x=x,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde1",
+                      "y": "1"})
+        self.ldb.add({"dn": "x=y,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde2",
+                      "y": "1"})
+        self.ldb.add({"dn": "x=z,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde3",
+                      "y": "1"})
+
+        res = self.ldb.search(expression="(y=1)",
+                              base="dc=samba,dc=org")
+        self.assertEquals(len(res), 3)
+
+        # Now set this to unique index, but forget to check the result
+        try:
+            self.ldb.add({"dn": "@ATTRIBUTES",
+                        "y": "UNIQUE_INDEX"})
+            self.fail()
+        except ldb.LdbError:
+            pass
+
+        # We must still have a working index
+        res = self.ldb.search(expression="(y=1)",
+                              base="dc=samba,dc=org")
+        self.assertEquals(len(res), 3)
+
+    def test_unique_transaction(self):
+        self.ldb.add({"dn": "x=x,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde1",
+                      "y": "1"})
+        self.ldb.add({"dn": "x=y,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde2",
+                      "y": "1"})
+        self.ldb.add({"dn": "x=z,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde3",
+                      "y": "1"})
+
+        res = self.ldb.search(expression="(y=1)",
+                              base="dc=samba,dc=org")
+        self.assertEquals(len(res), 3)
+
+        self.ldb.transaction_start()
+
+        # Now set this to unique index, but forget to check the result
+        try:
+            self.ldb.add({"dn": "@ATTRIBUTES",
+                        "y": "UNIQUE_INDEX"})
+        except ldb.LdbError:
+            pass
+
+        try:
+            self.ldb.transaction_commit()
+            self.fail()
+
+        except ldb.LdbError as err:
+            enum = err.args[0]
+            self.assertEqual(enum, ldb.ERR_OPERATIONS_ERROR)
+
+        # We must still have a working index
+        res = self.ldb.search(expression="(y=1)",
+                              base="dc=samba,dc=org")
+
+        self.assertEquals(len(res), 3)
+
+    def test_casefold(self):
+        self.ldb.add({"dn": "x=x,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde1",
+                      "y": "a"})
+        self.ldb.add({"dn": "x=y,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde2",
+                      "y": "A"})
+        self.ldb.add({"dn": "x=z,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde3",
+                      "y": ["a", "A"]})
+
+        res = self.ldb.search(expression="(y=a)",
+                              base="dc=samba,dc=org")
+        self.assertEquals(len(res), 2)
+
+        self.ldb.add({"dn": "@ATTRIBUTES",
+                      "y": "CASE_INSENSITIVE"})
+
+        # We must still have a working index
+        res = self.ldb.search(expression="(y=a)",
+                              base="dc=samba,dc=org")
+
+        if hasattr(self, 'IDXGUID'):
+            self.assertEquals(len(res), 3)
+        else:
+            # We should not return this entry twice, but sadly
+            # we have not yet fixed
+            # https://bugzilla.samba.org/show_bug.cgi?id=13361
+            self.assertEquals(len(res), 4)
+
+    def test_casefold_transaction(self):
+        self.ldb.add({"dn": "x=x,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde1",
+                      "y": "a"})
+        self.ldb.add({"dn": "x=y,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde2",
+                      "y": "A"})
+        self.ldb.add({"dn": "x=z,dc=samba,dc=org",
+                      "objectUUID": b"0123456789abcde3",
+                      "y": ["a", "A"]})
+
+        res = self.ldb.search(expression="(y=a)",
+                              base="dc=samba,dc=org")
+        self.assertEquals(len(res), 2)
+
+        self.ldb.transaction_start()
+
+        self.ldb.add({"dn": "@ATTRIBUTES",
+                      "y": "CASE_INSENSITIVE"})
+
+        self.ldb.transaction_commit()
+
+        # We must still have a working index
+        res = self.ldb.search(expression="(y=a)",
+                              base="dc=samba,dc=org")
+
+        if hasattr(self, 'IDXGUID'):
+            self.assertEquals(len(res), 3)
+        else:
+            # We should not return this entry twice, but sadly
+            # we have not yet fixed
+            # https://bugzilla.samba.org/show_bug.cgi?id=13361
+            self.assertEquals(len(res), 4)
+
+
+    def tearDown(self):
+        super(BadIndexTests, self).tearDown()
+
+
+class GUIDBadIndexTests(BadIndexTests):
+    """Test Bad index things with GUID index mode"""
+    def setUp(self):
+        self.IDXGUID = True
+
+        super(GUIDBadIndexTests, self).setUp()
+
+
 class DnTests(TestCase):
 
     def setUp(self):
-- 
2.11.0



More information about the samba-technical mailing list