fix: use preservePaths in batch extraction to keep correct directory …

[LiHua000]

…structure

fix incorrect params

Log: fix bug

Bug: https://pms.uniontech.com/bug-view-362685.html

[sourcery-ai]

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

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

deepin pr auto review

你好！我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。本次修改主要涉及两处变更：一是将子进程的工作目录设置方式从 pProcess->setWorkingDirectory 改为了 QDir::setCurrent；二是将 extractArgs 的第三个参数从 false 改为了 true。

下面我将从语法逻辑、代码质量、代码性能和代码安全四个维度对这段代码进行审查，并提供改进意见。

1. 语法与逻辑审查 (高风险 🚨)

问题：QDir::setCurrent 改变了全局状态，存在严重的逻辑隐患

• 分析：pProcess->setWorkingDirectory(options.strTargetPath) 仅针对当前启动的子进程设置工作目录，是线程安全且作用域局限的。而 QDir::setCurrent(options.strTargetPath) 会改变整个应用程序的当前工作目录（CWD）。
• 后果：
  1. 并发灾难：如果应用内有多个线程同时执行解压任务，调用 QDir::setCurrent 会导致互相覆盖工作目录，使得进程在错误的路径下执行。
  2. 路径污染：解压完成后，应用程序的工作目录被永久改变，后续所有使用相对路径的文件操作（如读取配置、写入日志）都会指向这个解压目标路径，导致不可预期的Bug。
• 改进意见：强烈建议回退此修改，继续使用 pProcess->setWorkingDirectory。如果由于某些第三方库的限制，子进程必须在CWD下运行，也应在子进程启动后（或使用线程局部存储），并在完成后立即恢复原工作目录。

问题：extractArgs 参数变更缺乏上下文，但逻辑上存在疑点

• 分析：将 extractArgs(..., false, ...) 改为 extractArgs(..., true, ...)。从函数名和所在的方法名 handleLongNameExtract（处理长文件名解压）来看，这个布尔参数极大概率代表“是否保留长文件名/完整路径”。
• 疑点：原代码传入 false，现在改为 true。如果是为了修复长文件名被截断的Bug，这个改动是合理的；但如果该参数代表“解压时保留目录结构”，强制设为 true 可能会导致原本只想解压单个文件的用户解压出完整的目录树，改变了原有行为逻辑。
• 改进意见：请确认该布尔参数的准确含义。如果它代表“保留长文件名”，修改合理；如果代表“保留绝对/完整路径结构”，建议根据实际业务需求动态传参，而非硬编码为 true。

2. 代码质量审查

问题：阻塞式调用 waitForFinished(-1) 导致界面冻结

• 分析：在第二处修改下方（第1396行左右），存在 pProcess->waitForFinished(-1)。参数 -1 表示无限期等待，如果解压的文件非常大或磁盘IO阻塞，这会导致当前线程（如果是主线程则会导致UI）直接卡死。
• 改进意见：在GUI程序中，应避免使用阻塞式的 waitForFinished。建议使用状态机或信号槽机制（如 QProcess::finished、QProcess::readyReadStandardOutput）来异步处理解压过程和结果。

问题：代码重复

• 分析：两处 setPtyChannels、setOutputChannelMode、setNextOpenMode、setProgram 和 start() 的代码完全一致，属于典型的代码重复。
• 改进意见：建议将这段进程配置和启动的逻辑抽取为一个私有方法（如 startExtractProcess(bool keepLongName)），以减少冗余，提高可维护性。

3. 代码性能审查

问题：QDir::setCurrent 带来的隐式性能开销

• 分析：如前所述，QDir::setCurrent 修改全局状态，在多线程环境下，为了保证正确性，不得不引入互斥锁来保护当前工作目录的切换和恢复，这会引入不必要的锁竞争和性能损耗。而原方案 setWorkingDirectory 是进程级别的属性设置，由操作系统在创建进程时处理，零额外开销。

4. 代码安全审查 (高风险 🚨)

问题：工作目录劫持与路径穿越风险

• 分析：使用 QDir::setCurrent 后，如果 options.strTargetPath 是一个受控于外部（如用户输入或压缩包内恶意构造的路径）的相对路径，改变全局CWD可能导致程序后续在执行其他敏感操作（如删除临时文件、写入配置）时被误导至恶意目录。
• 改进意见：坚持使用 pProcess->setWorkingDirectory，并确保 options.strTargetPath 和 absoluteDestinationPath 在传入前已经通过规范化和安全性检查，防止路径穿越攻击。

---

💡 综合改进建议代码示例

基于以上分析，我建议的修改方案如下（回退工作目录修改，提取公共代码）：

// 建议提取的私有方法
void CliInterface::startExtractProcess(const QString &targetPath, const QList<FileEntry> &files, bool preserveLongName, const QString &pwd)
{
    // 1. 安全性：仅设置子进程的工作目录，绝不改变全局CWD
    pProcess->setWorkingDirectory(targetPath);
    
    pProcess->setPtyChannels(KPtyProcess::StdinChannel);
    pProcess->setOutputChannelMode(KProcess::MergedChannels);
    pProcess->setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text);
    
    // 2. 逻辑：根据需求传入长文件名参数
    pProcess->setProgram(m_cliProps->property("extractProgram").toString(),
                         m_cliProps->extractArgs(targetPath, files, preserveLongName, pwd));
    pProcess->start();
}

// 原代码修改处 1
bool CliInterface::handleLongNameExtract(const QList<FileEntry> &files)
{
    qInfo() << "handleLongNameExtract: batch extract files:" << allFileList.count();

    if (!allFileList.isEmpty()) {
        // 调用重构后的方法，传入 true 保留长文件名
        startExtractProcess(options.strTargetPath, allFileList, true, password);

        if (password.isEmpty()) {
            // ... 密码处理逻辑 ...
        }
        // ...
    }
}

// 原代码修改处 2 (重试逻辑处)
// ...
startExtractProcess(options.strTargetPath, allFileList, true, password);
// 注意：强烈建议将下方的 waitForFinished(-1) 改为异步信号槽处理
// pProcess->waitForFinished(-1); 

总结：本次Diff中最核心的问题是 QDir::setCurrent 的误用，这会引入严重的全局状态污染和线程安全问题，必须修改回 setWorkingDirectory。其次，需关注 extractArgs 参数变更带来的业务逻辑影响，并建议重构重复的进程启动代码及异步化阻塞调用。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, max-lvs

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

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

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

[LiHua000]

/merge