tglogutil-compaction: reject symlink target

project-tsurugi · Sep 29, 2024 · 1c56836 · 1c56836
1 parent d581188
commit 1c56836
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 6 deletions.
diff --git a/src/limestone/dblogutil/dblogutil.cpp b/src/limestone/dblogutil/dblogutil.cpp
@@ -187,6 +187,14 @@ void compaction(dblog_scan &ds, std::optional<epoch_id_type> epoch) {
         std::cout << "durable-epoch: " << ld_epoch << std::endl;
     }
     auto from_dir = ds.get_dblogdir();
+    {
+        auto p = from_dir;  // make copy
+        remove_trailing_dir_separators(p);
+        if (boost::filesystem::is_symlink(p)) {
+            LOG(ERROR) << "dblogdir is symlink; compaction target must not be symlink";
+            log_and_exit(64);
+        }
+    }
     boost::filesystem::path tmp;
     if (!FLAGS_working_dir.empty()) {
         tmp = FLAGS_working_dir;

diff --git a/src/limestone/filepath.cpp b/src/limestone/filepath.cpp
@@ -22,16 +22,21 @@
 
 namespace limestone::internal {
 
+void remove_trailing_dir_separators(boost::filesystem::path& p) {
+    std::string str = p.string();
+    std::size_t prev_len{};
+    do {
+        prev_len = str.size();
+        p.remove_trailing_separator();  // remove only one char
+        str = p.string();
+    } while (str.size() < prev_len);
+}
+
 boost::filesystem::path make_tmp_dir_next_to(const boost::filesystem::path& target_dir, const char* suffix) {
     auto canonicalpath = boost::filesystem::canonical(target_dir);
     // some versions of boost::filesystem::canonical do not remove trailing directory-separators ('/')
+    remove_trailing_dir_separators(canonicalpath);
     std::string targetdirstring = canonicalpath.string();
-    std::size_t prev_len{};
-    do {
-        prev_len = targetdirstring.size();
-        canonicalpath.remove_trailing_separator();  // remove only one char
-        targetdirstring = canonicalpath.string();
-    } while (targetdirstring.size() < prev_len);
 
     auto tmpdirname = targetdirstring + suffix;
     if (::mkdtemp(tmpdirname.data()) == nullptr) {

diff --git a/src/limestone/internal.h b/src/limestone/internal.h
@@ -70,6 +70,7 @@ void create_compact_pwal(
 
 // filepath.cpp
 
+void remove_trailing_dir_separators(boost::filesystem::path& p);
 boost::filesystem::path make_tmp_dir_next_to(const boost::filesystem::path& target_dir, const char* suffix);
 
 }
diff --git a/test/limestone/utils/dblogutil_compaction_test.cpp b/test/limestone/utils/dblogutil_compaction_test.cpp
@@ -171,4 +171,28 @@ TEST_F(dblogutil_compaction_test, nondblogdir) {
     EXPECT_TRUE(contains(out, "unsupport"));
 }
 
+TEST_F(dblogutil_compaction_test, rejects_symlink) {
+    // setup normal logdir as same as case1
+    boost::filesystem::path dir{location};
+    dir /= "log";
+    boost::filesystem::create_directory(dir);
+    create_file(dir / "epoch", data_case1_epoch);
+    create_file(dir / std::string(manifest_file_name), data_manifest());
+    create_file(dir / "pwal_0000", data_case1_pwal0);
+    create_file(dir / "pwal_0001", data_case1_pwal1);
+
+    // make symlink to data
+    boost::filesystem::path symlink{location};
+    symlink /= "symbolic_link";
+    boost::filesystem::create_directory_symlink(dir, symlink);
+
+    std::string command;
+    command = UTIL_COMMAND " compaction --force " + symlink.string() + " 2>&1";
+    std::string out;
+    int rc = invoke(command, out);
+    EXPECT_GE(rc, 64 << 8);
+    EXPECT_TRUE(contains_line_starts_with(out, "E"));  // LOG(ERROR)
+    EXPECT_TRUE(contains(out, "must not be symlink"));
+}
+
 }  // namespace limestone::testing
diff --git a/test/limestone/utils/make_tmp_test.cpp b/test/limestone/utils/make_tmp_test.cpp
@@ -32,6 +32,23 @@ static constexpr const char* location = "/tmp/make_tmp_test";
 
 };
 
+TEST_F(make_tmp_test, remove_trailing_dir_separators) {
+    boost::filesystem::path p0{"/tmp/nonexist/0"};
+    remove_trailing_dir_separators(p0);
+    ASSERT_EQ(p0.string(), "/tmp/nonexist/0");
+    ASSERT_EQ(p0.filename().string(), "0");
+
+    boost::filesystem::path p1{"/tmp/nonexist/1/"};
+    remove_trailing_dir_separators(p1);
+    ASSERT_EQ(p1.string(), "/tmp/nonexist/1");
+    ASSERT_EQ(p1.filename().string(), "1");
+
+    boost::filesystem::path p2{"/tmp/nonexist/2//"};
+    remove_trailing_dir_separators(p2);
+    ASSERT_EQ(p2.string(), "/tmp/nonexist/2");
+    ASSERT_EQ(p2.filename().string(), "2");
+}
+
 TEST_F(make_tmp_test, make_tmp_dir_next_to_0slash) {
     std::string p = std::string(location) + "/test0";
     boost::filesystem::create_directory(p);