Skip to content

fix: parse install hook lines with proper quote handling#1695

Merged
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_hooks
Jun 25, 2026
Merged

fix: parse install hook lines with proper quote handling#1695
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_hooks

Conversation

@reddevillg

Copy link
Copy Markdown
Collaborator

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces trim_left and trim_right string helper functions, along with their respective unit tests. It also refactors the install hook parsing logic by introducing a robust parseInstallHookCommandLine function that correctly handles quoted commands and trailing whitespaces, accompanied by comprehensive unit tests. The reviewer suggested optimizing the hook parsing loop by breaking early once a matching command is found, which avoids redundant parsing attempts for other prefixes on the same line.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread libs/utils/src/linglong/utils/hooks.cpp
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.10526% with 22 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
libs/utils/src/linglong/utils/hooks.cpp 26.66% 14 Missing and 8 partials ⚠️
Files with missing lines Coverage Δ
libs/common/src/linglong/common/strings.cpp 97.43% <100.00%> (+0.13%) ⬆️
libs/common/src/linglong/common/strings.h 100.00% <ø> (ø)
libs/utils/src/linglong/utils/hooks.h 0.00% <ø> (ø)
libs/utils/src/linglong/utils/hooks.cpp 9.87% <26.66%> (+9.87%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: reddevillg <reddevillg@gmail.com>
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

★ 总体评分:99分

■ 【总体评价】

代码重构了安装钩子解析逻辑,彻底修复了注释误执行和单行截断的严重缺陷,质量优异
逻辑严密且测试完备,仅因错误提示信息不够精确扣1分

■ 【详细分析】

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

parseInstallHookCommandLine 函数通过 starts_with 严格匹配前缀,通过 trim_left/trim_right 与引号检查剥离外层引号,逻辑严密。测试用例充分覆盖了无引号、单双引号、内部引号、未闭合引号等边界情况。
潜在问题:当输入为 ll-post-install="echo ok"; 时,报错信息为 "unterminated quoted command",实际上引号已闭合,错误在于尾部存在非法字符,该提示具有误导性。
建议:将针对外部带分号等非法尾部字符的报错信息修改为 "trailing characters after quoted command" 以提高可调试性。

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

将原先臃肿的 parseInstallHooks 循环体抽离为职责单一的 parseInstallHookCommandLine 函数,符合单一职责原则。使用 std::array 替代多个 if-else 分支,配合结构化绑定使代码更加简洁。新增的单元测试覆盖了所有核心分支。
建议:无

  • 3.代码性能(良好)✓

广泛使用 std::string_view 及其原位操作(remove_prefixremove_suffixsubstr),避免了不必要的字符串内存分配与拷贝。文件遍历从提前退出的 O(N) 变为完整遍历,属于修复 Bug 的必要开销。
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
新代码使用 starts_with 替代 find,彻底消除了原代码中注释行(如 # ll-post-install=...)被误匹配并执行的高危漏洞。同时,对带引号的命令严格校验闭合状态并拒绝外部追加的分号,有效防止了命令注入绕过。
建议:无

■ 【改进建议代码示例】

     command.remove_prefix(1);
 
     auto suffix = common::strings::trim_right(command, WHITESPACE_CHARS);
     if (suffix.empty() || suffix.back() != quote) {
-        return LINGLONG_ERR("Invalid install hook command: unterminated quoted command");
+        if (!suffix.empty()) {
+            return LINGLONG_ERR("Invalid install hook command: trailing characters after quoted command");
+        }
+        return LINGLONG_ERR("Invalid install hook command: unterminated quoted command");
     }
 
     suffix.remove_suffix(1);

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

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

@dengbo11 dengbo11 merged commit 9f1400f into OpenAtom-Linyaps:master Jun 25, 2026
18 checks passed
@reddevillg reddevillg deleted the fix_hooks branch June 25, 2026 05:23
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