From 447cace77f2533e616f82e42787c14f7fcbc039b Mon Sep 17 00:00:00 2001 From: Jack <66967891+ASpoonPlaysGames@users.noreply.github.com> Date: Sat, 20 Jan 2024 22:40:27 +0000 Subject: Add and use ScopeGuard (#643) Use a scope guard instead of `GOTO` statements for curl cleanup --- primedev/masterserver/masterserver.cpp | 114 ++++++++++++----------- primedev/mods/autodownload/moddownloader.cpp | 131 ++++++++++++++------------- primedev/util/utils.h | 20 ++++ 3 files changed, 147 insertions(+), 118 deletions(-) diff --git a/primedev/masterserver/masterserver.cpp b/primedev/masterserver/masterserver.cpp index aa248464..c4cc22ef 100644 --- a/primedev/masterserver/masterserver.cpp +++ b/primedev/masterserver/masterserver.cpp @@ -7,6 +7,7 @@ #include "engine/r2engine.h" #include "mods/modmanager.h" #include "shared/misccommands.h" +#include "util/utils.h" #include "util/version.h" #include "server/auth/bansystem.h" #include "dedicated/dedicated.h" @@ -118,6 +119,13 @@ void MasterServerManager::AuthenticateOriginWithMasterServer(const char* uid, co curl_easy_setopt(curl, CURLOPT_WRITEDATA, &readBuffer); CURLcode result = curl_easy_perform(curl); + ScopeGuard cleanup( + [&] + { + m_bOriginAuthWithMasterServerInProgress = false; + m_bOriginAuthWithMasterServerDone = true; + curl_easy_cleanup(curl); + }); if (result == CURLcode::CURLE_OK) { @@ -131,13 +139,13 @@ void MasterServerManager::AuthenticateOriginWithMasterServer(const char* uid, co spdlog::error( "Failed reading origin auth info response: encountered parse error \"{}\"", rapidjson::GetParseError_En(originAuthInfo.GetParseError())); - goto REQUEST_END_CLEANUP; + return; } if (!originAuthInfo.IsObject() || !originAuthInfo.HasMember("success")) { spdlog::error("Failed reading origin auth info response: malformed response object {}", readBuffer); - goto REQUEST_END_CLEANUP; + return; } if (originAuthInfo["success"].IsTrue() && originAuthInfo.HasMember("token") && originAuthInfo["token"].IsString()) @@ -174,12 +182,6 @@ void MasterServerManager::AuthenticateOriginWithMasterServer(const char* uid, co spdlog::error("Failed performing northstar origin auth: error {}", curl_easy_strerror(result)); m_bSuccessfullyConnected = false; } - - // we goto this instead of returning so we always hit this - REQUEST_END_CLEANUP: - m_bOriginAuthWithMasterServerInProgress = false; - m_bOriginAuthWithMasterServerDone = true; - curl_easy_cleanup(curl); }); requestThread.detach(); @@ -213,6 +215,13 @@ void MasterServerManager::RequestServerList() curl_easy_setopt(curl, CURLOPT_WRITEDATA, &readBuffer); CURLcode result = curl_easy_perform(curl); + ScopeGuard cleanup( + [&] + { + m_bRequestingServerList = false; + m_bScriptRequestingServerList = false; + curl_easy_cleanup(curl); + }); if (result == CURLcode::CURLE_OK) { @@ -226,20 +235,20 @@ void MasterServerManager::RequestServerList() spdlog::error( "Failed reading masterserver response: encountered parse error \"{}\"", rapidjson::GetParseError_En(serverInfoJson.GetParseError())); - goto REQUEST_END_CLEANUP; + return; } if (serverInfoJson.IsObject() && serverInfoJson.HasMember("error")) { spdlog::error("Failed reading masterserver response: got fastify error response"); spdlog::error(readBuffer); - goto REQUEST_END_CLEANUP; + return; } if (!serverInfoJson.IsArray()) { spdlog::error("Failed reading masterserver response: root object is not an array"); - goto REQUEST_END_CLEANUP; + return; } rapidjson::GenericArray serverArray = serverInfoJson.GetArray(); @@ -251,7 +260,7 @@ void MasterServerManager::RequestServerList() if (!serverObj.IsObject()) { spdlog::error("Failed reading masterserver response: member of server array is not an object"); - goto REQUEST_END_CLEANUP; + return; } // todo: verify json props are fine before adding to m_remoteServers @@ -342,12 +351,6 @@ void MasterServerManager::RequestServerList() spdlog::error("Failed requesting servers: error {}", curl_easy_strerror(result)); m_bSuccessfullyConnected = false; } - - // we goto this instead of returning so we always hit this - REQUEST_END_CLEANUP: - m_bRequestingServerList = false; - m_bScriptRequestingServerList = false; - curl_easy_cleanup(curl); }); requestThread.detach(); @@ -374,6 +377,7 @@ void MasterServerManager::RequestMainMenuPromos() curl_easy_setopt(curl, CURLOPT_WRITEDATA, &readBuffer); CURLcode result = curl_easy_perform(curl); + ScopeGuard cleanup([&] { curl_easy_cleanup(curl); }); if (result == CURLcode::CURLE_OK) { @@ -387,20 +391,20 @@ void MasterServerManager::RequestMainMenuPromos() spdlog::error( "Failed reading masterserver main menu promos response: encountered parse error \"{}\"", rapidjson::GetParseError_En(mainMenuPromoJson.GetParseError())); - goto REQUEST_END_CLEANUP; + return; } if (!mainMenuPromoJson.IsObject()) { spdlog::error("Failed reading masterserver main menu promos response: root object is not an object"); - goto REQUEST_END_CLEANUP; + return; } if (mainMenuPromoJson.HasMember("error")) { spdlog::error("Failed reading masterserver response: got fastify error response"); spdlog::error(readBuffer); - goto REQUEST_END_CLEANUP; + return; } if (!mainMenuPromoJson.HasMember("newInfo") || !mainMenuPromoJson["newInfo"].IsObject() || @@ -428,7 +432,7 @@ void MasterServerManager::RequestMainMenuPromos() !mainMenuPromoJson["smallButton2"]["ImageIndex"].IsNumber()) { spdlog::error("Failed reading masterserver main menu promos response: malformed json object"); - goto REQUEST_END_CLEANUP; + return; } m_sMainMenuPromoData.newInfoTitle1 = mainMenuPromoJson["newInfo"]["Title1"].GetString(); @@ -455,10 +459,6 @@ void MasterServerManager::RequestMainMenuPromos() spdlog::error("Failed requesting main menu promos: error {}", curl_easy_strerror(result)); m_bSuccessfullyConnected = false; } - - REQUEST_END_CLEANUP: - // nothing lol - curl_easy_cleanup(curl); }); requestThread.detach(); @@ -495,6 +495,21 @@ void MasterServerManager::AuthenticateWithOwnServer(const char* uid, const char* curl_easy_setopt(curl, CURLOPT_WRITEDATA, &readBuffer); CURLcode result = curl_easy_perform(curl); + ScopeGuard cleanup( + [&] + { + m_bAuthenticatingWithGameServer = false; + m_bScriptAuthenticatingWithGameServer = false; + + if (m_bNewgameAfterSelfAuth) + { + // pretty sure this is threadsafe? + Cbuf_AddText(Cbuf_GetCurrentPlayer(), "ns_end_reauth_and_leave_to_lobby", cmd_source_t::kCommandSrcCode); + m_bNewgameAfterSelfAuth = false; + } + + curl_easy_cleanup(curl); + }); if (result == CURLcode::CURLE_OK) { @@ -508,13 +523,13 @@ void MasterServerManager::AuthenticateWithOwnServer(const char* uid, const char* spdlog::error( "Failed reading masterserver authentication response: encountered parse error \"{}\"", rapidjson::GetParseError_En(authInfoJson.GetParseError())); - goto REQUEST_END_CLEANUP; + return; } if (!authInfoJson.IsObject()) { spdlog::error("Failed reading masterserver authentication response: root object is not an object"); - goto REQUEST_END_CLEANUP; + return; } if (authInfoJson.HasMember("error")) @@ -529,13 +544,13 @@ void MasterServerManager::AuthenticateWithOwnServer(const char* uid, const char* else m_sAuthFailureReason = "No error message provided"; - goto REQUEST_END_CLEANUP; + return; } if (!authInfoJson["success"].IsTrue()) { spdlog::error("Authentication with masterserver failed: \"success\" is not true"); - goto REQUEST_END_CLEANUP; + return; } if (!authInfoJson.HasMember("success") || !authInfoJson.HasMember("id") || !authInfoJson["id"].IsString() || @@ -543,7 +558,7 @@ void MasterServerManager::AuthenticateWithOwnServer(const char* uid, const char* !authInfoJson.HasMember("persistentData") || !authInfoJson["persistentData"].IsArray()) { spdlog::error("Failed reading masterserver authentication response: malformed json object"); - goto REQUEST_END_CLEANUP; + return; } RemoteAuthData newAuthData {}; @@ -561,7 +576,7 @@ void MasterServerManager::AuthenticateWithOwnServer(const char* uid, const char* if (!byte.IsUint() || byte.GetUint() > 255) { spdlog::error("Failed reading masterserver authentication response: malformed json object"); - goto REQUEST_END_CLEANUP; + return; } newAuthData.pdata[i++] = static_cast(byte.GetUint()); @@ -581,19 +596,6 @@ void MasterServerManager::AuthenticateWithOwnServer(const char* uid, const char* m_bSuccessfullyAuthenticatedWithGameServer = false; m_bScriptAuthenticatingWithGameServer = false; } - - REQUEST_END_CLEANUP: - m_bAuthenticatingWithGameServer = false; - m_bScriptAuthenticatingWithGameServer = false; - - if (m_bNewgameAfterSelfAuth) - { - // pretty sure this is threadsafe? - Cbuf_AddText(Cbuf_GetCurrentPlayer(), "ns_end_reauth_and_leave_to_lobby", cmd_source_t::kCommandSrcCode); - m_bNewgameAfterSelfAuth = false; - } - - curl_easy_cleanup(curl); }); requestThread.detach(); @@ -651,6 +653,13 @@ void MasterServerManager::AuthenticateWithServer(const char* uid, const char* pl } CURLcode result = curl_easy_perform(curl); + ScopeGuard cleanup( + [&] + { + m_bAuthenticatingWithGameServer = false; + m_bScriptAuthenticatingWithGameServer = false; + curl_easy_cleanup(curl); + }); if (result == CURLcode::CURLE_OK) { @@ -664,13 +673,13 @@ void MasterServerManager::AuthenticateWithServer(const char* uid, const char* pl spdlog::error( "Failed reading masterserver authentication response: encountered parse error \"{}\"", rapidjson::GetParseError_En(connectionInfoJson.GetParseError())); - goto REQUEST_END_CLEANUP; + return; } if (!connectionInfoJson.IsObject()) { spdlog::error("Failed reading masterserver authentication response: root object is not an object"); - goto REQUEST_END_CLEANUP; + return; } if (connectionInfoJson.HasMember("error")) @@ -685,13 +694,13 @@ void MasterServerManager::AuthenticateWithServer(const char* uid, const char* pl else m_sAuthFailureReason = "No error message provided"; - goto REQUEST_END_CLEANUP; + return; } if (!connectionInfoJson["success"].IsTrue()) { spdlog::error("Authentication with masterserver failed: \"success\" is not true"); - goto REQUEST_END_CLEANUP; + return; } if (!connectionInfoJson.HasMember("success") || !connectionInfoJson.HasMember("ip") || @@ -700,7 +709,7 @@ void MasterServerManager::AuthenticateWithServer(const char* uid, const char* pl !connectionInfoJson["authToken"].IsString()) { spdlog::error("Failed reading masterserver authentication response: malformed json object"); - goto REQUEST_END_CLEANUP; + return; } m_pendingConnectionInfo.ip.S_un.S_addr = inet_addr(connectionInfoJson["ip"].GetString()); @@ -725,11 +734,6 @@ void MasterServerManager::AuthenticateWithServer(const char* uid, const char* pl m_bSuccessfullyAuthenticatedWithGameServer = false; m_bScriptAuthenticatingWithGameServer = false; } - - REQUEST_END_CLEANUP: - m_bAuthenticatingWithGameServer = false; - m_bScriptAuthenticatingWithGameServer = false; - curl_easy_cleanup(curl); }); requestThread.detach(); diff --git a/primedev/mods/autodownload/moddownloader.cpp b/primedev/mods/autodownload/moddownloader.cpp index 165399e3..80be8d19 100644 --- a/primedev/mods/autodownload/moddownloader.cpp +++ b/primedev/mods/autodownload/moddownloader.cpp @@ -1,4 +1,5 @@ #include "moddownloader.h" +#include "util/utils.h" #include #include #include @@ -74,6 +75,7 @@ void ModDownloader::FetchModsListFromAPI() curl_easy_setopt(easyhandle, CURLOPT_WRITEDATA, &readBuffer); curl_easy_setopt(easyhandle, CURLOPT_WRITEFUNCTION, WriteToString); result = curl_easy_perform(easyhandle); + ScopeGuard cleanup([&] { curl_easy_cleanup(easyhandle); }); if (result == CURLcode::CURLE_OK) { @@ -82,7 +84,7 @@ void ModDownloader::FetchModsListFromAPI() else { spdlog::error("Fetching mods list failed: {}", curl_easy_strerror(result)); - goto REQUEST_END_CLEANUP; + return; } // Load mods list into local state @@ -110,9 +112,6 @@ void ModDownloader::FetchModsListFromAPI() } spdlog::info("Done loading verified mods list."); - - REQUEST_END_CLEANUP: - curl_easy_cleanup(easyhandle); }); requestThread.detach(); } @@ -170,23 +169,23 @@ std::optional ModDownloader::FetchModFromDistantStore(std::string_view curl_easy_setopt(easyhandle, CURLOPT_XFERINFOFUNCTION, ModDownloader::ModFetchingProgressCallback); curl_easy_setopt(easyhandle, CURLOPT_XFERINFODATA, this); result = curl_easy_perform(easyhandle); + ScopeGuard cleanup( + [&] + { + curl_easy_cleanup(easyhandle); + fclose(fp); + }); if (result == CURLcode::CURLE_OK) { spdlog::info("Mod archive successfully fetched."); - goto REQUEST_END_CLEANUP; + return std::optional(downloadPath); } else { spdlog::error("Fetching mod archive failed: {}", curl_easy_strerror(result)); - failed = true; - goto REQUEST_END_CLEANUP; + return std::optional(); } - -REQUEST_END_CLEANUP: - curl_easy_cleanup(easyhandle); - fclose(fp); - return failed ? std::optional() : std::optional(downloadPath); } bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecksum) @@ -212,6 +211,22 @@ bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecks std::vector buffer(bufferSize, '\0'); std::ifstream fp(modPath.generic_string(), std::ios::binary); + ScopeGuard cleanup( + [&] + { + if (NULL != hashHandle) + { + BCryptDestroyHash(hashHandle); // Handle to hash/MAC object which needs to be destroyed + } + + if (NULL != algorithmHandle) + { + BCryptCloseAlgorithmProvider( + algorithmHandle, // Handle to the algorithm provider which needs to be closed + 0); // Flags + } + }); + // Open an algorithm handle // This sample passes BCRYPT_HASH_REUSABLE_FLAG with BCryptAlgorithmProvider(...) to load a provider which supports reusable hash status = BCryptOpenAlgorithmProvider( @@ -222,7 +237,7 @@ bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecks if (!NT_SUCCESS(status)) { modState.state = MOD_CORRUPTED; - goto cleanup; + return false; } // Obtain the length of the hash @@ -235,7 +250,6 @@ bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecks 0); // Flags if (!NT_SUCCESS(status)) { - // goto cleanup; modState.state = MOD_CORRUPTED; return false; } @@ -252,7 +266,7 @@ bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecks if (!NT_SUCCESS(status)) { modState.state = MOD_CORRUPTED; - goto cleanup; + return false; } // Hash archive content @@ -273,7 +287,7 @@ bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecks if (!NT_SUCCESS(status)) { modState.state = MOD_CORRUPTED; - goto cleanup; + return false; } } } @@ -289,7 +303,7 @@ bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecks if (!NT_SUCCESS(status)) { modState.state = MOD_CORRUPTED; - goto cleanup; + return false; } // Convert hash to string using bytes raw values @@ -302,21 +316,6 @@ bool ModDownloader::IsModLegit(fs::path modPath, std::string_view expectedChecks spdlog::info("Expected checksum: {}", expectedChecksum.data()); spdlog::info("Computed checksum: {}", ss.str()); return expectedChecksum.compare(ss.str()) == 0; - -cleanup: - if (NULL != hashHandle) - { - BCryptDestroyHash(hashHandle); // Handle to hash/MAC object which needs to be destroyed - } - - if (NULL != algorithmHandle) - { - BCryptCloseAlgorithmProvider( - algorithmHandle, // Handle to the algorithm provider which needs to be closed - 0); // Flags - } - - return false; } bool ModDownloader::IsModAuthorized(std::string_view modName, std::string_view modVersion) @@ -367,11 +366,20 @@ void ModDownloader::ExtractMod(fs::path modPath) fs::path modDirectory; file = unzOpen(modPath.generic_string().c_str()); + ScopeGuard cleanup( + [&] + { + if (unzClose(file) != MZ_OK) + { + spdlog::error("Failed closing mod archive after extraction."); + } + }); + if (file == NULL) { spdlog::error("Cannot open archive located at {}.", modPath.generic_string()); modState.state = FAILED_READING_ARCHIVE; - goto EXTRACTION_CLEANUP; + return; } unz_global_info64 gi; @@ -381,7 +389,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("Failed getting information from archive (error code: {})", status); modState.state = FAILED_READING_ARCHIVE; - goto EXTRACTION_CLEANUP; + return; } // Update state @@ -414,7 +422,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("Parent directory ({}) creation failed.", fileDestination.parent_path().generic_string()); modState.state = FAILED_WRITING_TO_DISK; - goto EXTRACTION_CLEANUP; + return; } } @@ -426,7 +434,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("Directory creation failed: {}", ec.message()); modState.state = FAILED_WRITING_TO_DISK; - goto EXTRACTION_CLEANUP; + return; } } // ...else create file @@ -437,7 +445,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("File \"{}\" was not found in archive.", zipFilename); modState.state = FAILED_READING_ARCHIVE; - goto EXTRACTION_CLEANUP; + return; } // Create file @@ -452,7 +460,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("Could not open file {} from archive.", zipFilename); modState.state = FAILED_READING_ARCHIVE; - goto EXTRACTION_CLEANUP; + return; } // Create destination file @@ -461,7 +469,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("Failed creating destination file."); modState.state = FAILED_WRITING_TO_DISK; - goto EXTRACTION_CLEANUP; + return; } // Allocate memory for buffer @@ -470,7 +478,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("Error while allocating memory."); modState.state = FAILED_WRITING_TO_DISK; - goto EXTRACTION_CLEANUP; + return; } // Extract file to destination @@ -501,7 +509,7 @@ void ModDownloader::ExtractMod(fs::path modPath) { spdlog::error("An error occurred during file extraction (code: {})", err); modState.state = FAILED_WRITING_TO_DISK; - goto EXTRACTION_CLEANUP; + return; } err = unzCloseCurrentFile(file); if (err != UNZ_OK) @@ -522,16 +530,10 @@ void ModDownloader::ExtractMod(fs::path modPath) if (status != UNZ_OK) { spdlog::error("Error while browsing archive files (error code: {}).", status); - goto EXTRACTION_CLEANUP; + return; } } } - -EXTRACTION_CLEANUP: - if (unzClose(file) != MZ_OK) - { - spdlog::error("Failed closing mod archive after extraction."); - } } void ModDownloader::DownloadMod(std::string modName, std::string modVersion) @@ -548,6 +550,22 @@ void ModDownloader::DownloadMod(std::string modName, std::string modVersion) { fs::path archiveLocation; + ScopeGuard cleanup( + [&] + { + try + { + remove(archiveLocation); + } + catch (const std::exception& a) + { + spdlog::error("Error while removing downloaded archive: {}", a.what()); + } + + modState.state = DONE; + spdlog::info("Done downloading {}.", modName); + }); + // Download mod archive std::string expectedHash = verifiedMods[modName].versions[modVersion].checksum; std::optional fetchingResult = FetchModFromDistantStore(std::string_view(modName), std::string_view(modVersion)); @@ -555,31 +573,18 @@ void ModDownloader::DownloadMod(std::string modName, std::string modVersion) { spdlog::error("Something went wrong while fetching archive, aborting."); modState.state = MOD_FETCHING_FAILED; - goto REQUEST_END_CLEANUP; + return; } archiveLocation = fetchingResult.value(); if (!IsModLegit(archiveLocation, std::string_view(expectedHash))) { spdlog::warn("Archive hash does not match expected checksum, aborting."); modState.state = MOD_CORRUPTED; - goto REQUEST_END_CLEANUP; + return; } // Extract downloaded mod archive ExtractMod(archiveLocation); - - REQUEST_END_CLEANUP: - try - { - remove(archiveLocation); - } - catch (const std::exception& a) - { - spdlog::error("Error while removing downloaded archive: {}", a.what()); - } - - modState.state = DONE; - spdlog::info("Done downloading {}.", modName); }); requestThread.detach(); diff --git a/primedev/util/utils.h b/primedev/util/utils.h index 85922692..1a419607 100644 --- a/primedev/util/utils.h +++ b/primedev/util/utils.h @@ -1,3 +1,23 @@ #pragma once void RemoveAsciiControlSequences(char* str, bool allow_color_codes); + +class ScopeGuard +{ +public: + auto operator=(ScopeGuard&) = delete; + ScopeGuard(ScopeGuard&) = delete; + + ScopeGuard(std::function callback) : m_callback(callback) {} + ~ScopeGuard() + { + m_callback(); + } + void Dismiss() + { + m_callback = [] {}; + } + +private: + std::function m_callback; +}; -- cgit v1.2.3