use write and move in place for safer writing of files (#70)

* use write and move in place for safer writing of files

10 files changed, 36 insertions, 24 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4dedc38d9..deb95a381 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 - Bugfix: Don't capture ChunkIndex variable in CasImpl::IterateChunks by reference as it causes crash
 - Improvement: Make FileCasStrategy::IterateChunks (optionally) multithreaded (improves GetProjectFiles performance)
 - Improvement: Add batch scope for adding multiple cache values from single request efficently
+- Improvement: Use temp file write and move into place for manifest/state files to avoid partial incomplete file writes
 
 ## 5.5.0
 - Change: GCv2 is now the default option, use `--gc-v2=false` to fall back to GCv1
diff --git a/src/zenhttp/auth/authmgr.cpp b/src/zenhttp/auth/authmgr.cpp
index a520e8fd1..bf151ce6d 100644
--- a/src/zenhttp/auth/authmgr.cpp
+++ b/src/zenhttp/auth/authmgr.cpp
@@ -9,6 +9,7 @@
 #include <zencore/filesystem.h>
 #include <zencore/logging.h>
 #include <zenhttp/auth/oidc.h>
+#include <zenutil/basicfile.h>
 
 #include <condition_variable>
 #include <memory>
@@ -76,7 +77,7 @@ namespace details {
 			return;
 		}
 
-		WriteFile(Path, IoBuffer(IoBuffer::Wrap, EncryptedView.GetData(), EncryptedView.GetSize()));
+		TemporaryFile::SafeWriteFile(Path, EncryptedView);
 	}
 }  // namespace details
 
diff --git a/src/zenserver/objectstore/objectstore.cpp b/src/zenserver/objectstore/objectstore.cpp
index 846a228ce..e614b256b 100644
--- a/src/zenserver/objectstore/objectstore.cpp
+++ b/src/zenserver/objectstore/objectstore.cpp
@@ -12,6 +12,7 @@
 #include "zencore/compactbinarybuilder.h"
 #include "zenhttp/httpcommon.h"
 #include "zenhttp/httpserver.h"
+#include "zenutil/basicfile.h"
 
 #include <filesystem>
 #include <thread>
@@ -589,7 +590,8 @@ HttpObjectStoreService::PutObject(zen::HttpRouterRequest& Request)
 			return Request.ServerRequest().WriteResponse(HttpResponseCode::BadRequest);
 		}
 
-		WriteFile(FilePath, FileBuf);
+		TemporaryFile::SafeWriteFile(FilePath, FileBuf.GetView());
+
 		ZEN_LOG_DEBUG(LogObj,
 					  "PUT - '{}' [OK] ({})",
 					  (fs::path(BucketName) / RelativeBucketPath).make_preferred(),
diff --git a/src/zenserver/projectstore/projectstore.cpp b/src/zenserver/projectstore/projectstore.cpp
index afb2c100c..f5fecce24 100644
--- a/src/zenserver/projectstore/projectstore.cpp
+++ b/src/zenserver/projectstore/projectstore.cpp
@@ -818,7 +818,7 @@ ProjectStore::Oplog::Write()
 
 	ZEN_INFO("persisting config for oplog '{}' in project '{}' to {}", m_OplogId, m_OuterProject->Identifier, StateFilePath);
 
-	WriteFile(StateFilePath, IoBuffer(IoBuffer::Wrap, Mem.GetData(), Mem.GetSize()));
+	TemporaryFile::SafeWriteFile(StateFilePath, Mem.GetView());
 }
 
 void
@@ -1732,7 +1732,7 @@ ProjectStore::Project::Write()
 
 	ZEN_INFO("persisting config for project '{}' to {}", Identifier, ProjectStateFilePath);
 
-	WriteFile(ProjectStateFilePath, IoBuffer(IoBuffer::Wrap, Mem.GetData(), Mem.GetSize()));
+	TemporaryFile::SafeWriteFile(ProjectStateFilePath, Mem.GetView());
 }
 
 void
@@ -1832,7 +1832,7 @@ ProjectStore::Project::WriteAccessTimes()
 
 		ZEN_INFO("persisting access times for project '{}' to {}", Identifier, ProjectAccessTimesFilePath);
 
-		WriteFile(ProjectAccessTimesFilePath, Data.GetBuffer().AsIoBuffer());
+		TemporaryFile::SafeWriteFile(ProjectAccessTimesFilePath, Data.GetView());
 	}
 	catch (const std::exception& Err)
 	{
diff --git a/src/zenserver/zenserver.cpp b/src/zenserver/zenserver.cpp
index 3fd5d53c7..89f376709 100644
--- a/src/zenserver/zenserver.cpp
+++ b/src/zenserver/zenserver.cpp
@@ -448,9 +448,7 @@ ZenServer::InitializeState(const ZenServerOptions& ServerOptions)
 
 		if (UpdateManifest)
 		{
-			IoBuffer ManifestBuffer = m_RootManifest.GetBuffer().AsIoBuffer();
-
-			WriteFile(ManifestPath, ManifestBuffer);
+			TemporaryFile::SafeWriteFile(ManifestPath, m_RootManifest.GetBuffer().GetView());
 		}
 
 		if (!ServerOptions.IsTest)
diff --git a/src/zenstore/cache/cachedisklayer.cpp b/src/zenstore/cache/cachedisklayer.cpp
index a497c8969..6cb749b5a 100644
--- a/src/zenstore/cache/cachedisklayer.cpp
+++ b/src/zenstore/cache/cachedisklayer.cpp
@@ -398,7 +398,7 @@ BucketManifestSerializer::GenerateNewManifest(std::filesystem::path ManifestPath
 	Writer << "BucketId"sv << BucketId;
 	Writer << "Version"sv << CurrentDiskBucketVersion;
 	Manifest = Writer.Save();
-	WriteFile(ManifestPath, Manifest.GetBuffer().AsIoBuffer());
+	TemporaryFile::SafeWriteFile(ManifestPath, Manifest.GetBuffer().GetView());
 
 	return BucketId;
 }
@@ -1647,7 +1647,7 @@ ZenCacheDiskLayer::CacheBucket::SaveSnapshot(const std::function<uint64_t()>& Cl
 		}
 
 		std::filesystem::path ManifestPath = GetManifestPath(m_BucketDir, m_BucketName);
-		WriteFile(ManifestPath, Buffer);
+		TemporaryFile::SafeWriteFile(ManifestPath, Buffer.GetView());
 	}
 	catch (const std::exception& Err)
 	{
diff --git a/src/zenstore/cas.cpp b/src/zenstore/cas.cpp
index 67790e2c6..f300c08e3 100644
--- a/src/zenstore/cas.cpp
+++ b/src/zenstore/cas.cpp
@@ -220,19 +220,7 @@ CasImpl::UpdateManifest()
 
 	ZEN_TRACE("Writing new manifest to '{}'", ManifestPath);
 
-	TemporaryFile	Marker;
-	std::error_code Ec;
-	Marker.CreateTemporary(ManifestPath.parent_path(), Ec);
-	if (Ec)
-	{
-		throw std::system_error(Ec, fmt::format("Failed to create temp file for cas manifest at '{}'", ManifestPath));
-	}
-	Marker.Write(m_ManifestObject.GetBuffer(), 0);
-	Marker.MoveTemporaryIntoPlace(ManifestPath, Ec);
-	if (Ec)
-	{
-		throw std::system_error(Ec, fmt::format("Failed to move temp file '{}' to '{}'", Marker.GetPath(), ManifestPath));
-	}
+	TemporaryFile::SafeWriteFile(ManifestPath, m_ManifestObject.GetBuffer().GetView());
 }
 
 CasStore::InsertResult
diff --git a/src/zenstore/gc.cpp b/src/zenstore/gc.cpp
index 39a747dae..e8cf6ec5e 100644
--- a/src/zenstore/gc.cpp
+++ b/src/zenstore/gc.cpp
@@ -169,7 +169,7 @@ LoadCompactBinaryObject(const fs::path& Path)
 void
 SaveCompactBinaryObject(const fs::path& Path, const CbObject& Object)
 {
-	WriteFile(Path, Object.GetBuffer().AsIoBuffer());
+	TemporaryFile::SafeWriteFile(Path, Object.GetBuffer().GetView());
 }
 
 //////////////////////////////////////////////////////////////////////////
diff --git a/src/zenutil/basicfile.cpp b/src/zenutil/basicfile.cpp
index f553fe5a0..d837c2caf 100644
--- a/src/zenutil/basicfile.cpp
+++ b/src/zenutil/basicfile.cpp
@@ -560,6 +560,26 @@ TemporaryFile::MoveTemporaryIntoPlace(std::filesystem::path FinalFileName, std::
 
 //////////////////////////////////////////////////////////////////////////
 
+void
+TemporaryFile::SafeWriteFile(std::filesystem::path Path, MemoryView Data)
+{
+	TemporaryFile	TempFile;
+	std::error_code Ec;
+	TempFile.CreateTemporary(Path.parent_path(), Ec);
+	if (Ec)
+	{
+		throw std::system_error(Ec, fmt::format("Failed to create temp file for file at '{}'", Path));
+	}
+	TempFile.Write(Data, 0);
+	TempFile.MoveTemporaryIntoPlace(Path, Ec);
+	if (Ec)
+	{
+		throw std::system_error(Ec, fmt::format("Failed to move temp file '{}' to '{}'", TempFile.GetPath(), Path));
+	}
+}
+
+//////////////////////////////////////////////////////////////////////////
+
 LockFile::LockFile()
 {
 }
diff --git a/src/zenutil/include/zenutil/basicfile.h b/src/zenutil/include/zenutil/basicfile.h
index 0e4295ee3..375da20c3 100644
--- a/src/zenutil/include/zenutil/basicfile.h
+++ b/src/zenutil/include/zenutil/basicfile.h
@@ -101,6 +101,8 @@ public:
 	void						 MoveTemporaryIntoPlace(std::filesystem::path FinalFileName, std::error_code& Ec);
 	const std::filesystem::path& GetPath() const { return m_TempPath; }
 
+	static void SafeWriteFile(std::filesystem::path Path, MemoryView Data);
+
 private:
 	std::filesystem::path m_TempPath;