From 2d5b7b5fb48d182fbab8e4ad69e9a552a3c07c6e Mon Sep 17 00:00:00 2001
From: sfan5 <sfan5@live.de>
Date: Sun, 19 Sep 2021 16:55:35 +0200
Subject: Make fs::extractZipFile thread-safe

---
 src/filesys.cpp | 102 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 57 insertions(+), 45 deletions(-)

(limited to 'src/filesys.cpp')

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 <windows.h>
@@ -201,7 +208,11 @@ std::string CreateTempFile()
 	return path;
 }
 
-#else // POSIX
+#else
+
+/*********
+ * POSIX *
+ *********/
 
 #include <sys/types.h>
 #include <dirent.h>
@@ -392,6 +403,10 @@ std::string CreateTempFile()
 
 #endif
 
+/****************************
+ * portable implementations *
+ ****************************/
+
 void GetRecursiveDirs(std::vector<std::string> &dirs, const std::string &dir)
 {
 	static const std::set<char> 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<io::IFileArchive> 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);
-				return false;
-			}
-
-			io::IReadFile* toread = opened_zip->createAndOpenFile(i);
+		if (files_in_zip->isDirectory(i))
+			continue; // ignore, we create dirs as necessary
 
-			FILE *targetfile = fopen(fullpath.c_str(),"wb");
+		if (!fs::PathExists(fullpath_dir) && !fs::CreateAllDirs(fullpath_dir))
+			return false;
 
-			if (targetfile == NULL) {
-				fs->removeFileArchive(fs->getFileArchiveCount()-1);
-				return false;
-			}
+		irr_ptr<io::IReadFile> toread(opened_zip->createAndOpenFile(i));
 
-			char read_buffer[1024];
-			long total_read = 0;
+		std::ofstream os(fullpath.c_str(), std::ios::binary);
+		if (!os.good())
+			return false;
 
-			while (total_read < toread->getSize()) {
+		char buffer[4096];
+		long total_read = 0;
 
-				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;
+		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();
 			}
-
-			fclose(targetfile);
+			if (error) {
+				os.close();
+				remove(fullpath.c_str());
+				return false;
+			}
+			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)
-- 
cgit v1.2.3