Issue 6090 - Fix dbscan options and man pages (389ds#6315)

* Issue 6090 - Fix dbscan options and man pages

dbscan -d option is dangerously confusing as it removes a database instance while in db_stat it identify the database
(cf issue 389ds#5609 ).
This fix implements long options in dbscan, rename -d in --remove, and requires a new --do-it option for action that change the database content.
The fix should also align both the usage and the dbscan man page with the new set of options

Issue: 389ds#6090

Reviewed by: @tbordaz, @droideck (Thanks!)

dirsrvtests/tests/suites/clu/dbscan_test.py

@@ -0,0 +1,253 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2024 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+import logging
+import os
+import pytest
+import re
+import subprocess
+import sys
+
+from lib389 import DirSrv
+from lib389._constants import DBSCAN
+from lib389.topologies import topology_m2 as topo_m2
+from difflib import context_diff
+
+pytestmark = pytest.mark.tier0
+
+logging.getLogger(__name__).setLevel(logging.DEBUG)
+log = logging.getLogger(__name__)
+
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+
+
+class CalledProcessUnexpectedReturnCode(subprocess.CalledProcessError):
+    def __init__(self, result, expected_rc):
+        super().__init__(cmd=result.args, returncode=result.returncode, output=result.stdout, stderr=result.stderr)
+        self.expected_rc = expected_rc
+        self.result = result
+
+    def __str__(self):
+        return f'Command {self.result.args} returned {self.result.returncode} instead of {self.expected_rc}'
+
+
+class DbscanPaths:
+    @staticmethod
+    def list_instances(inst, dblib, dbhome):
+        # compute db instance pathnames
+        instances = dbscan(['-D', dblib, '-L', dbhome], inst=inst).stdout
+        dbis = []
+        if dblib == 'bdb':
+            pattern = r'^ (.*) $'
+            prefix = f'{dbhome}/'
+        else:
+            pattern = r'^ (.*) flags:'
+            prefix = f''
+        for match in re.finditer(pattern, instances, flags=re.MULTILINE):
+            dbis.append(prefix+match.group(1))
+        return dbis
+
+    @staticmethod
+    def list_options(inst):
+        # compute supported options
+        options = []
+        usage = dbscan(['-h'], inst=inst, expected_rc=None).stdout
+        pattern = r'^\s+(?:(-[^-,]+), +)?(--[^ ]+).*$'
+        for match in re.finditer(pattern, usage, flags=re.MULTILINE):
+            for idx in range(1,3):
+                if match.group(idx) is not None:
+                    options.append(match.group(idx))
+        return options
+
+    def __init__(self, inst):
+        dblib = inst.get_db_lib()
+        dbhome = inst.ds_paths.db_home_dir
+        self.inst = inst
+        self.dblib = dblib
+        self.dbhome = dbhome
+        self.options = DbscanPaths.list_options(inst)
+        self.dbis = DbscanPaths.list_instances(inst, dblib, dbhome)
+        self.ldif_dir = inst.ds_paths.ldif_dir
+
+    def get_dbi(self, attr, backend='userroot'):
+        for dbi in self.dbis:
+            if f'{backend}/{attr}.'.lower() in dbi.lower():
+                return dbi
+        raise KeyError(f'Unknown dbi {backend}/{attr}')
+
+    def __repr__(self):
+        attrs = ['inst', 'dblib', 'dbhome', 'ldif_dir', 'options', 'dbis' ]
+        res = ", ".join(map(lambda x: f'{x}={self.__dict__[x]}', attrs))
+        return f'DbscanPaths({res})'
+
+
+def dbscan(args, inst=None, expected_rc=0):
+    if inst is None:
+        prefix = os.environ.get('PREFIX', "")
+        prog = f'{prefix}/bin/dbscan'
+    else:
+        prog = os.path.join(inst.ds_paths.bin_dir, DBSCAN)
+    args.insert(0, prog)
+    output = subprocess.run(args, encoding='utf-8', stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+    log.debug(f'{args} result is {output.returncode} output is {output.stdout}')
+    if expected_rc is not None and expected_rc != output.returncode:
+        raise CalledProcessUnexpectedReturnCode(output, expected_rc)
+    return output
+
+
+def log_export_file(filename):
+    with open(filename, 'r') as file:
+        log.debug(f'=========== Dump of {filename} ================')
+        for line in file:
+            log.debug(line.rstrip('\n'))
+        log.debug(f'=========== Enf of {filename} =================')
+
+
+@pytest.fixture(scope='module')
+def paths(topo_m2, request):
+    inst = topo_m2.ms["supplier1"]
+    if sys.version_info < (3,5):
+        pytest.skip('requires python version >= 3.5')
+    paths = DbscanPaths(inst)
+    if '--do-it' not in paths.options:
+       pytest.skip('Not supported with this dbscan version')
+    inst.stop()
+    return paths
+
+
+def test_dbscan_destructive_actions(paths, request):
+    """Test that dbscan remove/import actions
+
+    :id: f40b0c42-660a-11ef-9544-083a88554478
+    :setup: Stopped standalone instance
+    :steps:
+         1. Export cn instance with dbscan
+         2. Run dbscan --remove ...
+         3. Check the error message about missing --do-it
+         4. Check that cn instance is still present
+         5. Run dbscan -I import_file ...
+         6. Check it was properly imported
+         7. Check that cn instance is still present
+         8. Run dbscan --remove ... --doit
+         9. Check the error message about missing --do-it
+         10. Check that cn instance is still present
+         11. Run dbscan -I import_file ... --do-it
+         12. Check it was properly imported
+         13. Check that cn instance is still present
+         14. Export again the database
+         15. Check that content of export files are the same
+    :expectedresults:
+         1. Success
+         2. dbscan return code should be 1 (error)
+         3. Error message should be present
+         4. cn instance should be present
+         5. dbscan return code should be 1 (error)
+         6. Error message should be present
+         7. cn instance should be present
+         8. dbscan return code should be 0 (success)
+         9. Error message should not be present
+         10. cn instance should not be present
+         11. dbscan return code should be 0 (success)
+         12. Error message should not be present
+         13. cn instance should be present
+         14. Success
+         15. Export files content should be the same
+    """
+
+    # Export cn instance with dbscan
+    export_cn = f'{paths.ldif_dir}/dbscan_cn.data'
+    export_cn2 = f'{paths.ldif_dir}/dbscan_cn2.data'
+    cndbi = paths.get_dbi('replication_changelog')
+    inst = paths.inst
+    dblib = paths.dblib
+    exportok = False
+    def fin():
+        if os.path.exists(export_cn):
+            # Restore cn if it was exported successfully but does not exists any more
+            if exportok and cndbi not in DbscanPaths.list_instances(inst, dblib, paths.dbhome):
+                    dbscan(['-D', dblib, '-f', cndbi, '-I', export_cn, '--do-it'], inst=inst)
+            if not DEBUGGING:
+                os.remove(export_cn)
+        if os.path.exists(export_cn) and not DEBUGGING:
+            os.remove(export_cn2)
+
+    fin()
+    request.addfinalizer(fin)
+    dbscan(['-D', dblib,  '-f', cndbi, '-X', export_cn], inst=inst)
+    exportok = True
+
+    expected_msg = "without specifying '--do-it' parameter."
+
+    # Run dbscan --remove ...
+    result = dbscan(['-D', paths.dblib, '--remove', '-f', cndbi],
+                    inst=paths.inst, expected_rc=1)
+
+    # Check the error message about missing --do-it
+    assert expected_msg in result.stdout
+
+    # Check that cn instance is still present
+    curdbis = DbscanPaths.list_instances(paths.inst, paths.dblib, paths.dbhome)
+    assert cndbi in curdbis
+
+    # Run dbscan -I import_file ...
+    result = dbscan(['-D', paths.dblib, '-f', cndbi, '-I', export_cn],
+                    inst=paths.inst, expected_rc=1)
+
+    # Check the error message about missing --do-it
+    assert expected_msg in result.stdout
+
+    # Check that cn instance is still present
+    curdbis = DbscanPaths.list_instances(paths.inst, paths.dblib, paths.dbhome)
+    assert cndbi in curdbis
+
+    # Run dbscan --remove ... --doit
+    result = dbscan(['-D', paths.dblib, '--remove', '-f', cndbi, '--do-it'],
+                    inst=paths.inst, expected_rc=0)
+
+    # Check the error message about missing --do-it
+    assert expected_msg not in result.stdout
+
+    # Check that cn instance is still present
+    curdbis = DbscanPaths.list_instances(paths.inst, paths.dblib, paths.dbhome)
+    assert cndbi not in curdbis
+
+    # Run dbscan -I import_file ... --do-it
+    result = dbscan(['-D', paths.dblib, '-f', cndbi,
+                     '-I', export_cn, '--do-it'],
+                    inst=paths.inst, expected_rc=0)
+
+    # Check the error message about missing --do-it
+    assert expected_msg not in result.stdout
+
+    # Check that cn instance is still present
+    curdbis = DbscanPaths.list_instances(paths.inst, paths.dblib, paths.dbhome)
+    assert cndbi in curdbis
+
+    # Export again the database
+    dbscan(['-D', dblib,  '-f', cndbi, '-X', export_cn2], inst=inst)
+
+    # Check that content of export files are the same
+    with open(export_cn) as f1:
+        f1lines = f1.readlines()
+    with open(export_cn2) as f2:
+        f2lines = f2.readlines()
+    diffs = list(context_diff(f1lines, f2lines))
+    if len(diffs) > 0:
+        log.debug("Export file differences are:")
+        for d in diffs:
+            log.debug(d)
+        log_export_file(export_cn)
+        log_export_file(export_cn2)
+        assert diffs is None
+
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main("-s %s" % CURRENT_FILE)

dirsrvtests/tests/suites/clu/repl_monitor_test.py

@@ -77,13 +77,13 @@ def get_hostnames_from_log(port1, port2):
     # search for Supplier :hostname:port 
     # and use \D to insure there is no more number is after
     # the matched port (i.e that 10 is not matching 101)
-    regexp = '(Supplier: )([^:]*)(:' + str(port1) + '\D)'
+    regexp = '(Supplier: )([^:]*)(:' + str(port1) + r'\D)'
     match=re.search(regexp, logtext)
     host_m1 = 'localhost.localdomain'
     if (match is not None):
         host_m1 = match.group(2)
     # Same for supplier 2 
-    regexp = '(Supplier: )([^:]*)(:' + str(port2) + '\D)'
+    regexp = '(Supplier: )([^:]*)(:' + str(port2) + r'\D)'
     match=re.search(regexp, logtext)
     host_m2 = 'localhost.localdomain'
     if (match is not None):

ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c

@@ -5794,8 +5794,16 @@ bdb_import_file_name(ldbm_instance *inst)
 static char *
 bdb_restore_file_name(struct ldbminfo *li)
 {
-    char *fname = slapi_ch_smprintf("%s/../.restore", li->li_directory);
-
+    char *pt = strrchr(li->li_directory, '/');
+    char *fname =  NULL;
+    if (pt == NULL) {
+        fname = slapi_ch_strdup(".restore");
+    } else {
+        size_t len = pt-li->li_directory;
+        fname = slapi_ch_malloc(len+10);
+        strncpy(fname, li->li_directory, len);
+        strcpy(fname+len, "/.restore");
+    }
     return fname;
 }
 

ldap/servers/slapd/back-ldbm/dbimpl.c

@@ -397,7 +397,48 @@ const char *dblayer_op2str(dbi_op_t op)
     return str[idx];
 }
 
-/* Open db env, db and db file privately */
+/* Get the li_directory directory from the database instance name -
+ * Caller should free the returned value
+ */
+static char *
+get_li_directory(const char *fname)
+{
+    /*
+     * li_directory is an existing directory.
+     * it can be fname or its parent or its greatparent
+     * in case of problem returns the provided name
+     */
+    char *lid = slapi_ch_strdup(fname);
+    struct stat sbuf = {0};
+    char *pt = NULL;
+    for (int count=0; count<3; count++) {
+        if (stat(lid, &sbuf) == 0) {
+            if (S_ISDIR(sbuf.st_mode)) {
+                return lid;
+            }
+            /* Non directory existing file could be regular
+             * at the first iteration otherwise it is an error.
+             */
+            if (count>0 || !S_ISREG(sbuf.st_mode)) {
+                break;
+            }
+        }
+        pt = strrchr(lid, '/');
+        if (pt == NULL) {
+            slapi_ch_free_string(&lid);
+            return slapi_ch_strdup(".");
+        }
+        *pt = '\0';
+    }
+    /*
+     * Error case. Returns a copy of the original string:
+     *  and let dblayer_private_open_fn fail to open the database
+     */
+    slapi_ch_free_string(&lid);
+    return slapi_ch_strdup(fname);
+}
+
+/* Open db env, db and db file privately (for dbscan) */
 int dblayer_private_open(const char *plgname, const char *dbfilename, int rw, Slapi_Backend **be, dbi_env_t **env, dbi_db_t **db)
 {
     struct ldbminfo *li;
@@ -412,7 +453,7 @@ int dblayer_private_open(const char *plgname, const char *dbfilename, int rw, Sl
     li->li_plugin = (*be)->be_database;
     li->li_plugin->plg_name = (char*) "back-ldbm-dbimpl";
     li->li_plugin->plg_libpath = (char*) "libback-ldbm";
-    li->li_directory = slapi_ch_strdup(dbfilename);
+    li->li_directory = get_li_directory(dbfilename);
 
     /* Initialize database plugin */
     rc = dbimpl_setup(li, plgname);
@@ -439,7 +480,10 @@ int dblayer_private_close(Slapi_Backend **be, dbi_env_t **env, dbi_db_t **db)
         }
         slapi_ch_free((void**)&li->li_dblayer_private);
         slapi_ch_free((void**)&li->li_dblayer_config);
-        ldbm_config_destroy(li);
+        if (dblayer_is_lmdb(*be)) {
+            /* Generate use after free and double free in bdb case */
+            ldbm_config_destroy(li);
+        }
         slapi_ch_free((void**)&(*be)->be_database);
         slapi_ch_free((void**)&(*be)->be_instance_info);
         slapi_ch_free((void**)be);