Better handling of temporary folders

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),
 		}

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
+  * `param`=true: returns path to a newly created temporary file
-    otherwise: returns path to the temporary folder
+  * otherwise: returns path to a newly created temporary folder
 
 
 HTTP Requests

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) {

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)
+/// Returns path to securely-created temporary file (will already exist when this function returns).
-// can return "" on error
+/// @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 _)
 */

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;

src/unittest/test.cpp

@@ -324,13 +324,8 @@ std::string TestBase::getTestTempDirectory()
 	if (!m_test_dir.empty())
 		return m_test_dir;
 
-	char buf[32];
+	m_test_dir = fs::CreateTempDir();
-	porting::mt_snprintf(buf, sizeof(buf), "%08X", myrand());
+	UASSERT(!m_test_dir.empty());
-
-	m_test_dir = fs::TempPath() + DIR_DELIM "mttest_" + buf;
-	if (!fs::CreateDir(m_test_dir))
-		UASSERT(false);
-
 	return m_test_dir;
 }