Merge pull request #889 from flowln/fix_zip_extract

This commit is contained in:
Sefa Eyeoglu 2023-03-01 09:51:08 +01:00 committed by GitHub
commit 0eae9355e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 43 deletions

View File

@ -66,7 +66,12 @@ bool InstanceImportTask::abort()
if (m_filesNetJob) if (m_filesNetJob)
m_filesNetJob->abort(); m_filesNetJob->abort();
if (m_extractFuture.isRunning()) {
// NOTE: The tasks created by QtConcurrent::run() can't actually get cancelled,
// but we can use this call to check the state when the extraction finishes.
m_extractFuture.cancel(); m_extractFuture.cancel();
m_extractFuture.waitForFinished();
}
return Task::abort(); return Task::abort();
} }
@ -185,18 +190,20 @@ void InstanceImportTask::processZipPack()
// make sure we extract just the pack // make sure we extract just the pack
m_extractFuture = QtConcurrent::run(QThreadPool::globalInstance(), MMCZip::extractSubDir, m_packZip.get(), root, extractDir.absolutePath()); m_extractFuture = QtConcurrent::run(QThreadPool::globalInstance(), MMCZip::extractSubDir, m_packZip.get(), root, extractDir.absolutePath());
connect(&m_extractFutureWatcher, &QFutureWatcher<QStringList>::finished, this, &InstanceImportTask::extractFinished); connect(&m_extractFutureWatcher, &QFutureWatcher<QStringList>::finished, this, &InstanceImportTask::extractFinished);
connect(&m_extractFutureWatcher, &QFutureWatcher<QStringList>::canceled, this, &InstanceImportTask::extractAborted);
m_extractFutureWatcher.setFuture(m_extractFuture); m_extractFutureWatcher.setFuture(m_extractFuture);
} }
void InstanceImportTask::extractFinished() void InstanceImportTask::extractFinished()
{ {
m_packZip.reset(); m_packZip.reset();
if (!m_extractFuture.result())
{ if (m_extractFuture.isCanceled())
return;
if (!m_extractFuture.result().has_value()) {
emitFailed(tr("Failed to extract modpack")); emitFailed(tr("Failed to extract modpack"));
return; return;
} }
QDir extractDir(m_stagingPath); QDir extractDir(m_stagingPath);
qDebug() << "Fixing permissions for extracted pack files..."; qDebug() << "Fixing permissions for extracted pack files...";
@ -250,11 +257,6 @@ void InstanceImportTask::extractFinished()
} }
} }
void InstanceImportTask::extractAborted()
{
emitAborted();
}
void InstanceImportTask::processFlame() void InstanceImportTask::processFlame()
{ {
FlameCreationTask* inst_creation_task = nullptr; FlameCreationTask* inst_creation_task = nullptr;

View File

@ -81,7 +81,6 @@ private slots:
void downloadProgressChanged(qint64 current, qint64 total); void downloadProgressChanged(qint64 current, qint64 total);
void downloadAborted(); void downloadAborted();
void extractFinished(); void extractFinished();
void extractAborted();
private: /* data */ private: /* data */
NetJob::Ptr m_filesNetJob; NetJob::Ptr m_filesNetJob;

View File

@ -275,7 +275,7 @@ bool MMCZip::findFilesInZip(QuaZip * zip, const QString & what, QStringList & re
// ours // ours
std::optional<QStringList> MMCZip::extractSubDir(QuaZip *zip, const QString & subdir, const QString &target) std::optional<QStringList> MMCZip::extractSubDir(QuaZip *zip, const QString & subdir, const QString &target)
{ {
auto absDirectoryUrl = QUrl::fromLocalFile(target); auto target_top_dir = QUrl::fromLocalFile(target);
QStringList extracted; QStringList extracted;
@ -295,58 +295,53 @@ std::optional<QStringList> MMCZip::extractSubDir(QuaZip *zip, const QString & su
return std::nullopt; return std::nullopt;
} }
do do {
{ QString file_name = zip->getCurrentFileName();
QString name = zip->getCurrentFileName(); if (!file_name.startsWith(subdir))
if(!name.startsWith(subdir))
{
continue; continue;
}
name.remove(0, subdir.size()); auto relative_file_name = QDir::fromNativeSeparators(file_name.remove(0, subdir.size()));
auto original_name = name; auto original_name = relative_file_name;
// Fix subdirs/files ending with a / getting transformed into absolute paths // Fix subdirs/files ending with a / getting transformed into absolute paths
if(name.startsWith('/')){ if (relative_file_name.startsWith('/'))
name = name.mid(1); relative_file_name = relative_file_name.mid(1);
}
// Fix weird "folders with a single file get squashed" thing // Fix weird "folders with a single file get squashed" thing
QString path; QString sub_path;
if(name.contains('/') && !name.endsWith('/')){ if (relative_file_name.contains('/') && !relative_file_name.endsWith('/')) {
path = name.section('/', 0, -2) + "/"; sub_path = relative_file_name.section('/', 0, -2) + '/';
FS::ensureFolderPathExists(FS::PathCombine(target, path)); FS::ensureFolderPathExists(FS::PathCombine(target, sub_path));
name = name.split('/').last(); relative_file_name = relative_file_name.split('/').last();
} }
QString absFilePath; QString target_file_path;
if(name.isEmpty()) if (relative_file_name.isEmpty()) {
{ target_file_path = target + '/';
absFilePath = FS::PathCombine(target, "/"); // FIXME this seems weird } else {
} target_file_path = FS::PathCombine(target_top_dir.path(), sub_path, relative_file_name);
else if (relative_file_name.endsWith('/') && !target_file_path.endsWith('/'))
{ target_file_path += '/';
absFilePath = FS::PathCombine(target, path + name);
} }
if (!absDirectoryUrl.isParentOf(QUrl::fromLocalFile(absFilePath))) { if (!target_top_dir.isParentOf(QUrl::fromLocalFile(target_file_path))) {
qWarning() << "Extracting" << name << "was cancelled, because it was effectively outside of the target path" << target; qWarning() << "Extracting" << relative_file_name << "was cancelled, because it was effectively outside of the target path" << target;
return std::nullopt; return std::nullopt;
} }
if (!JlCompress::extractFile(zip, "", absFilePath)) if (!JlCompress::extractFile(zip, "", target_file_path)) {
{ qWarning() << "Failed to extract file" << original_name << "to" << target_file_path;
qWarning() << "Failed to extract file" << original_name << "to" << absFilePath;
JlCompress::removeFile(extracted); JlCompress::removeFile(extracted);
return std::nullopt; return std::nullopt;
} }
extracted.append(absFilePath); extracted.append(target_file_path);
QFile::setPermissions(absFilePath, QFileDevice::Permission::ReadUser | QFileDevice::Permission::WriteUser | QFileDevice::Permission::ExeUser); QFile::setPermissions(target_file_path, QFileDevice::Permission::ReadUser | QFileDevice::Permission::WriteUser | QFileDevice::Permission::ExeUser);
qDebug() << "Extracted file" << name << "to" << absFilePath; qDebug() << "Extracted file" << relative_file_name << "to" << target_file_path;
} while (zip->goToNextFile()); } while (zip->goToNextFile());
return extracted; return extracted;
} }