Skip to content

fix: handle wrong password when processing longname-extract#414

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
LiHua000:release/eagle
May 19, 2026
Merged

fix: handle wrong password when processing longname-extract#414
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
LiHua000:release/eagle

Conversation

@LiHua000
Copy link
Copy Markdown
Contributor

@LiHua000 LiHua000 commented May 19, 2026

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @LiHua000, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link
Copy Markdown
Contributor

@lzwind lzwind left a comment

Choose a reason for hiding this comment

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

要贴上pms单子

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已经仔细审查了你提供的Git Diff,本次修改主要集中在增强CLI解压插件的错误处理逻辑,特别是针对长文件名解压失败、子进程退出状态以及密码错误检测的场景。

整体来看,这次修改填补了之前代码中一些错误未正确上报和信号未发出的漏洞,方向是正确的。但代码在逻辑严谨性、代码性能、代码安全方面还有改进空间。以下是我的详细审查意见:

1. 语法与逻辑

问题 1.1:runProcess 中存在信号重复发送的风险
runProcess 的 lambda 中,新增了对 extractFiles 返回值的判断:

if (exitCode != 0) {
    emit signalprogress(100);
    emit signalFinished(PFT_Error);
    return; // 这里正确返回了
}
deleteProcess();
PluginFinishType ret = extractFiles(m_files, m_extractOptions, property("lnfs").toBool());
if (ret == PFT_Error) {
    emit signalprogress(100);
    emit signalFinished(PFT_Error); // 这里可能造成信号重复发送
}

分析: 如果 extractFiles 返回 PFT_Error,这里会发送 signalFinished(PFT_Error)。然而,从 diff 的上半部分可以看到,extractFiles 内部在 handleLongNameExtract 失败时已经自己 emit 了 signalFinished(PFT_Error)。这会导致同一个错误触发两次 signalFinished 信号,可能引发前端或上层业务逻辑的异常(如弹两次错误框)。
改进意见: 建议统一信号发出的层级。要么由底层 extractFiles 负责发射信号,上层只管返回值;要么底层只设置状态不发射信号,由上层统一发射。考虑到代码原有逻辑,建议移除 extractFiles 内部新增的 emit signalFinished(PFT_Error);,将信号发送权保留给最外层的调用者(如 runProcess)。

问题 1.2:exitCode 检查的时机与 QProcess::ExitStatus

if (pProcess->exitCode() != 0) {

分析: 在 Qt 中,如果进程崩溃(被信号杀死,如 SIGSEGV),exitCode() 的返回值是不可靠的。在调用 exitCode() 之前,必须先检查 exitStatus() == QProcess::NormalExit
改进意见: 增加退出状态的判断:

if (pProcess->exitStatus() == QProcess::NormalExit && pProcess->exitCode() != 0) {
    // 处理正常退出但返回非0的情况
} else if (pProcess->exitStatus() == QProcess::CrashExit) {
    // 处理进程崩溃的情况
    m_eErrorType = ET_CrashError; // 假设有类似枚举
    return false;
}

2. 代码性能

问题 2.1:waitForFinished(-1) 导致线程阻塞

pProcess->start();
pProcess->waitForFinished(-1);

分析: waitForFinished(-1) 会无限期阻塞当前线程。如果这段代码运行在主线程(UI线程),会导致界面完全冻结;如果运行在后台线程,也会导致该线程无法响应取消操作。
改进意见:

  1. 确保此代码不在主线程执行。
  2. 考虑实现异步处理或使用本地事件循环 QEventLoop,以便在等待期间仍能处理一些信号(如用户取消信号)。
  3. 如果必须阻塞,建议设置一个合理的超时时间(如 30 秒),并在超时后判断处理。

3. 代码安全

问题 3.1:通过字符串匹配检测密码错误极度脆弱

QByteArray output = pProcess->readAllStandardOutput();
if (output.contains("Wrong password")) {
    m_eErrorType = ET_WrongPassword;
    return false;
}

分析:

  1. 不安全且脆弱:依赖具体的字符串 "Wrong password" 进行判断是非常不可靠的。如果用户使用的是非英文环境的解压工具(如中文提示“密码错误”),或者解压工具更新了提示语,这段代码将完全失效。
  2. 读取了错误的输出流:通常解压工具(如 unrar, 7z)的报错信息是输出到 stderr 而不是 stdout。这里使用 readAllStandardOutput() 很可能根本读不到密码错误的信息。
    改进意见:
  3. readAllStandardOutput() 改为 readAllStandardError(),或者两者都读。
  4. 使用更健壮的匹配机制,比如匹配多种语言,或者更好的是:检查该解压工具特定的退出码。例如,unrar 在密码错误时通常返回特定的退出码(如 exit code 2 或 5,具体取决于版本),通过退出码判断比通过输出文本判断安全得多。

4. 代码质量

问题 4.1:m_eErrorType 的覆盖保护逻辑略显繁琐

if (m_eErrorType == ET_NoError) {
    m_eErrorType = ET_FileWriteError;
}

分析: 这种写法虽然安全,但如果有多个地方需要设置错误类型,会导致大量模板代码。
改进意见: 建议封装一个设置错误的辅助函数,使代码更简洁:

void CliInterface::setErrorType(ErrorType type) {
    if (m_eErrorType == ET_NoError) {
        m_eErrorType = type;
    }
}
// 调用处:
setErrorType(ET_FileWriteError);

综合修改建议代码示例

针对 handleLongNameExtract 中的新增逻辑,我建议做如下改进:

// 在 cliinterface.h 中添加辅助函数
private:
    void setErrorType(ErrorType type);

// 在 cliinterface.cpp 中实现
void CliInterface::setErrorType(ErrorType type) {
    if (m_eErrorType == ET_NoError) {
        m_eErrorType = type;
    }
}

// 修改 handleLongNameExtract 中的逻辑
pProcess->start();
// 建议不要无限期等待,设置一个超时时间(如30000ms),避免死锁
if (!pProcess->waitForFinished(30000)) {
    setErrorType(ET_TimeoutError); // 假设有超时枚举
    return false;
}

if (pProcess->exitStatus() == QProcess::NormalExit && pProcess->exitCode() != 0) {
    // 优先通过标准错误输出判断,且兼顾多语言或不同提示语的情况
    QByteArray stdErr = pProcess->readAllStandardError();
    QByteArray stdOut = pProcess->readAllStandardOutput();
    // 建议将判断逻辑提取为虚函数,以便针对不同的CLI工具(7z, unrar等)做不同实现
    if (stdErr.contains("Wrong password") || stdOut.contains("Wrong password") || 
        stdErr.contains("密码错误")) { // 临时方案,最佳方案是判断 exitCode
        setErrorType(ET_WrongPassword);
        return false;
    }
} else if (pProcess->exitStatus() == QProcess::CrashExit) {
    // 处理进程崩溃
    setErrorType(ET_CrashError); 
    return false;
}

对于 extractFilesrunProcess 的信号发射逻辑,建议去掉 extractFiles 内部的 emit signalFinished(PFT_Error);,保持底层函数只返回状态,由 runProcess 统一控制信号的发射,避免信号风暴。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@LiHua000
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit 1497bf2 into linuxdeepin:release/eagle May 19, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants