fix block cloning copy argument validation (#560)

5 files changed, 41 insertions, 4 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1615f8ab3..68c1fdba6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 - Bugfix: Fix NameEvent test to avoid race condition
 - Bugfix: Fix BlockingQueue asserts
 - Bugfix: Catch exceptions in WorkerThreadPool when running single-threaded
+- Bugfix: Improved block cloning copy argument validation, to properly catching the case where source or target trees overlap
 - Feature: Adding a file named `root_manifest.ignore_schema_mismatch` in the root of zenserver data dir prevents wipe of data when schema mismatches
 - Feature: Added `zen run` command which can be used to run a stress test or benchmark repeatedly while redirecting output and other state to separate subdirectories
   - Example usage: `zen run -n 10 -- zenserver-test` will run the `zenserver-test` command 10 times
diff --git a/src/zen/cmds/copy_cmd.cpp b/src/zen/cmds/copy_cmd.cpp
index d68a99616..956d9c9d2 100644
--- a/src/zen/cmds/copy_cmd.cpp
+++ b/src/zen/cmds/copy_cmd.cpp
@@ -14,6 +14,9 @@ CopyCommand::CopyCommand()
 {
 	m_Options.add_options()("h,help", "Print help");
 	m_Options.add_options()("no-clone", "Do not perform block clone", cxxopts::value(m_NoClone)->default_value("false"));
+	m_Options.add_options()("must-clone",
+							"Always perform block clone (fails if clone is not possible)",
+							cxxopts::value(m_MustClone)->default_value("false"));
 	m_Options.add_option("", "s", "source", "Copy source", cxxopts::value(m_CopySource), "<file/directory>");
 	m_Options.add_option("", "t", "target", "Copy target", cxxopts::value(m_CopyTarget), "<file/directory>");
 	m_Options.parse_positional({"source", "target"});
@@ -96,8 +99,8 @@ CopyCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 
 		if (!Ec)
 		{
-			if (!std::filesystem::relative(ToCanonical, FromCanonical).empty() ||
-				!std::filesystem::relative(FromCanonical, ToCanonical).empty())
+			if (ToCanonical.generic_string().starts_with(FromCanonical.generic_string()) ||
+				FromCanonical.generic_string().starts_with(ToCanonical.generic_string()))
 			{
 				throw std::runtime_error("Invalid parent/child relationship for source/target directories");
 			}
@@ -166,6 +169,7 @@ CopyCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 
 		zen::CopyFileOptions CopyOptions;
 		CopyOptions.EnableClone = !m_NoClone;
+		CopyOptions.MustClone	= m_MustClone;
 
 		CopyVisitor Visitor{FromPath, CopyOptions};
 		Visitor.TargetPath = ToPath;
diff --git a/src/zen/cmds/copy_cmd.h b/src/zen/cmds/copy_cmd.h
index 0dbebddfa..876aff3f5 100644
--- a/src/zen/cmds/copy_cmd.h
+++ b/src/zen/cmds/copy_cmd.h
@@ -19,10 +19,11 @@ public:
 	virtual ZenCmdCategory&	  CommandCategory() const override { return g_UtilitiesCategory; }
 
 private:
-	cxxopts::Options m_Options{"copy", "Copy files"};
+	cxxopts::Options m_Options{"copy", "Copy files efficiently"};
 	std::string		 m_CopySource;
 	std::string		 m_CopyTarget;
-	bool			 m_NoClone = false;
+	bool			 m_NoClone	 = false;
+	bool			 m_MustClone = false;
 };
 
 }  // namespace zen
diff --git a/src/zencore/filesystem.cpp b/src/zencore/filesystem.cpp
index f117001b5..85f7690bd 100644
--- a/src/zencore/filesystem.cpp
+++ b/src/zencore/filesystem.cpp
@@ -624,6 +624,9 @@ CopyTree(std::filesystem::path FromPath, std::filesystem::path ToPath, const Cop
 	if (ToPath.empty())
 		throw std::runtime_error("no CopyTree target specified");
 
+	if (Options.MustClone && !SupportsBlockRefCounting(FromPath))
+		throw std::runtime_error(fmt::format("cloning not possible from '{}'", FromPath));
+
 	if (std::filesystem::exists(ToPath))
 	{
 		if (!std::filesystem::is_directory(ToPath))
@@ -636,6 +639,33 @@ CopyTree(std::filesystem::path FromPath, std::filesystem::path ToPath, const Cop
 		std::filesystem::create_directories(ToPath);
 	}
 
+	if (Options.MustClone && !SupportsBlockRefCounting(ToPath))
+		throw std::runtime_error(fmt::format("cloning not possible from '{}'", ToPath));
+
+	// Verify source/target relationships
+
+	std::error_code		  Ec;
+	std::filesystem::path FromCanonical = std::filesystem::canonical(FromPath, Ec);
+
+	if (!Ec)
+	{
+		std::filesystem::path ToCanonical = std::filesystem::canonical(ToPath, Ec);
+
+		if (!Ec)
+		{
+			if (FromCanonical == ToCanonical)
+			{
+				throw std::runtime_error("Target and source must be distinct files or directories");
+			}
+
+			if (ToCanonical.generic_string().starts_with(FromCanonical.generic_string()) ||
+				FromCanonical.generic_string().starts_with(ToCanonical.generic_string()))
+			{
+				throw std::runtime_error("Invalid parent/child relationship for source/target directories");
+			}
+		}
+	}
+
 	struct CopyVisitor : public FileSystemTraversal::TreeVisitor
 	{
 		CopyVisitor(std::filesystem::path InBasePath, zen::CopyFileOptions InCopyOptions) : BasePath(InBasePath), CopyOptions(InCopyOptions)
diff --git a/src/zenserver/main.cpp b/src/zenserver/main.cpp
index 1aa0aa4df..ff4df183e 100644
--- a/src/zenserver/main.cpp
+++ b/src/zenserver/main.cpp
@@ -349,6 +349,7 @@ main(int argc, char* argv[])
 
 		if (!ServerOptions.BaseSnapshotDir.empty())
 		{
+			ZEN_CONSOLE_INFO("copying snapshot from '{}' into '{}", ServerOptions.BaseSnapshotDir, ServerOptions.DataDir);
 			CopyTree(ServerOptions.BaseSnapshotDir, ServerOptions.DataDir, {.EnableClone = true});
 		}