Merge pull request #313 from crymp-net/use-temp-files-for-download

Use temporary unique filenames during download

Author: ccomrade

CMakeLists.txt

@@ -626,6 +626,8 @@ add_library(Base OBJECT
 	Code/Library/PathTools.h
 	Code/Library/PathTree.h
 	Code/Library/SlotVector.h
+	Code/Library/StdFile.cpp
+	Code/Library/StdFile.h
 	Code/Library/StlportMap.h
 	Code/Library/StlportSet.h
 	Code/Library/StlportVector.h

Code/CryMP/Client/Client.cpp

@@ -15,6 +15,7 @@
 #include "CrySystem/Logger.h"
 #include "CrySystem/RandomGenerator.h"
 #include "Launcher/Resources.h"
+#include "Library/StdFile.h"
 #include "Library/StringTools.h"
 #include "Library/Util.h"
 #include "Library/WinAPI.h"
@@ -40,19 +41,11 @@ void Client::InitMasters()
 {
 	std::string content;
 
-	WinAPI::File file("masters.txt", WinAPI::FileAccess::READ_ONLY);  // Crysis main directory
-	if (file)
+	if (StdFile file("masters.txt", "r"); file.IsOpen())  // Crysis main directory
 	{
 		CryLogAlways("$6[CryMP] Using local masters.txt as the master server list provider");
 
-		try
-		{
-			content = file.Read();
-		}
-		catch (const std::exception& ex)
-		{
-			CryLogAlways("$4[CryMP] Failed to read the masters.txt file: %s", ex.what());
-		}
+		content = file.ReadAll();
 	}
 	else
 	{

Code/CryMP/Client/FileCache.cpp

@@ -2,9 +2,8 @@
 
 #include "CryCommon/CrySystem/ISystem.h"
 #include "CryCommon/CrySystem/ICryPak.h"
-#include "Library/StringTools.h"
+#include "Library/StdFile.h"
 #include "Library/Util.h"
-#include "Library/WinAPI.h"
 
 #include "FileCache.h"
 #include "Client.h"
@@ -14,58 +13,43 @@ json FileCache::LoadIndex()
 {
 	json index;
 
-	try
+	if (StdFile file((m_cacheDir / "index").string().c_str(), "r"); file.IsOpen())
 	{
-		WinAPI::File indexFile(m_cacheDir / "index", WinAPI::FileAccess::READ_ONLY);
-		if (!indexFile)
-		{
-			throw StringTools::SysErrorFormat("Failed to open the index file for reading");
-		}
-
-		const std::string content = indexFile.Read();
-
-		if (!content.empty())
-		{
-			index = json::parse(content);
-		}
-	}
-	catch (const std::exception & ex)
-	{
-		CryLogAlways("$4[CryMP] [FileCache] Index load error: %s", ex.what());
+		index = json::parse(file.GetHandle(), nullptr, false);  // no exceptions
 	}
 
 	if (!index.contains("files") || !index["files"].is_object())
+	{
 		index["files"] = json::object();
+	}
 
 	if (!index.contains("lru") || !index["lru"].is_array())
+	{
 		index["lru"] = json::array();
+	}
 
 	return index;
 }
 
 void FileCache::SaveIndex(const json & index)
 {
-	try
-	{
-		WinAPI::File indexFile(m_cacheDir / "index", WinAPI::FileAccess::WRITE_ONLY);
-		if (!indexFile)
-		{
-			throw StringTools::SysErrorFormat("Failed to open the index file for writing");
-		}
+	std::string content = index.dump(-1, ' ', false, json::error_handler_t::replace);  // no exceptions
 
-		indexFile.Write(index.dump());
-	}
-	catch (const std::exception & ex)
+	StdFile file((m_cacheDir / "index").string().c_str(), "w");
+	if (!file.IsOpen())
 	{
-		CryLogAlways("$4[CryMP] [FileCache] Index save error: %s", ex.what());
+		CryLogAlways("$4[CryMP] [FileCache] Failed to open index file for writing");
+		return;
 	}
+
+	file.Write(content.c_str(), content.length());
 }
 
 void FileCache::DownloadFile(FileCacheRequest && request, const std::filesystem::path & filePath)
 {
 	FileDownloaderRequest download;
 	download.url = request.fileURL;
-	download.filePath = filePath;
+	download.filePath = FileDownloaderRequest::MakeFileNameUnique(filePath); // temporary file during download
 
 	CryLogAlways("$3[CryMP] [FileCache] Downloading from $6%s$3", download.url.c_str());
 
@@ -77,7 +61,7 @@ void FileCache::DownloadFile(FileCacheRequest && request, const std::filesystem:
 			return true;
 	};
 
-	download.onComplete = [request = std::move(request), this](FileDownloaderResult & result)
+	download.onComplete = [request = std::move(request), finalPath = filePath, this](FileDownloaderResult & result)
 	{
 		bool success = false;
 
@@ -111,12 +95,27 @@ void FileCache::DownloadFile(FileCacheRequest && request, const std::filesystem:
 			success = true;
 		}
 
+		if (success)
+		{
+			// rename the temporary file
+			// no exceptions
+			std::error_code error;
+			std::filesystem::rename(result.filePath, finalPath, error);
+			if (error)
+			{
+				CryLogAlways("$4[CryMP] [FileCache] Renaming temporary file failed: Error %d (%s)",
+					error.value(), error.message().c_str());
+
+				success = false;
+			}
+		}
+
 		if (!success)
 		{
 			RemoveFile(result.filePath);
 		}
 
-		CompleteRequest(request, success, result.filePath);
+		CompleteRequest(request, success, finalPath);
 	};
 
 	gClient->GetFileDownloader()->Request(std::move(download));
@@ -149,9 +148,6 @@ FileCache::FileCache()
 
 	// make sure the cache directory exists
 	std::filesystem::create_directories(m_cacheDir);
-
-	// make sure the index file exists
-	WinAPI::File(m_cacheDir / "index", WinAPI::FileAccess::READ_WRITE_CREATE);
 }
 
 FileCache::~FileCache()

Code/CryMP/Client/FileDownloader.cpp

@@ -2,6 +2,8 @@
 #include <chrono>
 
 #include "CryMP/Common/Executor.h"
+#include "CrySystem/RandomGenerator.h"
+#include "Library/StdFile.h"
 #include "Library/StringTools.h"
 #include "Library/Util.h"
 #include "Library/WinAPI.h"
@@ -10,33 +12,6 @@
 #include "SpeedAggregator.h"
 #include "Client.h"
 
-class OutputFile
-{
-	WinAPI::File m_file;
-
-public:
-	OutputFile(const std::filesystem::path & path)
-	{
-		bool created = false;
-
-		if (!m_file.Open(path, WinAPI::FileAccess::WRITE_ONLY_CREATE, &created))
-		{
-			throw StringTools::SysErrorFormat("Failed to open the output file");
-		}
-
-		if (!created)
-		{
-			// clear the existing file
-			m_file.Resize(0);
-		}
-	}
-
-	void Write(const char *chunk, size_t chunkLength)
-	{
-		m_file.Write(std::string_view(chunk, chunkLength));
-	}
-};
-
 struct FileDownloaderTask : public IExecutorTask
 {
 	FileDownloaderRequest request;
@@ -77,25 +52,30 @@ struct FileDownloaderTask : public IExecutorTask
 
 		try
 		{
-			OutputFile file(request.filePath);
-
 			result.statusCode = WinAPI::HTTPRequest(
 				"GET",
 				request.url,
 				{},  // data
 				{},  // headers
-				[this, &file](uint64_t contentLength, const WinAPI::HTTPRequestReader & reader)
+				[this](uint64_t contentLength, const WinAPI::HTTPRequestReader& reader)
 				{
+					StdFile file(request.filePath.string().c_str(), "wb");
+					if (!file.IsOpen())
+					{
+						throw StringTools::SysErrorErrnoFormat("Output file open failed");
+					}
+
 					// content length is zero if not provided by the server
 					result.contentLength = contentLength;
 
 					while (isActive)
 					{
 						char chunk[8192];
 						const size_t chunkLength = reader(chunk, sizeof chunk);
-
 						if (chunkLength == 0)
+						{
 							break;
+						}
 
 						file.Write(chunk, chunkLength);
 						result.downloadedBytes += chunkLength;
@@ -142,6 +122,38 @@ std::string FileDownloaderProgress::ToString() const
 	}
 }
 
+std::string FileDownloaderRequest::CreateUniqueFileName(std::string_view baseName, std::string_view extension)
+{
+	std::string name(baseName);
+
+	// lowercase
+	StringTools::ToLowerInPlace(name);
+
+	// sanitize
+	for (char& ch : name)
+	{
+		if (!(ch >= '0' && ch <= '9') && !(ch >= 'a' && ch <= 'z'))
+		{
+			ch = '_';
+		}
+	}
+
+	// add a random suffix for uniqueness
+	StringTools::FormatTo(name, "_%09u", RandomGenerator::GenerateUInt32(0, 999'999'999));
+
+	// extension
+	name += extension;
+
+	return name;
+}
+
+std::filesystem::path FileDownloaderRequest::MakeFileNameUnique(const std::filesystem::path& path)
+{
+	std::filesystem::path newPath(path);
+	newPath.replace_filename(CreateUniqueFileName(path.filename().string(), path.extension().string()));
+	return newPath;
+}
+
 FileDownloader::FileDownloader()
 {
 }

Code/CryMP/Client/FileDownloader.h

@@ -2,6 +2,7 @@
 
 #include <stdint.h>
 #include <string>
+#include <string_view>
 #include <filesystem>
 #include <functional>
 
@@ -32,6 +33,9 @@ struct FileDownloaderRequest
 	std::filesystem::path filePath;
 	std::function<bool(FileDownloaderProgress&)> onProgress;  // return false to cancel download
 	std::function<void(FileDownloaderResult&)> onComplete;
+
+	static std::string CreateUniqueFileName(std::string_view baseName, std::string_view extension);
+	static std::filesystem::path MakeFileNameUnique(const std::filesystem::path& path);
 };
 
 class FileDownloader

Code/CryMP/Client/MapDownloader.cpp

@@ -3,8 +3,8 @@
 #include "CryCommon/CryAction/IGameFramework.h"
 #include "CryCommon/CryAction/ILevelSystem.h"
 #include "CrySystem/CryPak.h"
+#include "Library/StdFile.h"
 #include "Library/Util.h"
-#include "Library/WinAPI.h"
 
 #include "MapDownloader.h"
 #include "MapExtractor.h"
@@ -13,12 +13,9 @@
 
 void MapDownloader::DownloadMap(MapDownloaderRequest && request)
 {
-	// convert "multiplayer/ps/mymap" to "mymap.zip"
-	const std::string zipFileName = std::filesystem::path(request.map).filename().string() + ".zip";
-
 	FileDownloaderRequest download;
 	download.url = request.mapURL;
-	download.filePath = m_downloadDir / zipFileName;
+	download.filePath = m_downloadDir / FileDownloaderRequest::CreateUniqueFileName(request.map, ".zip");
 
 	CryLogAlways("$3[CryMP] [MapDownloader] Downloading map $6%s$3", request.map.c_str());
 
@@ -152,41 +149,25 @@ bool MapDownloader::CheckMapExists(const std::string_view & mapName)
 
 bool MapDownloader::CheckMapVersion(const std::string_view & mapName, const std::string_view & mapVersion)
 {
-	if (mapVersion.empty())
+	StdFile file(GetVersionFilePath(mapName).string().c_str(), "rb");
+	if (!file.IsOpen())
 	{
-		return true;
+		return mapVersion.empty();
 	}
 
-	const std::filesystem::path versionFilePath = GetVersionFilePath(mapName);
-
-	try
-	{
-		WinAPI::File file(versionFilePath, WinAPI::FileAccess::READ_ONLY);
-
-		return file.IsOpen() && file.Read() == mapVersion;
-	}
-	catch (const CryMP_Error& error)
-	{
-		CryLogAlways("$4[CryMP] [MapDownloader] Failed to read map version: %s", error.what());
-		return false;
-	}
+	return file.ReadAll() == mapVersion;
 }
 
 void MapDownloader::StoreMapVersion(const std::string_view & mapName, const std::string_view & mapVersion)
 {
-	const std::filesystem::path versionFilePath = GetVersionFilePath(mapName);
-
-	try
-	{
-		WinAPI::File file(versionFilePath, WinAPI::FileAccess::WRITE_ONLY_CREATE);
-
-		file.Resize(0);
-		file.Write(mapVersion);
-	}
-	catch (const CryMP_Error& error)
+	StdFile file(GetVersionFilePath(mapName).string().c_str(), "wb");
+	if (!file.IsOpen())
 	{
-		CryLogAlways("$4[CryMP] [MapDownloader] Failed to store map version: %s", error.what());
+		CryLogAlways("$4[CryMP] [MapDownloader] Failed to open map version file for writing");
+		return;
 	}
+
+	file.Write(mapVersion.data(), mapVersion.length());
 }
 
 std::filesystem::path MapDownloader::GetVersionFilePath(const std::string_view & mapName)