From 7e4462e0acd4461e002e3d90157bdfe6ec41322c Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 5 Apr 2024 15:55:54 +0200 Subject: [PATCH] Better handling of temporary folders --- builtin/mainmenu/content/dlg_contentstore.lua | 5 +---- doc/menu_lua_api.md | 4 ++-- src/filesys.cpp | 19 +++++++++++++++++++ src/filesys.h | 12 +++++++++--- src/script/lua_api/l_mainmenu.cpp | 2 +- src/unittest/test.cpp | 9 ++------- 6 files changed, 34 insertions(+), 17 deletions(-) diff --git a/builtin/mainmenu/content/dlg_contentstore.lua b/builtin/mainmenu/content/dlg_contentstore.lua index 1d6b30cf6..f42b3d4e5 100644 --- a/builtin/mainmenu/content/dlg_contentstore.lua +++ b/builtin/mainmenu/content/dlg_contentstore.lua @@ -98,15 +98,12 @@ local function download_and_extract(param) local tempfolder = core.get_temp_path() if tempfolder ~= "" then - tempfolder = tempfolder .. DIR_DELIM .. "MT_" .. math.random(1, 1024000) if not core.extract_zip(filename, tempfolder) then tempfolder = nil end - else - tempfolder = nil end os.remove(filename) - if not tempfolder then + if not tempfolder or tempfolder == "" then return { msg = fgettext_ne("Failed to extract \"$1\" (unsupported file type or broken archive)", package.title), } diff --git a/doc/menu_lua_api.md b/doc/menu_lua_api.md index 979ef868f..cb1f07a90 100644 --- a/doc/menu_lua_api.md +++ b/doc/menu_lua_api.md @@ -93,8 +93,8 @@ Filesystem registered in the core (possible in async calls) * `core.get_cache_path()` -> path of cache * `core.get_temp_path([param])` (possible in async calls) - * `param`=true: returns path to a temporary file - otherwise: returns path to the temporary folder + * `param`=true: returns path to a newly created temporary file + * otherwise: returns path to a newly created temporary folder HTTP Requests diff --git a/src/filesys.cpp b/src/filesys.cpp index 9f493d7cf..1ae7f57aa 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -223,6 +223,16 @@ std::string CreateTempFile() return path; } +std::string CreateTempDir() +{ + std::string path = TempPath() + DIR_DELIM "MT_XXXXXX"; + _mktemp_s(&path[0], path.size() + 1); // modifies path + // will error if it already exists + if (!CreateDirectory(path.c_str(), nullptr)) + return ""; + return path; +} + bool CopyFileContents(const std::string &source, const std::string &target) { BOOL ok = CopyFileEx(source.c_str(), target.c_str(), nullptr, nullptr, @@ -446,6 +456,15 @@ std::string CreateTempFile() return path; } +std::string CreateTempDir() +{ + std::string path = TempPath() + DIR_DELIM "MT_XXXXXX"; + auto r = mkdtemp(&path[0]); // modifies path + if (!r) + return ""; + return path; +} + namespace { struct FileDeleter { void operator()(FILE *stream) { diff --git a/src/filesys.h b/src/filesys.h index 2a6476107..3edc3eef6 100644 --- a/src/filesys.h +++ b/src/filesys.h @@ -75,13 +75,19 @@ bool RecursiveDelete(const std::string &path); bool DeleteSingleFileOrEmptyDirectory(const std::string &path); -// Returns path to temp directory, can return "" on error +/// Returns path to temp directory. +/// You probably don't want to use this directly, see `CreateTempFile` or `CreateTempDir`. +/// @return path or "" on error std::string TempPath(); -// Returns path to securely-created temporary file (will already exist when this function returns) -// can return "" on error +/// Returns path to securely-created temporary file (will already exist when this function returns). +/// @return path or "" on error std::string CreateTempFile(); +/// Returns path to securely-created temporary directory (will already exist when this function returns). +/// @return path or "" on error +std::string CreateTempDir(); + /* Returns a list of subdirectories, including the path itself, but excluding hidden directories (whose names start with . or _) */ diff --git a/src/script/lua_api/l_mainmenu.cpp b/src/script/lua_api/l_mainmenu.cpp index fdb9e3740..a5913e807 100644 --- a/src/script/lua_api/l_mainmenu.cpp +++ b/src/script/lua_api/l_mainmenu.cpp @@ -729,7 +729,7 @@ int ModApiMainMenu::l_get_cache_path(lua_State *L) int ModApiMainMenu::l_get_temp_path(lua_State *L) { if (lua_isnoneornil(L, 1) || !lua_toboolean(L, 1)) - lua_pushstring(L, fs::TempPath().c_str()); + lua_pushstring(L, fs::CreateTempDir().c_str()); else lua_pushstring(L, fs::CreateTempFile().c_str()); return 1; diff --git a/src/unittest/test.cpp b/src/unittest/test.cpp index 1fd9001bf..e2d19b3e1 100644 --- a/src/unittest/test.cpp +++ b/src/unittest/test.cpp @@ -324,13 +324,8 @@ std::string TestBase::getTestTempDirectory() if (!m_test_dir.empty()) return m_test_dir; - char buf[32]; - porting::mt_snprintf(buf, sizeof(buf), "%08X", myrand()); - - m_test_dir = fs::TempPath() + DIR_DELIM "mttest_" + buf; - if (!fs::CreateDir(m_test_dir)) - UASSERT(false); - + m_test_dir = fs::CreateTempDir(); + UASSERT(!m_test_dir.empty()); return m_test_dir; }