Skip to content

fix: update QProcess command syntax for package management#603

Open
dengzhongyuan365-dev wants to merge 1 commit intolinuxdeepin:release/eaglefrom
dengzhongyuan365-dev:release/eagle
Open

fix: update QProcess command syntax for package management#603
dengzhongyuan365-dev wants to merge 1 commit intolinuxdeepin:release/eaglefrom
dengzhongyuan365-dev:release/eagle

Conversation

@dengzhongyuan365-dev
Copy link
Contributor

@dengzhongyuan365-dev dengzhongyuan365-dev commented Feb 3, 2026

Refactor the way QProcess commands are constructed for package management functions in PageInfo, DriverManager, and Utils. This change improves readability and consistency by using QStringList for command arguments instead of string concatenation.

Summary by Sourcery

Use QProcess with separate program and argument list for package management commands to improve robustness and consistency across device manager components.

Enhancements:

  • Update printer driver installation, removal, and status checks to invoke dpkg and apt via QProcess arguments instead of concatenated command strings.
  • Align package installation checks in PageInfo with the new QProcess argument-based invocation pattern.
  • Adjust utility package removal to use QProcess with explicit apt arguments rather than formatted shell command strings.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev

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

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 3, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors QProcess invocations for package management operations to use the structured overload with a program name and QStringList of arguments instead of building shell command strings, improving safety, consistency, and readability across DriverManager, PageInfo, and Utils.

File-Level Changes

Change Details Files
Refactor DriverManager package management QProcess calls to use program-plus-arguments syntax instead of concatenated command strings.
  • Update printerHasInstalled to start QProcess with "sudo" as the program and a QStringList of dpkg arguments including the package name.
  • Update installPrinter to invoke apt install via QProcess using separate program and arguments rather than a single concatenated command string.
  • Update unInstallPrinter to invoke dpkg -r via QProcess with separate arguments, mirroring the new style used elsewhere.
deepin-devicemanager-server/deepin-devicecontrol/src/drivercontrol/drivermanager.cpp
Align PageInfo package installation check with new QProcess argument-based invocation style.
  • Change packageHasInstalled to start QProcess with "dpkg" as the program and a QStringList containing "-s" and the package name instead of constructing a single command string.
deepin-devicemanager/src/Page/PageInfo.cpp
Refactor Utils unInstallPackage QProcess invocation to use program and argument list instead of formatted shell command string.
  • Change unInstallPackage to call process.start with "apt" as the program and a QStringList containing "remove" and the package name, replacing the previous QString-based shell command.
deepin-devicemanager-server/deepin-devicecontrol/src/drivercontrol/utils.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Refactor the way QProcess commands are constructed for package management functions in PageInfo, DriverManager, and Utils. This change improves readability and consistency by using QStringList for command arguments instead of string concatenation.

https://pms.uniontech.com/task-view-386519.html
Copy link

@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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要是将 QProcess::start() 的调用方式从字符串拼接改为列表形式,这是一个很好的改进。以下是对这段代码的详细审查和改进建议:

1. 代码改进点分析

语法逻辑:✅ 改进正确

  • 原代码使用字符串拼接命令(如 "sudo dpkg -s " + packageName),这种方式容易受到命令注入攻击,尤其是当 packageName 来自于不可信的输入时。
  • 修改后的代码使用 QStringList 传递参数,QProcess 会自动处理参数转义,避免了命令注入的风险。

代码质量:✅ 改进良好

  • 修改后的代码更符合 Qt 的最佳实践,提高了代码的可读性和安全性。
  • 将命令和参数分开,使得意图更加清晰。

代码性能⚠️ 无明显影响

  • QProcess 在内部处理参数列表时会有轻微的开销,但相比字符串拼接带来的安全风险,这个开销是可以接受的。

代码安全:✅ 显著提升

  • 修改后的代码有效防止了命令注入攻击,这是最重要的改进点。

2. 进一步改进建议

尽管修改已经提高了安全性,但还有一些地方可以进一步优化:

a. 错误处理改进

当前代码没有检查进程的退出状态或错误输出,建议添加错误处理:

bool DriverManager::printerHasInstalled(const QString &packageName)
{
    QProcess p;
    p.start("sudo", QStringList() << "dpkg" << "-s" << packageName);
    if (!p.waitForFinished(-1)) {
        qWarning() << "dpkg command failed to execute:" << p.errorString();
        return false;
    }
    
    if (p.exitCode() != 0) {
        qWarning() << "dpkg command failed with exit code:" << p.exitCode();
        return false;
    }

    QByteArray r = p.readAllStandardOutput();
    return r.contains("Status: install ok installed");
}

b. 避免使用 sudo

在服务器端代码中使用 sudo 可能会带来安全风险,建议:

  1. 如果程序需要以 root 权限运行,整个程序应该以 root 身份运行
  2. 或者使用 polkit 等机制进行权限管理
// 假设程序已经以 root 权限运行
bool DriverManager::printerHasInstalled(const QString &packageName)
{
    QProcess p;
    p.start("dpkg", QStringList() << "-s" << packageName);
    // ... 其余代码
}

c. 输入验证

packageName 进行验证,确保它符合包名规范:

bool isValidPackageName(const QString &packageName) {
    // 简单的包名验证,实际可能需要更复杂的正则表达式
    QRegularExpression re("^[a-z0-9][a-z0-9+.-]*$");
    return re.match(packageName).hasMatch();
}

bool DriverManager::printerHasInstalled(const QString &packageName)
{
    if (!isValidPackageName(packageName)) {
        qWarning() << "Invalid package name:" << packageName;
        return false;
    }
    // ... 其余代码
}

d. 超时处理

使用 waitForFinished(-1) 会无限等待,建议设置合理的超时时间:

bool DriverManager::printerHasInstalled(const QString &packageName)
{
    QProcess p;
    p.start("dpkg", QStringList() << "-s" << packageName);
    if (!p.waitForFinished(30000)) { // 30秒超时
        qWarning() << "dpkg command timed out";
        return false;
    }
    // ... 其余代码
}

e. 异步处理

对于耗时操作(如安装/卸载软件包),建议使用异步方式处理,避免阻塞主线程:

void DriverManager::installPrinterAsync(const QString &packageName)
{
    QProcess *p = new QProcess(this);
    connect(p, QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
            [this, packageName, p](int exitCode, QProcess::ExitStatus exitStatus) {
        bool success = (exitCode == 0) && printerHasInstalled(packageName);
        emit installFinished(success);
        p->deleteLater();
    });
    p->start("apt-get", QStringList() << "install" << "-y" << packageName);
}

3. 其他建议

  1. 使用 apt-get 而不是 apt
    在脚本和程序化调用中,推荐使用 apt-get 而不是 apt,因为 apt-get 的输出更稳定,更适合脚本解析。

  2. 添加 -y 参数
    对于 apt install 命令,建议添加 -y 参数自动确认安装,避免交互式提示。

  3. 日志记录
    建议添加详细的日志记录,便于调试和问题追踪。

  4. 资源清理
    确保所有 QProcess 对象都被正确清理,避免资源泄漏。

总结

这段代码的修改是一个很好的安全改进,将命令参数从字符串拼接改为列表传递。建议在此基础上进一步改进错误处理、输入验证和异步处理等方面,以提高代码的健壮性和可靠性。

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.

2 participants