From 0a1aed4e717e68c4b6dbf3c21e9cc04ebe5b5171 Mon Sep 17 00:00:00 2001 From: Baptiste Lemaire Date: Tue, 22 Jun 2021 11:45:14 -0700 Subject: [PATCH] Fix double slashes in user-provided db path. (#8439) Summary: At the moment, the following command : "`./ --db=mypath/ dump_file_files`" returns a series of erronous names with double slashes, ie: "`mypath//000xxx.sst`", including manifest file names with double slashes "`mypath//MANIFEST-00XXX`", whereas "`./ --db=mypath dump_file_files`" correctly returns "`mypath/000xxx.sst`" and "`mypath/MANIFEST-00XXX`". This (very short) PR simply checks if there is a need to add or remove any '`/`' character when the `db_path` and `manifest_filename`/sst `filenames` are concatenated. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8439 Reviewed By: akankshamahajan15 Differential Revision: D29301349 Pulled By: bjlemaire fbshipit-source-id: 3e9e58f9749d278b654ae838fcee13ad698705a8 --- tools/ldb_cmd.cc | 11 ++++++++++- tools/ldb_test.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 118f39802..ec4e0d0f6 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -3352,6 +3352,11 @@ void DBFileDumperCommand::DoCommand() { // remove the trailing '\n' manifest_filename.resize(manifest_filename.size() - 1); std::string manifest_filepath = db_->GetName() + "/" + manifest_filename; + // Correct concatenation of filepath and filename: + // Check that there is no double slashes (or more!) when concatenation + // happens. + manifest_filepath = NormalizePath(manifest_filepath); + std::cout << manifest_filepath << std::endl; DumpManifestFile(options_, manifest_filepath, false, false, false); std::cout << std::endl; @@ -3361,7 +3366,11 @@ void DBFileDumperCommand::DoCommand() { std::vector metadata; db_->GetLiveFilesMetaData(&metadata); for (auto& fileMetadata : metadata) { - std::string filename = fileMetadata.db_path + fileMetadata.name; + std::string filename = fileMetadata.db_path + "/" + fileMetadata.name; + // Correct concatenation of filepath and filename: + // Check that there is no double slashes (or more!) when concatenation + // happens. + filename = NormalizePath(filename); std::cout << filename << " level:" << fileMetadata.level << std::endl; std::cout << "------------------------------" << std::endl; DumpSstFile(options_, filename, false, true); diff --git a/tools/ldb_test.py b/tools/ldb_test.py index f329699a2..699317b95 100644 --- a/tools/ldb_test.py +++ b/tools/ldb_test.py @@ -423,8 +423,36 @@ class LDBTestCase(unittest.TestCase): self.assertRunOK("delete x1", "OK") self.assertRunOK("put x3 y3", "OK") dumpFilePath = os.path.join(self.TMP_DIR, "dump2") + + # Test that if the user provides a db path that ends with + # a slash '/', there is no double (or more!) slashes in the + # SST and manifest file names. + + # Add a '/' at the end of dbPath (which normally shouldnt contain any) + if dbPath[-1] != "/": + dbPath += "/" + + # Call the dump_live_files function with the edited dbPath name. self.assertTrue(self.dumpLiveFiles("--db=%s" % dbPath, dumpFilePath)) + # Investigate the output + with open(dumpFilePath, "r") as tmp: + data = tmp.read() + + # Check that all the SST filenames have a correct full path (no multiple '/'). + sstFileList = re.findall(r"%s.*\d+.sst" % dbPath, data) + for sstFilename in sstFileList: + filenumber = re.findall(r"\d+.sst", sstFilename)[0] + self.assertEqual(sstFilename, dbPath+filenumber) + + # Check that all the manifest filenames + # have a correct full path (no multiple '/'). + manifestFileList = re.findall(r"%s.*MANIFEST-\d+" % dbPath, data) + for manifestFilename in manifestFileList: + filenumber = re.findall(r"(?<=MANIFEST-)\d+", manifestFilename)[0] + self.assertEqual(manifestFilename, dbPath+"MANIFEST-"+filenumber) + + def getManifests(self, directory): return glob.glob(directory + "/MANIFEST-*")