Fixes some jasp save and load issues

libarchive seemed to mess up the strings from utf8, the code still seemed to assume an oldschool codepage however this is easy to avoid, I just added back the usage of the archive_..._w interface I also noticed removing an analysis didnt trigger a "modified" flag on datasetpackage so I added that too. Saving the modified file however also didnt work because the check for the importer somehow blew out the constructor of ArchiveReader thereby avoiding the close() in the destructor we just never noticed because none of us work on windows a lot I think Fixes jasp-stats/jasp-issues#2289
shun2wang · Sep 13, 2023 · c7e0f72 · c7e0f72
1 parent 5406a63
commit c7e0f72
Show file tree

Hide file tree

Showing 8 changed files with 53 additions and 15 deletions.
diff --git a/Common/utils.cpp b/Common/utils.cpp
@@ -256,7 +256,10 @@ std::wstring Utils::getShortPathWin(const std::wstring & longPath)
 
 	length = GetShortPathName(longPath.c_str(), buffer, length);
 	if (length == 0)
+	{
+		delete[] buffer;
 		return longPath;
+	}
 
 	std::wstring shortPath(buffer, length);
 
@@ -280,4 +283,19 @@ string Utils::wstringToString(const std::wstring & wstr)
 	return str;
 
 }
+
+wstring Utils::stringToWString(const std::string &str)
+{
+	std::wstring wstr;
+
+	//get size of buffer we need
+	int requiredSize = MultiByteToWideChar(CP_UTF8, 0, str.data(), -1, NULL, 0);
+	wstr.resize(requiredSize);
+
+	//convert it
+	MultiByteToWideChar(CP_UTF8, 0, str.data(), -1, wstr.data(), wstr.size());
+	wstr.resize(requiredSize-1);//drop /nul
+
+	return wstr;
+}
 #endif
diff --git a/Common/utils.h b/Common/utils.h
@@ -69,6 +69,7 @@ class Utils
 #ifdef _WIN32
 	static std::wstring	getShortPathWin(const std::wstring & path);
 	static std::string  wstringToString(const std::wstring & wstr);
+	static std::wstring stringToWString(const std::string & str);
 #endif
 
 

diff --git a/CommonData/archivereader.cpp b/CommonData/archivereader.cpp
@@ -26,6 +26,7 @@
 #include <fcntl.h>
 #include <archive_entry.h>
 #include "log.h"
+#include "utils.h"
 
 using namespace std;
 
@@ -57,7 +58,11 @@ void ArchiveReader::openEntry(const string &archivePath, const string &entryPath
 		archive_read_support_filter_all(_archive);
 		archive_read_support_format_all(_archive);
 
-        int r = archive_read_open_filename(_archive, pathArchive.string().c_str(), 10240);
+#ifdef _WIN32
+		int r = archive_read_open_filename_w(_archive, Utils::stringToWString(pathArchive.string()).c_str(), 10240);
+#else
+		int r = archive_read_open_filename(_archive, pathArchive.c_str(), 10240);
+#endif
 
 		if (r == ARCHIVE_OK)
 		{
@@ -76,6 +81,7 @@ void ArchiveReader::openEntry(const string &archivePath, const string &entryPath
 					break;
 				}
 			}
+
 			if (!success)
 				throw runtime_error("No entry (" + entryPath + ") found in archive file.");
 		}
@@ -238,7 +244,11 @@ vector<string> ArchiveReader::getEntryPaths(const string &archivePath, const str
 		archive_read_support_filter_all(a);
 		archive_read_support_format_all(a);
 
-        int r = archive_read_open_filename(a, pathArchive.string().c_str(), 10240);
+#ifdef _WIN32
+		int r = archive_read_open_filename_w(a, Utils::stringToWString(pathArchive.string()).c_str(), 10240);
+#else
+		int r = archive_read_open_filename(a, pathArchive.c_str(), 10240);
+#endif
 
 		if (r == ARCHIVE_OK)
 		{

diff --git a/CommonData/archivereader.h b/CommonData/archivereader.h
@@ -35,7 +35,9 @@
 class ArchiveReader
 {
 public:
+	ArchiveReader(){}
 	ArchiveReader(const std::string &archivePath, const std::string &entryPath);
+	ArchiveReader(ArchiveReader && other) = default;
 
 	~ArchiveReader();
 
@@ -76,6 +78,8 @@ class ArchiveReader
 	 */
 	std::string readAllData(int blockSize, int &errorCode);
 
+	void openEntry(const std::string &archivePath, const std::string &entryPath);
+
 	/**
 	 * @brief close Closes archive/file.
 	 */
@@ -137,7 +141,7 @@ class ArchiveReader
 	std::string					_archivePath,
 								_entryPath;
 
-	void openEntry(const std::string &archivePath, const std::string &entryPath);
+
 };
 
 #endif // ARCHIVEREADER_H
diff --git a/Desktop/analysis/analyses.cpp b/Desktop/analysis/analyses.cpp
@@ -286,6 +286,7 @@ void Analyses::removeAnalysis(Analysis *analysis)
 
 	emit countChanged();
 	emit analysisRemoved(analysis);
+	emit somethingModified();
 
 	delete analysis;
 }

diff --git a/Desktop/data/exporters/jaspexporter.cpp b/Desktop/data/exporters/jaspexporter.cpp
@@ -50,14 +50,18 @@ void JASPExporter::saveDataSet(const std::string &path, std::function<void(int)>
 	a = archive_write_new();
 	archive_write_set_format_zip(a);
 	archive_write_set_compression_xz(a);
-
+
+#ifdef _WIN32
+	if (archive_write_open_filename_w(a, tq(path).toStdWString().c_str()) != ARCHIVE_OK)
+#else
 	if (archive_write_open_filename(a, path.c_str()) != ARCHIVE_OK)
-		throw std::runtime_error("File could not be opened.");
+#endif
+		throw std::runtime_error(std::string("File could not be opened because of ") + archive_error_string(a));
 
-    saveManifest(a);    progressCallback(10);
-    saveAnalyses(a);    progressCallback(30);
-    saveResults(a);     progressCallback(70);
-    saveDatabase(a);    progressCallback(100);
+	saveManifest(a);    progressCallback(10);
+	saveAnalyses(a);    progressCallback(30);
+	saveResults(a);     progressCallback(70);
+	saveDatabase(a);    progressCallback(100);
 
 	if (archive_write_close(a) != ARCHIVE_OK)
 		throw std::runtime_error("File could not be closed.");

diff --git a/Desktop/data/importers/jaspimporter.cpp b/Desktop/data/importers/jaspimporter.cpp
@@ -40,7 +40,7 @@ void JASPImporter::loadDataSet(const std::string &path, std::function<void(int)>
 
 	DataSetPackage * packageData = DataSetPackage::pkg();
 
-        packageData->setIsJaspFile(true);
+	packageData->setIsJaspFile(true);
 
 	readManifest(path);
 
@@ -174,9 +174,10 @@ void JASPImporter::readManifest(const std::string &path)
 {
 	bool            foundVersion		= false;
 	std::string     manifestName		= "manifest.json";
-	ArchiveReader	manifestReader		= ArchiveReader(path, manifestName);
+	ArchiveReader	manifestReader;
+	manifestReader.openEntry(path, manifestName); //separate from constructor to avoid a failed close (because an exception in constructor messes up destructor)
 	int             size				= manifestReader.bytesAvailable(),
-		errorCode;
+					errorCode;
 
 	if (size > 0)
 	{

diff --git a/Desktop/data/importers/jaspimporterold.cpp b/Desktop/data/importers/jaspimporterold.cpp
@@ -320,7 +320,8 @@ void JASPImporterOld::readManifest(const std::string &path)
 {
 	bool            foundVersion		= false;
 	std::string     manifestName		= "META-INF/MANIFEST.MF";
-	ArchiveReader	manifest			= ArchiveReader(path, manifestName);
+	ArchiveReader	manifest;
+	manifest.openEntry(path, manifestName);  //separate from constructor to avoid a failed close (because an exception in constructor messes up destructor)
 	int             size				= manifest.bytesAvailable();
 
 	if (size > 0)
@@ -352,8 +353,6 @@ void JASPImporterOld::readManifest(const std::string &path)
 
 	if ( ! foundVersion)
 		throw std::runtime_error("Archive missing version information.");
-
-	manifest.close();
 }
 
 bool JASPImporterOld::parseJsonEntry(Json::Value &root, const std::string &path,  const std::string &entry, bool required)