fix: fix config reload abort on file update failure and uppercase path matching#148
Conversation
There was a problem hiding this comment.
Sorry @18202781743, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
38ed8bd to
98dfcda
Compare
failure 1. Updated regex patterns in dconfig_global.h to accept uppercase letters (A-Z) in config file paths, fixing parsing failures for files with uppercase names 2. Refactored update() by extracting updateInternal() that returns error messages instead of early-aborting via sendErrorReply on DBus 3. Modified reload() to continue processing remaining files when individual updates fail, instead of aborting the entire reload 4. Enhanced reload() to collect and report total failed file count for better debugging Log: Fixed config file matching for uppercase names and improved reload robustness Influence: 1. Verify config files with uppercase names (e.g., MyConfig.json) are correctly parsed 2. Test scenario: multiple files to update, one file update fails, verify reload continues processing remaining files 3. Verify error count is correctly reported in logs when partial reload failures occur 4. Test DBus update calls still report errors correctly via sendErrorReply 5. Verify no regression for existing lowercase-path config files 6. Test with mixed case paths in config directories fix: 支持大写文件名称并防止重新加载时因部分失败而终止 1. 修改了 dconfig_global.h 中的正则表达式,增加了对大写字母 (A-Z) 的支 持,修复了含有大写字母的配置文件路径解析失败的问题 2. 重构了 update() 方法,提取出 updateInternal() 返回错误信息,而不是通 过 sendErrorReply 提前终止 DBus 调用 3. 修改 reload() 方法,当个别文件更新失败时继续处理剩余文件,而不是整个 重载过程提前终止 4. 增强 reload() 日志,收集并报告失败的文件数量,方便调试 Log: 修复了配置文件名大写匹配问题,增强了重载的鲁棒性 Influence: 1. 验证包含大写的配置文件(如 MyConfig.json)能够正确解析 2. 测试多文件更新场景:其中一个文件更新失败,验证重载继续处理剩余文件 3. 验证部分重载失败时日志中正确报告错误数量 4. 测试 DBus 更新调用仍通过 sendErrorReply 正确报告错误 5. 验证现有小写路径配置文件无回归问题 6. 测试配置目录中混合大小写路径的情况 PMS: BUG-360197 Change-Id: I054dd047b2924d6475b357ec1a83643048e7e2ea
deepin pr auto review你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改主要涉及三个方面:正则表达式的优化与重构、业务逻辑与DBus通信的解耦(提取 整体来看,这次修改的方向非常好,提高了代码的内聚性,并修复了正则表达式的一些潜在问题。以下是针对语法逻辑、代码质量、代码性能和代码安全的详细审查意见及改进建议: 一、 语法与逻辑1. 正则表达式:原子组与捕获组的混用问题
2. 正则表达式:转义字符的冗余与潜在风险
3. 正则表达式:大小写扩展的覆盖范围
二、 代码质量1. 解耦与职责分离(优秀)
2.
三、 代码性能1. 正则表达式回溯控制(优秀)
2.
四、 代码安全1. 路径遍历攻击防范
2. ReDoS 防范(已解决)
综合修改建议代码示例结合以上审查意见,我为你优化了 // dconfig_global.h
inline ConfigureId getMetaConfigureId(const QString &path)
{
ConfigureId info;
// /usr/share/dsg/configs/[$appid]/[$subpath]/$resource.json
// 使用 (?i) 忽略大小写,使用占有量词 ++ 防止回溯,将 - 放在字符类末尾避免混乱转义
static QRegularExpression usrReg(
R"((?i)/configs/(?!overrides/)(?<appid>[a-z0-9\s_@^!#$%&.+-]++\/)?(?<subpath>(?:[a-z0-9\s_@^!#$%&.+-]++\/)*)(?<resource>[a-z0-9\s_@^!#$%&.+-]++)\.json$)"
);
QRegularExpressionMatch match;
match = usrReg.match(path);
// ... 后续逻辑保持不变
}
inline ConfigureId getOverrideConfigureId(const QString &path)
{
ConfigureId info;
// /usr/share/dsg/configs/overrides/[$appid]/$resource/[$subpath]/$override_id.json
static QRegularExpression usrReg(
R"((?i)/configs/overrides/(?<appid>[a-z0-9\s_@^!#$%&.+-]++\/)?(?<resource>[a-z0-9\s_@^!#$%&.+-]++)/(?<subpath>(?:[a-z0-9\s_@^!#$%&.+-]++\/)*)(?<configurationid>[a-z0-9\s_@^!#$%&.+-]++)\.json$)"
);
// /etc/dsg/configs/overrides/[$appid]/$resource/[$subpath]/$override_id.json
static QRegularExpression etcReg(
R"((?i)^/etc/dsg/configs/overrides/(?<appid>[a-z0-9\s_@^!#$%&.+-]++\/)?(?<resource>[a-z0-9\s_@^!#$%&.+-]++)/(?<subpath>(?:[a-z0-9\s_@^!#$%&.+-]++\/)*)(?<configurationid>[a-z0-9\s_@^!#$%&.+-]++)\.json$)"
);
QRegularExpressionMatch match;
match = usrReg.match(path);
// ... 后续逻辑保持不变
}// dconfigserver.cpp
void DSGConfigServer::update(const QString &path)
{
const auto errorMsg = updateInternal(path);
if (errorMsg) {
const QString& errMsg = *errorMsg; // 使用常量引用避免多余的QString深拷贝
qWarning() << errMsg;
if (calledFromDBus()) {
sendErrorReply(QDBusError::Failed, errMsg);
}
}
}总结:本次Diff整体质量很高,逻辑解耦十分合理,性能与安全性均有提升。只需稍微打磨一下正则表达式的书写规范,并注意防范路径跳转漏洞,代码就会非常健壮。希望这些意见对你有所帮助! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
update fails, instead of aborting early
DBus error, allowing reload to handle failures gracefully
configuration paths, as file systems may use mixed case
results
Log: Fixed reload aborting on file update failure and added uppercase
path support
Influence:
remaining files still process
matching
updates
fix: 修复配置重载在文件更新失败时提前终止以及大写路径匹配错误的问题
终止
reload 能优雅处理失败
用混合大小写
Log: 修复重载在文件更新失败时提前终止的问题,并增加大写路径支持
Influence:
PMS: BUG-360197