From 2d5b7b5fb48d182fbab8e4ad69e9a552a3c07c6e Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sun, 19 Sep 2021 16:55:35 +0200 Subject: [PATCH] Make fs::extractZipFile thread-safe --- src/filesys.cpp | 110 +++++++++++++++++------------- src/filesys.h | 6 +- src/script/lua_api/l_mainmenu.cpp | 8 +-- 3 files changed, 69 insertions(+), 55 deletions(-) diff --git a/src/filesys.cpp b/src/filesys.cpp index a07370c0e..0972acbf9 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -28,11 +28,18 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "log.h" #include "config.h" #include "porting.h" +#ifndef SERVER +#include "irr_ptr.h" +#endif namespace fs { -#ifdef _WIN32 // WINDOWS +#ifdef _WIN32 + +/*********** + * Windows * + ***********/ #define _WIN32_WINNT 0x0501 #include @@ -201,7 +208,11 @@ std::string CreateTempFile() return path; } -#else // POSIX +#else + +/********* + * POSIX * + *********/ #include #include @@ -392,6 +403,10 @@ std::string CreateTempFile() #endif +/**************************** + * portable implementations * + ****************************/ + void GetRecursiveDirs(std::vector &dirs, const std::string &dir) { static const std::set chars_to_ignore = { '_', '.' }; @@ -753,69 +768,66 @@ bool safeWriteToFile(const std::string &path, const std::string &content) return true; } +#ifndef SERVER bool extractZipFile(io::IFileSystem *fs, const char *filename, const std::string &destination) { - if (!fs->addFileArchive(filename, false, false, io::EFAT_ZIP)) { + // Be careful here not to touch the global file hierarchy in Irrlicht + // since this function needs to be thread-safe! + + io::IArchiveLoader *zip_loader = nullptr; + for (u32 i = 0; i < fs->getArchiveLoaderCount(); i++) { + if (fs->getArchiveLoader(i)->isALoadableFileFormat(io::EFAT_ZIP)) { + zip_loader = fs->getArchiveLoader(i); + break; + } + } + if (!zip_loader) { + warningstream << "fs::extractZipFile(): Irrlicht said it doesn't support ZIPs." << std::endl; return false; } - sanity_check(fs->getFileArchiveCount() > 0); - - /**********************************************************************/ - /* WARNING this is not threadsafe!! */ - /**********************************************************************/ - io::IFileArchive* opened_zip = fs->getFileArchive(fs->getFileArchiveCount() - 1); - + irr_ptr opened_zip(zip_loader->createArchive(filename, false, false)); const io::IFileList* files_in_zip = opened_zip->getFileList(); - unsigned int number_of_files = files_in_zip->getFileCount(); - - for (unsigned int i=0; i < number_of_files; i++) { - std::string fullpath = destination; - fullpath += DIR_DELIM; + for (u32 i = 0; i < files_in_zip->getFileCount(); i++) { + std::string fullpath = destination + DIR_DELIM; fullpath += files_in_zip->getFullFileName(i).c_str(); std::string fullpath_dir = fs::RemoveLastPathComponent(fullpath); - if (!files_in_zip->isDirectory(i)) { - if (!fs::PathExists(fullpath_dir) && !fs::CreateAllDirs(fullpath_dir)) { - fs->removeFileArchive(fs->getFileArchiveCount()-1); + if (files_in_zip->isDirectory(i)) + continue; // ignore, we create dirs as necessary + + if (!fs::PathExists(fullpath_dir) && !fs::CreateAllDirs(fullpath_dir)) + return false; + + irr_ptr toread(opened_zip->createAndOpenFile(i)); + + std::ofstream os(fullpath.c_str(), std::ios::binary); + if (!os.good()) + return false; + + char buffer[4096]; + long total_read = 0; + + while (total_read < toread->getSize()) { + long bytes_read = toread->read(buffer, sizeof(buffer)); + bool error = true; + if (bytes_read != 0) { + os.write(buffer, bytes_read); + error = os.fail(); + } + if (error) { + os.close(); + remove(fullpath.c_str()); return false; } - - io::IReadFile* toread = opened_zip->createAndOpenFile(i); - - FILE *targetfile = fopen(fullpath.c_str(),"wb"); - - if (targetfile == NULL) { - fs->removeFileArchive(fs->getFileArchiveCount()-1); - return false; - } - - char read_buffer[1024]; - long total_read = 0; - - while (total_read < toread->getSize()) { - - unsigned int bytes_read = - toread->read(read_buffer,sizeof(read_buffer)); - if ((bytes_read == 0 ) || - (fwrite(read_buffer, 1, bytes_read, targetfile) != bytes_read)) - { - fclose(targetfile); - fs->removeFileArchive(fs->getFileArchiveCount() - 1); - return false; - } - total_read += bytes_read; - } - - fclose(targetfile); + total_read += bytes_read; } - } - fs->removeFileArchive(fs->getFileArchiveCount() - 1); return true; } +#endif bool ReadFile(const std::string &path, std::string &out) { @@ -829,7 +841,7 @@ bool ReadFile(const std::string &path, std::string &out) is.seekg(0); is.read(&out[0], size); - return true; + return !is.fail(); } bool Rename(const std::string &from, const std::string &to) diff --git a/src/filesys.h b/src/filesys.h index f72cb0ba2..233e56bba 100644 --- a/src/filesys.h +++ b/src/filesys.h @@ -24,12 +24,12 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include "exceptions.h" -#ifdef _WIN32 // WINDOWS +#ifdef _WIN32 #define DIR_DELIM "\\" #define DIR_DELIM_CHAR '\\' #define FILESYS_CASE_INSENSITIVE true #define PATH_DELIM ";" -#else // POSIX +#else #define DIR_DELIM "/" #define DIR_DELIM_CHAR '/' #define FILESYS_CASE_INSENSITIVE false @@ -133,7 +133,9 @@ const char *GetFilenameFromPath(const char *path); bool safeWriteToFile(const std::string &path, const std::string &content); +#ifndef SERVER bool extractZipFile(irr::io::IFileSystem *fs, const char *filename, const std::string &destination); +#endif bool ReadFile(const std::string &path, std::string &out); diff --git a/src/script/lua_api/l_mainmenu.cpp b/src/script/lua_api/l_mainmenu.cpp index 57fddc0be..4cfbaec71 100644 --- a/src/script/lua_api/l_mainmenu.cpp +++ b/src/script/lua_api/l_mainmenu.cpp @@ -644,9 +644,9 @@ int ModApiMainMenu::l_extract_zip(lua_State *L) std::string absolute_destination = fs::RemoveRelativePathComponents(destination); if (ModApiMainMenu::mayModifyPath(absolute_destination)) { - auto rendering_engine = getGuiEngine(L)->m_rendering_engine; - fs::CreateAllDirs(absolute_destination); - lua_pushboolean(L, fs::extractZipFile(rendering_engine->get_filesystem(), zipfile, destination)); + auto fs = RenderingEngine::get_raw_device()->getFileSystem(); + bool ok = fs::extractZipFile(fs, zipfile, destination); + lua_pushboolean(L, ok); return 1; } @@ -916,7 +916,7 @@ void ModApiMainMenu::InitializeAsync(lua_State *L, int top) API_FCT(delete_dir); API_FCT(copy_dir); API_FCT(is_dir); - //API_FCT(extract_zip); //TODO remove dependency to GuiEngine + API_FCT(extract_zip); API_FCT(may_modify_path); API_FCT(download_file); API_FCT(get_min_supp_proto);