add format version check before restore (old method)

project-tsurugi · Nov 30, 2023 · e9addd1 · e9addd1
1 parent cef5f33
commit e9addd1
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 8 deletions.
diff --git a/src/limestone/datastore_format.cpp b/src/limestone/datastore_format.cpp
@@ -61,18 +61,37 @@ void setup_initial_logdir(const boost::filesystem::path& logdir) {
     }
 }
 
-void check_logdir_format(const boost::filesystem::path& logdir) {
-    boost::filesystem::path manifest_path = logdir / std::string(manifest_file_name);
+bool is_supported_version(const boost::filesystem::path& manifest_path, std::string& errmsg) {
     std::ifstream istrm(manifest_path.string());
     if (!istrm) {
+        errmsg = "cannot open for read " + manifest_path.string();
+        return false;
+    }
+    nlohmann::json manifest;
+    try {
+        istrm >> manifest;
+        auto version = manifest["persistent_format_version"];
+        if (!version.is_number_integer() || version != 1) {
+            errmsg = "format version mismatch: version = " + version.dump();
+            return false;
+        }
+    } catch (nlohmann::json::exception& e) {
+        errmsg = "JSON read error: ";
+        errmsg.append(e.what());
+        return false;
+    };
+    return true;
+}
+
+void check_logdir_format(const boost::filesystem::path& logdir) {
+    boost::filesystem::path manifest_path = logdir / std::string(manifest_file_name);
+    if (!boost::filesystem::exists(manifest_path)) {
         LOG_LP(ERROR) << "no config file in logdir, maybe v0";
         throw std::runtime_error("logdir version mismatch");
     }
-    nlohmann::json manifest;
-    istrm >> manifest;
-    auto version = manifest["persistent_format_version"];
-    if (!version.is_number_integer() || version != 1) {
-        LOG_LP(ERROR) << "logdir version mismatch: version = " << version;
+    std::string errmsg;
+    if (!is_supported_version(manifest_path, errmsg)) {
+        LOG_LP(ERROR) << errmsg;
         throw std::runtime_error("logdir version mismatch");
     }
 }

diff --git a/src/limestone/datastore_restore.cpp b/src/limestone/datastore_restore.cpp
@@ -22,11 +22,21 @@
 
 #include <limestone/api/datastore.h>
 #include <limestone/status.h>
+#include "internal.h"
 
 namespace limestone::api {
 
 status datastore::restore(std::string_view from, bool keep_backup) const noexcept {
     VLOG_LP(log_debug) << "restore begin, from directory = " << from << " , keep_backup = " << std::boolalpha << keep_backup;
+    auto from_dir = boost::filesystem::path(std::string(from));
+
+    // log_dir version check
+    boost::filesystem::path manifest_path = from_dir / std::string(internal::manifest_file_name);
+    std::string ver_err;
+    if (!internal::is_supported_version(manifest_path, ver_err)) {  // notfound or invalid or unsupport
+        LOG_LP(ERROR) << ver_err;
+        return status::err_broken_data;
+    }
 
     BOOST_FOREACH(const boost::filesystem::path& p, std::make_pair(boost::filesystem::directory_iterator(location_), boost::filesystem::directory_iterator())) {
         if(!boost::filesystem::is_directory(p)) {
@@ -39,7 +49,6 @@ status datastore::restore(std::string_view from, bool keep_backup) const noexcep
         }
     }
 
-    auto from_dir = boost::filesystem::path(std::string(from));
     BOOST_FOREACH(const boost::filesystem::path& p, std::make_pair(boost::filesystem::directory_iterator(from_dir), boost::filesystem::directory_iterator())) {
         try {
             boost::filesystem::copy_file(p, location_ / p.filename());

diff --git a/src/limestone/internal.h b/src/limestone/internal.h
@@ -30,6 +30,8 @@ inline constexpr const std::string_view manifest_file_name = "limestone-manifest
 
 void setup_initial_logdir(const boost::filesystem::path& logdir);
 
+bool is_supported_version(const boost::filesystem::path& manifest_path, std::string& errmsg);
+
 void check_logdir_format(const boost::filesystem::path& logdir);
 
 }
diff --git a/test/limestone/log/log_dir_test.cpp b/test/limestone/log/log_dir_test.cpp
@@ -104,4 +104,47 @@ TEST_F(log_dir_test, reject_directory_of_different_version) {
     EXPECT_THROW({ limestone::internal::check_logdir_format(location); }, std::exception);
 }
 
+TEST_F(log_dir_test, rotate_old_ok_v1_dir) {
+    // setup backups
+    boost::filesystem::path bk_path = boost::filesystem::path(location) / "bk";
+    if (!boost::filesystem::create_directory(bk_path)) {
+        LOG(FATAL) << "cannot make directory";
+    }
+    create_file(bk_path / "epoch", "\x04\x00\x00\x00\x00\x00\x00\x00\x00");
+    create_file(bk_path / std::string(limestone::internal::manifest_file_name),
+                "{ \"format_version\": \"1.0\", \"persistent_format_version\": 1 }");
+
+    gen_datastore();
+
+    EXPECT_EQ(datastore_->restore(bk_path.string(), true), limestone::status::ok);
+}
+
+TEST_F(log_dir_test, rotate_old_rejects_unsupported_data) {
+    // setup backups
+    boost::filesystem::path bk_path = boost::filesystem::path(location) / "bk";
+    if (!boost::filesystem::create_directory(bk_path)) {
+        LOG(FATAL) << "cannot make directory";
+    }
+    create_file(bk_path / "epoch", "\x04\x00\x00\x00\x00\x00\x00\x00\x00");
+    create_file(bk_path / std::string(limestone::internal::manifest_file_name),
+                "{ \"format_version\": \"1.0\", \"persistent_format_version\": 2 }");
+
+    gen_datastore();
+
+    EXPECT_EQ(datastore_->restore(bk_path.string(), true), limestone::status::err_broken_data);
+}
+
+TEST_F(log_dir_test, rotate_old_rejects_v0_logdir_missing_manifest) {
+    // setup backups
+    boost::filesystem::path bk_path = boost::filesystem::path(location) / "bk";
+    if (!boost::filesystem::create_directory(bk_path)) {
+        LOG(FATAL) << "cannot make directory";
+    }
+    create_file(bk_path / "epoch", "\x04\x00\x00\x00\x00\x00\x00\x00\x00");
+
+    gen_datastore();
+
+    EXPECT_EQ(datastore_->restore(bk_path.string(), true), limestone::status::err_broken_data);
+}
+
 }  // namespace limestone::testing