fix: auto-disable XDP for non-conforming apps#1662
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: reddevillg The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request introduces a validation mechanism for XDG Desktop Portal IDs (app IDs) within the CLI. It adds a new utility function isValidXdgDesktopPortalId in libs/utils/src/linglong/utils/xdp.h and integrates it into the runResolvedContext flow to automatically disable XDP integration if the ID is invalid. Comprehensive unit tests for the validation logic were also added. The review feedback suggests replacing std::isalnum with explicit ASCII range checks to avoid locale-dependent behavior and potential undefined behavior when handling signed characters.
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
Disable XDP integration when the appid does not conform xdp id Signed-off-by: reddevillg <reddevillg@gmail.com>
b79cce5 to
955b29b
Compare
deepin pr auto review这是一份针对您提供的 Git Diff 的代码审查报告。本次代码变更的主要目的是在运行容器前校验应用 ID(appid)是否符合 XDG Desktop Portal (XDP) 规范,如果不符合则发出警告并自动禁用 XDP 集成,同时提供了相应的工具函数和单元测试。 整体来看,代码逻辑清晰,测试覆盖较为全面,但存在一些性能隐患、潜在的逻辑漏洞以及代码细节可以优化。以下是详细的审查意见: 1. 代码性能问题:不必要的堆内存分配 改进意见: inline bool isValidXdgDesktopPortalId(std::string_view appid) noexcept
{
if (appid.empty()) {
return false;
}
size_t segment_count = 1;
for (char c : appid) {
if (c == '.') {
++segment_count;
}
}
if (segment_count < 2) {
return false;
}
size_t start = 0;
size_t segment_index = 0;
for (size_t i = 0; i <= appid.size(); ++i) {
// 遇到分隔符或到达末尾时处理分段
if (i == appid.size() || appid[i] == '.') {
size_t len = i - start;
if (len == 0) {
return false; // 空分段 (如 "org..app" 或末尾的 ".")
}
std::string_view segment = appid.substr(start, len);
bool is_last = (segment_index == segment_count - 1);
bool valid = std::all_of(segment.begin(), segment.end(), [is_last](char c) {
if (std::isalnum(static_cast<unsigned char>(c)) || c == '_') {
return true;
}
// 只有最后一个分段允许连字符 '-'
return is_last && c == '-';
});
if (!valid) {
return false;
}
start = i + 1;
++segment_index;
}
}
return true;
}2. 语法与逻辑问题:XDP 规范对分段首字符的限制被忽略 改进意见: // 在校验 segment 的 lambda 或循环中添加:
if (!std::isalpha(static_cast<unsigned char>(segment[0]))) {
return false; // 分段必须以字母开头
}(注意:如果修改此逻辑,请同步更新单元测试,将 3. 代码质量问题: 改进意见: inline bool isAlnum(char c) {
unsigned char uc = static_cast<unsigned char>(c);
return (uc >= '0' && uc <= '9') || (uc >= 'A' && uc <= 'Z') || (uc >= 'a' && uc <= 'z');
}这样可以移除对 4. 代码安全与业务逻辑问题: if (!options.disableXdp.has_value() && !utils::isValidXdgDesktopPortalId(appid)) {
LogW("appid '{}' doesn't conform to XDP ID specification, disabling XDP integration. "
"Use --enable-xdp to override.",
appid);
runOptions.disableXdp = true;
}这里判断
改进意见: 建议在代码审查时确认 CLI 参数绑定代码,确保以下逻辑映射:
5. 测试用例问题:版权年份与边界测试
总结代码的核心逻辑和意图是好的,通过前置校验避免了运行时的隐式错误。建议优先处理性能上的 |
Disable XDP integration when the appid does not conform xdp id