Skip to content

refactor(launch): drop QUrl intermediary and unused workingDir param#375

Open
ComixHe wants to merge 1 commit into
linuxdeepin:masterfrom
ComixHe:master
Open

refactor(launch): drop QUrl intermediary and unused workingDir param#375
ComixHe wants to merge 1 commit into
linuxdeepin:masterfrom
ComixHe:master

Conversation

@ComixHe

@ComixHe ComixHe commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Simplify Launch file-args handling by parsing QUrl inline from QStringList instead of a separate QList conversion. Removes unused workingDir parameter from processExec — working directory is resolved upstream via generateCommand.

Pms: BUG-368227 BUG-370055

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Please try again later or upgrade to continue using Sourcery

@ComixHe ComixHe requested a review from BLumia July 16, 2026 08:54
BLumia
BLumia previously approved these changes Jul 16, 2026
Simplify Launch file-args handling by parsing QUrl inline from
QStringList instead of a separate QList<QUrl> conversion.
Removes unused workingDir parameter from processExec — working directory
is resolved upstream via generateCommand.

Pms: BUG-368227 BUG-370055

Signed-off-by: Yuming He <heyuming@deepin.org>
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:55分

■ 【总体评价】

代码重构简化了资源类型处理并增加了URL有效性校验,但移除工作目录相对路径检查引入了安全隐患。
逻辑正确但因工作目录路径控制不当扣分,受中危漏洞上限限制总分降至55分。

■ 【详细分析】

  • 1.语法逻辑(正确)✓

代码在 ApplicationService::Launch 中使用迭代器安全地删除 formattedRes 中的元素,并正确调整了 task.argNumprocessExec 函数签名修改一致,类型转换逻辑清晰。
潜在问题:无
建议:无

  • 2.代码质量(良好)✓

将资源类型从 QList<QUrl> 统一改为 QStringList,减少了类型转换的复杂性,代码可读性提升。注释和警告信息清晰。
潜在问题:无
建议:无

  • 3.代码性能(无性能问题)✓

使用 erase 方法在遍历时删除元素,时间复杂度为 O(N),且预先调用了 reserve,性能表现良好。
潜在问题:无
建议:无

  • 4.代码安全(存在 1 个安全漏洞)✕

漏洞对比统计:新增漏洞 1 个,减少漏洞 0 个,持平 0 个
移除工作目录的相对路径检查可能导致进程在非预期的目录下启动,增加被劫持的风险。

  • 安全漏洞1(无):工作目录路径控制不当 在 ApplicationService::Launch 中,移除了对 workingDir 相对路径的检查,直接将其放入 optionsMap 传递给底层执行器,攻击者可通过传入相对路径导致进程以非预期的当前工作目录启动,可能引发配置文件或动态库劫持 ——非常重要

  • 建议:在设置 optionsMap["path"] 前,恢复对相对路径的检查,或使用 QDir::absolutePath() 将其转换为绝对路径。

■ 【改进建议代码示例】

diff --git a/src/dbus/applicationservice.cpp b/src/dbus/applicationservice.cpp
index e960a641..ce2ce5cf 100644
--- a/src/dbus/applicationservice.cpp
+++ b/src/dbus/applicationservice.cpp
@@ -492,13 +492,12 @@ QDBusObjectPath ApplicationService::Launch(const QString &action, const QStringL
         workingDir = std::move(optionPath);
     }
 
-    if (!workingDir.isEmpty() && QDir::isRelativePath(workingDir)) {
-        qWarning() << "relative working dir is not supported: " << workingDir;
-        workingDir.clear();
-    }
+    if (!workingDir.isEmpty() && QDir::isRelativePath(workingDir)) {
+        workingDir = QDir::absolutePath(workingDir); // 转换为绝对路径
+    }
 
-    auto cmds = generateCommand(optionsMap);
-    auto task = processExec(execStr, fields, workingDir);
+    optionsMap["path"] = workingDir;
+
+    auto cmds = generateCommand(optionsMap);
+    auto task = processExec(execStr, fields);
     if (!task) {
         safe_sendErrorReply(QDBusError::InternalError, "Invalid Command.");
         return {};

@deepin-ci-robot

Copy link
Copy Markdown

@ComixHe: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
github-pr-review-ci d6b5333 link true /test github-pr-review-ci

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, ComixHe

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

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