-
Notifications
You must be signed in to change notification settings - Fork 91
fix: validate subpath format in dconfig #539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds validation to ensure subpaths follow the expected format (starting with "/") and adds debug logging for meta file loading in the dconfig subsystem.
Changes:
- Added validation in
subpathIsValid()to reject subpaths that don't start with "/" character - Added debug logging when meta files are successfully loaded to aid troubleshooting
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/dconfigfile.cpp
Outdated
| static const QRegularExpression regex("^/"); | ||
| if (!regex.match(subpath).hasMatch()) |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file is missing the required include for QRegularExpression. This will cause compilation errors. Add #include <QRegularExpression> at the top of the file with the other Qt includes (around line 22). Note that this issue also affects the existing uses of QRegularExpression at lines 55 and 62 in the file.
src/dconfigfile.cpp
Outdated
| static const QRegularExpression regex("^/"); | ||
| if (!regex.match(subpath).hasMatch()) |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a regular expression for a simple prefix check is inefficient. Replace the regex check with subpath.startsWith('/') which is simpler, more readable, and more performant. This would change lines 45-47 to simply: if (!subpath.startsWith('/')) return false;
| static const QRegularExpression regex("^/"); | |
| if (!regex.match(subpath).hasMatch()) | |
| if (!subpath.startsWith(QLatin1Char('/'))) |
src/dconfigfile.cpp
Outdated
| static const QRegularExpression regex("^/"); | ||
| if (!regex.match(subpath).hasMatch()) | ||
| return false; |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new validation logic requires subpaths to start with "/", but there's no test case covering a subpath that doesn't start with "/" (which should now fail). Consider adding test cases in ut_dconfigfile.cpp's setSubpath test to verify that subpaths like "a/b" (without leading "/") are rejected, as mentioned in the PR description.
1. Added validation to ensure subpath starts with "/" character 2. Added debug logging when loading meta files 3. The subpath validation prevents invalid relative paths from being processed 4. Debug logging helps track meta file loading for troubleshooting Log: Fixed subpath validation to ensure proper format Influence: 1. Test configuration loading with subpaths starting with "/" 2. Test configuration loading with subpaths not starting with "/" (should fail) 3. Verify debug logging appears when loading meta files 4. Test empty subpath handling (should still work) 5. Test various valid subpath formats to ensure they pass validation fix: 修复dconfig中subpath格式验证 1. 添加验证确保subpath以"/"字符开头 2. 添加加载元文件时的调试日志记录 3. subpath验证防止处理无效的相对路径 4. 调试日志有助于跟踪元文件加载过程以进行故障排除 Log: 修复subpath验证以确保正确的格式 Influence: 1. 测试以"/"开头的subpath的配置加载 2. 测试不以"/"开头的subpath的配置加载(应该失败) 3. 验证加载元文件时是否出现调试日志 4. 测试空subpath处理(应该仍然有效) 5. 测试各种有效的subpath格式以确保它们通过验证
deepin pr auto review我来对这段代码进行审查,主要从以下几个方面分析:
if (!subpath.startsWith(QLatin1Char('/')))
return false;这个改动是合理的,强制要求子路径必须以'/'开头,可以统一路径格式,避免路径解析的歧义。 测试用例的补充也很完整: {
DConfigFile config(APP_ID, FILE_NAME, "a/b");
ASSERT_FALSE(config.load(LocalPrefix));
}
{
DConfigFile config(APP_ID, FILE_NAME, "/a/b");
ASSERT_TRUE(config.load(LocalPrefix));
}分别测试了不以'/'开头和以'/'开头的情况。
建议改进:
建议改进:
if (subpath.contains("..")) {
return false;
}
QFileInfo fileInfo(subpath);
QString canonicalPath = fileInfo.canonicalFilePath();
qCInfo(cfLog, "Load meta file: \"%s\"", qPrintable(path));总结:
|
|
[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 |
processed
Log: Fixed subpath validation to ensure proper format
Influence:
"/" (should fail)
fix: 修复dconfig中subpath格式验证
Log: 修复subpath验证以确保正确的格式
Influence: