Skip to content

fix(ui): fix compat mode reinstall failure after auth cancel#416

Merged
pengfeixx merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix/compat-mode-reinstall-after-auth-cancel
May 26, 2026
Merged

fix(ui): fix compat mode reinstall failure after auth cancel#416
pengfeixx merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix/compat-mode-reinstall-after-auth-cancel

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

Auth cancel handler set package status to Failed after signalAuthCancel triggered resetInstallStatus, overwriting the cleaned Prepare state. Also, slotReinstall bypassed the compat confirm flow leaving targetRootfs empty, causing installDebs to fall through to apt backend.

授权取消回调在 resetInstallStatus 之后将包状态覆盖为
Failed,且 slotReinstall 绕过确认流程导致 targetRootfs
为空,安装回退到 apt 后端而失败。

Log: 修复兼容模式授权取消后重装失败的问题
PMS: BUG-362603
Influence: 修复后兼容模式下卸载授权取消,点击重新安装按钮能正确走兼容安装流程完成重装。

Auth cancel handler set package status to Failed after
signalAuthCancel triggered resetInstallStatus, overwriting
the cleaned Prepare state. Also, slotReinstall bypassed
the compat confirm flow leaving targetRootfs empty, causing
installDebs to fall through to apt backend.

授权取消回调在 resetInstallStatus 之后将包状态覆盖为
Failed,且 slotReinstall 绕过确认流程导致 targetRootfs
为空,安装回退到 apt 后端而失败。

Log: 修复兼容模式授权取消后重装失败的问题
PMS: BUG-362603
Influence: 修复后兼容模式下卸载授权取消,点击重新安装按钮能正确走兼容安装流程完成重装。
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 @pengfeixx, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次代码变更主要涉及Deb包安装器在处理授权取消时的状态重置逻辑,以及为兼容模式重装场景补充缺失的targetRootfs参数。

以下是针对语法逻辑、代码质量、代码性能和代码安全四个维度的详细审查意见及改进建议:

1. 语法与逻辑

1.1 授权取消时移除 refreshOperatingPackageStatus(Pkg::Failed) 的逻辑合理性

  • 分析:当用户取消授权(ConfigAuthCancel)时,代码移除了将包状态刷新为 Failed 的调用,仅将工作状态重置为 WorkerPrepare 并发送 signalAuthCancel 信号。这在逻辑上是合理的:“取消”不应等同于“失败”。如果标记为 Failed,UI 可能会显示错误图标或提供重试按钮,而实际上用户只是主动取消。移除该调用可以避免误导用户。
  • 潜在风险:需要确认 UI 层在接收到 signalAuthCancel 后,是否有独立逻辑将该包的界面状态恢复为“就绪”或“未安装”。如果 UI 依赖 refreshOperatingPackageStatus 来重置界面,移除它可能导致界面卡在“安装中”的状态。
  • 建议:确认 UI 层(如 SingleInstallPage)对 signalAuthCancel 的槽函数中是否包含了状态重置逻辑。如果没有,建议在发送信号前,显式地将包状态重置为上一个稳定状态(如 Pkg::ReadyPkg::NotInstalled),而不是 Failed

1.2 slotReinstall 中的 targetRootfs 补偿逻辑

  • 分析:在 slotReinstall 中,增加了一段检查 CompatibleTargetRootfsRole 是否为空的逻辑,如果为空则设置默认值 kDefaultRootfs。注释说明这是为了处理授权取消后重装的场景。逻辑上弥补了控制流的漏洞。
  • 潜在风险:这里使用了 m_packagesModel->index(0) 硬编码取第一行数据。如果模型支持多包安装,或者当前操作的包不在索引 0 的位置,这里会修改错误的数据。
  • 建议:如果这是单包安装页面(SingleInstallPage),索引 0 是可以接受的,但建议增加有效性检查以防越界;如果是多包安装,需要获取当前正在操作的包的准确 QModelIndex

1.3 slotInstallPackages() 返回值处理

  • 分析:增加了对 slotInstallPackages() 返回值的判断,如果返回 false 则回退到文件选择界面。这是一个很好的防御性编程。
  • 注意点:在Qt的命名规范中,以 slot 前缀命名的函数通常被认为是无返回值的槽函数。既然现在它有了返回值并参与业务逻辑判断,建议评估是否需要重命名(如 installPackages),以符合常规的代码语义。

2. 代码质量

2.1 魔法值与硬编码

  • 问题m_packagesModel->index(0) 中的 0 属于魔法值。
  • 建议:如果确定是单包安装,可以定义一个常量,或者通过模型的当前选中项来获取 index。

2.2 代码重复

  • 问题:在 deblistmodel.cpp 中,ensureCompatibleProcessorensureImmutableProcessor 里的授权取消处理逻辑完全相同:
    m_workerStatus = WorkerPrepare;
    Q_EMIT signalAuthCancel();
    return;
  • 建议:虽然改动不大,但为了避免后续修改时遗漏(就像本次diff一样需要同时改两个地方),建议在 DebListModel 中提取一个私有方法,如 handleAuthCancel(),将这两处逻辑内聚。

2.3 注释的移除

  • 分析:移除了 // notify UI reset, cancel current flow 注释。
  • 建议:虽然代码逻辑已经比较清晰,但保留该注释有助于快速理解 signalAuthCancel 的业务意图,建议保留或优化为 // Reset worker status and notify UI to cancel current flow

3. 代码性能

  • 分析:本次变更涉及的都是简单的状态赋值、信号发送和条件判断,运行在主线程(UI线程)中。这些操作的时间复杂度均为 $O(1)$,对性能几乎没有任何负面影响。
  • setData 调用:在 slotReinstall 中调用了 setData,这会触发模型的 dataChanged 信号,可能导致 UI 刷新。由于这是低频的用户交互操作(点击重装),性能消耗完全可以忽略。

4. 代码安全

4.1 kDefaultRootfs 的安全性

  • 分析:在 targetRootfs 为空时,使用了 kDefaultRootfs 进行兜底。
  • 潜在风险:如果 kDefaultRootfs 定义不当(例如空字符串,或者指向一个不可信的路径),可能会导致兼容模式安装器在错误的根文件系统中执行提权操作,存在潜在的提权或文件破坏风险。
  • 建议:确保 kDefaultRootfs 是一个受信的、硬编码的绝对路径(如 /),并且在使用前验证其有效性。

4.2 授权绕过风险

  • 分析:移除 refreshOperatingPackageStatus(Pkg::Failed) 后,包的状态不会变成失败。
  • 潜在风险:需确保不存在某种自动化脚本或重试机制,会不断地在后台静默重试状态非 Failed 且非 Success 的包,这可能会频繁弹出授权弹窗,对用户造成骚扰或诱导用户误点授权。
  • 建议:确认 WorkerPrepare 状态下的包不会被自动重试,必须由用户主动点击触发。

综合改进建议代码示例

针对 singleinstallpage.cpp 中的改动,建议优化如下:

void SingleInstallPage::slotReinstall()
{
    // ... 前置代码 ...
    tr("Installing in compatibility mode %1").arg(m_pkgNameDescription));
    m_tipsLabel->setCustomDPalette(DPalette::TextLively);
    m_tipsLabel->setVisible(true);

    // Ensure targetRootfs is set when slotReinstall is called directly
    // (e.g. after auth cancel via afterGetAutherFalse), bypassing the
    // compat confirm flow where setData(targetRootfs) normally happens.
    const int firstRow = 0; // 避免魔法值,明确是单包安装的首行
    auto idx = m_packagesModel->index(firstRow);
    if (idx.isValid()) { // 确保索引有效,防止空模型导致越界
        if (idx.data(DebListModel::CompatibleTargetRootfsRole).toString().isEmpty()) {
            // kDefaultRootfs 必须是受控的安全路径
            m_packagesModel->setData(idx, kDefaultRootfs, AbstractPackageListModel::CompatibleTargetRootfsRole);
        }
    }
    // ... 后置代码 ...

    // 开始安装
    // 考虑将 slotInstallPackages 重命名为 installPackages,因为它现在有返回值参与逻辑判断
    if (!m_packagesModel->slotInstallPackages()) {
        Q_EMIT signalBacktoFileChooseWidget();
        return;
    }
}

总结:本次代码变更整体逻辑清晰,修复了授权取消后状态标记不当的问题,并补充了重装时缺失的参数。主要关注点在于确保 UI 对 signalAuthCancel 的响应能够正确重置界面,以及验证 kDefaultRootfs 的安全性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@pengfeixx pengfeixx merged commit 33372b2 into linuxdeepin:master May 26, 2026
20 of 22 checks passed
@pengfeixx pengfeixx deleted the fix/compat-mode-reinstall-after-auth-cancel branch May 26, 2026 06:13
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