Skip to content

cherry-pick 2 release/eagle commits to develop/snipe #367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LiHua000 wants to merge 2 commits into
base: develop/snipe
Choose a base branch
from
Open

cherry-pick 2 release/eagle commits to develop/snipe #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f12b93
fix: remove empty files left when split-volume encrypted extraction f…
LiHua000 Feb 2, 2026
450e707
fix: [unrar] can not extract rar files with longFilenames
LiHua000 Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions 3rdparty/interface/archiveinterface/cliinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,14 @@ PluginFinishType CliInterface::extractFiles(const QList<FileEntry> &files, const
}
}
}

if (bHandleLongName && !checkMoveCapability()) {
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the duplicated long-filename/moveProgram check into a helper and simplifying the cleanup routine into separate file/dir phases to make the logic clearer and easier to maintain.

You can reduce duplication and simplify control flow in two spots without changing behavior.

1) Consolidate bHandleLongName && !checkMoveCapability() logic

You now have the same warning + flag-reset logic twice. Extracting a helper keeps behavior identical and makes future changes safer:

// CliInterface.h
class CliInterface : public QObject {
    ...
private:
    void ensureMoveCapabilityForLongNames(bool &bHandleLongName);
    ...
};
// CliInterface.cpp
void CliInterface::ensureMoveCapabilityForLongNames(bool &bHandleLongName)
{
    if (!bHandleLongName) {
        return;
    }
    if (checkMoveCapability()) {
        return;
    }

    qWarning() << "Long filename detected, but moveProgram (" << m_cliProps->property("moveProgram").toString() << ") is not available.";
    qWarning() << "Archive format:" << m_mimetype.name() << "Skipping long name handling.";
    qWarning() << "The extraction tool will report errors for files with names exceeding system limit (255 bytes).";
    bHandleLongName = false;
}

Then replace both duplicated blocks with:

// first location
if (!bLnfs) {
    ...
}
ensureMoveCapabilityForLongNames(bHandleLongName);

// second location
if (!bLnfs) {
    ...
}
ensureMoveCapabilityForLongNames(bHandleLongName);

This keeps the timing of the check the same while centralizing the messages and policy.

2) Simplify removeExtractedFilesOnFailure

You can avoid the QList<QPair<QString,bool>> and the do/while by separating file and directory paths and processing them in two clearly named phases:

void CliInterface::removeExtractedFilesOnFailure(const QString &strTargetPath,
                                                 const QList<FileEntry> &entries)
{
    QList<FileEntry> listToRemove = entries;
    if (listToRemove.isEmpty()) {
        listToRemove = DataManager::get_instance().archiveData().mapFileEntry.values();
    }
    if (listToRemove.isEmpty()) {
        return;
    }

    QDir targetDir(strTargetPath);
    if (!targetDir.exists()) {
        return;
    }

    QStringList filePaths;
    QStringList dirPaths;
    for (const FileEntry &entry : listToRemove) {
        QString relPath = entry.strFullPath;
        if (relPath.endsWith(QLatin1Char('/'))) {
            relPath.chop(1);
        }
        if (relPath.isEmpty()) {
            continue;
        }

        const QString absPath = targetDir.absoluteFilePath(relPath);
        if (entry.isDirectory) {
            dirPaths << absPath;
        } else {
            filePaths << absPath;
        }
    }

    // remove zero-size files
    for (const QString &path : filePaths) {
        QFileInfo fi(path);
        if (fi.exists() && fi.isFile() && fi.size() == 0) {
            QFile::remove(path);
        }
    }

    // remove empty directories bottom-up
    // (one pass is usually enough; if you want to be extra safe, you can loop,
    // but in a helper with a clear name)
    std::sort(dirPaths.begin(), dirPaths.end(),
              [](const QString &a, const QString &b) { return a.length() > b.length(); });
    for (const QString &dirPath : dirPaths) {
        QDir d(dirPath);
        if (d.exists() && d.isEmpty()) {
            d.removeRecursively();
        }
    }
}

This keeps the functionality (cleaning up zero-size files and empty directories under the target) but reduces cognitive load by:

  • Removing the QPair<QString,bool> indirection.
  • Making the “file pass” and “directory pass” explicit.
  • Using a length-based sort for directories to achieve a bottom‑up removal instead of a do/while removed loop.

qWarning() << "Long filename detected, but moveProgram (" << m_cliProps->property("moveProgram").toString() << ") is not available.";
qWarning() << "Archive format:" << m_mimetype.name() << "Skipping long name handling.";
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Consider fixing the missing space and reducing duplication of this warning block.

The log currently joins m_mimetype.name() and "Skipping long name handling." without a space, yielding output like "zipSkipping long name handling.". Also, the bHandleLongName && !checkMoveCapability() block (including the warning and bHandleLongName = false) appears multiple times in extractFiles; consider a small helper or shared formatter so the behavior and message stay consistent across call sites.

Suggested implementation:

        if (bHandleLongName && !checkMoveCapability()) {
            qWarning() << "Long filename detected, but moveProgram (" << m_cliProps->property("moveProgram").toString() << ") is not available.";
            qWarning() << "Archive format:" << m_mimetype.name() << " Skipping long name handling.";
            qWarning() << "The extraction tool will report errors for files with names exceeding system limit (255 bytes).";
            bHandleLongName = false;
        }

To fully address the duplication concern across extractFiles, you can introduce a small helper (either a static function in an anonymous namespace in cliinterface.cpp or a private member on the relevant class), e.g.:

static void logLongNameWithoutMoveProgram(const QMimeType &mimeType, const QVariant &moveProgramProperty)
{
    const QString moveProgram = moveProgramProperty.toString();
    qWarning() << "Long filename detected, but moveProgram (" << moveProgram << ") is not available.";
    qWarning() << "Archive format:" << mimeType.name() << " Skipping long name handling.";
    qWarning() << "The extraction tool will report errors for files with names exceeding system limit (255 bytes).";
}

Then replace each duplicated block of:

if (bHandleLongName && !checkMoveCapability()) {
    qWarning() << ...;
    qWarning() << ...;
    qWarning() << ...;
    bHandleLongName = false;
}

with:

if (bHandleLongName && !checkMoveCapability()) {
    logLongNameWithoutMoveProgram(m_mimetype, m_cliProps->property("moveProgram"));
    bHandleLongName = false;
}

so all call sites share the same, correctly formatted message.

qWarning() << "The extraction tool will report errors for files with names exceeding system limit (255 bytes).";
bHandleLongName = false;
}

if (destPath.startsWith("/tmp") && destPath.contains("/deepin-compressor-")) { // 打开解压列表文件
if (!QDir(destPath).exists()) {
QDir(destPath).mkpath(destPath);
Expand Down Expand Up @@ -242,6 +250,7 @@ PluginFinishType CliInterface::extractFiles(const QList<FileEntry> &files, const
} else {
password = options.password;
}

if (!bLnfs) {
for (QMap<QString, FileEntry>::const_iterator iter = arcData.mapFileEntry.begin(); iter != arcData.mapFileEntry.end(); iter++) {
if (NAME_MAX < iter.value().strFileName.toLocal8Bit().length()) {
Expand All @@ -250,6 +259,14 @@ PluginFinishType CliInterface::extractFiles(const QList<FileEntry> &files, const
}
}
}

if (bHandleLongName && !checkMoveCapability()) {
qWarning() << "Long filename detected, but moveProgram (" << m_cliProps->property("moveProgram").toString() << ") is not available.";
qWarning() << "Archive format:" << m_mimetype.name() << "Skipping long name handling.";
qWarning() << "The extraction tool will report errors for files with names exceeding system limit (255 bytes).";
bHandleLongName = false;
}

if (bHandleLongName) {
if (!handleLongNameExtract(arcData.mapFileEntry.values())) {
m_eErrorType = ET_FileWriteError;
Expand Down Expand Up @@ -1110,6 +1127,70 @@ bool CliInterface::moveExtractTempFilesToDest(const QList<FileEntry> &files, con
return moveSuccess;
}

bool CliInterface::checkMoveCapability()
{
bool hasMoveCapability = false;
QString moveProgram = m_cliProps->property("moveProgram").toString();
if (!moveProgram.isEmpty()) {
QString moveProgramPath = QStandardPaths::findExecutable(moveProgram);
hasMoveCapability = !moveProgramPath.isEmpty();
}
return hasMoveCapability;
}

void CliInterface::removeExtractedFilesOnFailure(const QString &strTargetPath, const QList<FileEntry> &entries)
{
QList<FileEntry> listToRemove = entries;
if (listToRemove.isEmpty()) {
listToRemove = DataManager::get_instance().archiveData().mapFileEntry.values();
}
if (listToRemove.isEmpty()) {
return;
}

QDir targetDir(strTargetPath);
if (!targetDir.exists()) {
return;
}

QList<QPair<QString, bool> > paths; // path, isDirectory
for (const FileEntry &entry : listToRemove) {
QString relPath = entry.strFullPath;
if (relPath.endsWith(QLatin1Char('/'))) {
relPath.chop(1);
}
if (relPath.isEmpty()) {
continue;
}
paths.append(qMakePair(targetDir.absoluteFilePath(relPath), entry.isDirectory));
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Using absoluteFilePath(relPath) directly may allow .. segments to escape strTargetPath.

If relPath contains .. components (e.g. from a crafted archive), targetDir.absoluteFilePath(relPath) may resolve outside strTargetPath, and removeExtractedFilesOnFailure could delete files or directories beyond the intended root. Consider normalizing and validating relPath (e.g. rejecting .. or verifying the resolved path is under targetDir.canonicalPath()) before adding it to paths.

}

for (const auto &p : paths) {
const QString &path = p.first;
if (!p.second) { // 文件
QFileInfo fi(path);
if (fi.exists() && fi.isFile() && fi.size() == 0) {
QFile::remove(path);
}
}
}
// 空目录可能有多层，循环直到本轮没有可删的空目录
bool removed;
do {
removed = false;
for (const auto &p : paths) {
if (!p.second) {
continue;
}
QDir d(p.first);
if (d.exists() && d.isEmpty()) {
d.removeRecursively();
removed = true;
}
}
} while (removed);
}

bool CliInterface::handleLongNameExtract(const QList<FileEntry> &files)
{
ExtractionOptions &options = m_extractOptions;
Expand Down Expand Up @@ -1284,6 +1365,7 @@ void CliInterface::readStdout(bool handleAll)
// 第二个判断条件是处理rar的list，当rar文件含有comment信息的时候需要根据空行解析
if (!line.isEmpty() || (m_listEmptyLines && m_workStatus == WT_List)) {
if (!handleLine(QString::fromLocal8Bit(line), m_workStatus)) {
emit signalprogress(100);
killProcess();
return;
}
Expand Down Expand Up @@ -1328,6 +1410,11 @@ void CliInterface::extractProcessFinished(int exitCode, QProcess::ExitStatus exi
m_indexOfListRootEntry = 0;
m_isEmptyArchive = false;

// 解压失败（如分卷加密包输错密码）且为全部解压到目标路径时，清理已生成的 size 为 0 等残留文件
if (0 != exitCode && m_extractOptions.bAllExtract && !m_extractOptions.strTargetPath.isEmpty()) {
removeExtractedFilesOnFailure(m_extractOptions.strTargetPath, m_files);
}

if (!m_extractOptions.bAllExtract && (!(m_extractOptions.strTargetPath.startsWith("/tmp") && m_extractOptions.strTargetPath.contains("/deepin-compressor-") && m_extractOptions.strDestination.isEmpty()))) {
if (0 == exitCode) { // job正常结束
// 提取操作和打开解压列表文件非第一层的文件
Expand Down
9 changes: 9 additions & 0 deletions 3rdparty/interface/archiveinterface/cliinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,17 @@ class CliInterface : public ReadWriteArchiveInterface
*/
bool moveExtractTempFilesToDest(const QList<FileEntry> &files, const ExtractionOptions &options);

/**
* @brief removeExtractedFilesOnFailure 解压失败时清理已生成的文件（如分卷加密包输错密码时产生的 size 为 0 的文件）
* @param strTargetPath 解压目标路径
* @param entries 本次解压涉及的条目列表（可为空，为空时从 ArchiveData 获取全部）
*/
void removeExtractedFilesOnFailure(const QString &strTargetPath, const QList<FileEntry> &entries);

bool handleLongNameExtract(const QList<FileEntry> &files);

bool checkMoveCapability();

private slots:
/**
* @brief readStdout 读取命令行输出
Expand Down