Repository navigation
Conversation
1. Root cause: LibzipPlugin::updateArchiveData() ignored the options parameter, cleared all memory data, and re-read from disk via zip_open() with commented-out error handling on failure 2. Fix: rewrite updateArchiveData() to use the options parameter for in-memory updates (Delete/Rename/Add), mirroring LibarchivePlugin::updateArchiveData(), eliminating disk re-read fragility and restoring proper error handling 3. Impact: zip format archives now refresh UI immediately after rename/delete/add operations without reopening, consistent with tar format behavior; no signature change, single caller via virtual dispatch unaffected Influence: 1. Test zip archive file rename, verify UI shows new name immediately 2. Test zip archive file/folder delete, verify UI updates instantly 3. Test zip archive file add, verify new entry appears without reopen 4. Test multi-level path folder rename/delete in zip archives 5. Verify tar format operations remain unaffected fix: 修复zip格式压缩包重命名后需重新打开才显示的问题 1. 根因：LibzipPlugin::updateArchiveData() 忽略 options 参数，清空 全部内存数据后通过 zip_open() 从磁盘重读，且 zip_open 失败时 错误处理被注释掉，导致内存数据为空或过时，UI 无法正确刷新 2. 方案：参照 LibarchivePlugin::updateArchiveData() 实现，改用 options 参数做内存级更新（Delete/Rename/Add），消除磁盘重读 脆弱性，恢复错误处理 3. 影响：zip 格式压缩包重命名/删除/追加后 UI 即时刷新，与 tar 格式行为一致；函数签名不变，单一调用方通过虚函数分派不受影响 Influence: 1. 测试zip压缩包内文件重命名，验证UI即时显示新名称 2. 测试zip压缩包内文件/文件夹删除，验证UI即时更新 3. 测试zip压缩包内文件追加，验证新条目无需重开即可显示 4. 测试多层路径文件夹的重命名/删除操作 5. 验证tar格式操作不受影响 PMS: BUG-232441
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000 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 |
Reviewer's GuideZip archive metadata is now updated in memory for delete, rename, and add operations, matching libarchive behavior and avoiding fragile disk re-reads that could leave the UI with stale or empty data. Sequence diagram for in-memory zip archive updatessequenceDiagram
participant Operation as ArchiveOperation
participant Plugin as LibzipPlugin
participant Data as ArchiveData
participant UI as ArchiveUI
Operation->>Plugin: updateArchiveData(options)
Plugin->>Data: Apply Delete, Rename, or Add
Data-->>Plugin: Updated entries and sizes
Plugin->>Data: QFileInfo(m_strArchiveName).size()
Plugin-->>UI: PFT_Nomral
UI->>Data: Refresh archive listing
Flow diagram for zip metadata update operationsflowchart TD
A["updateArchiveData(options)"] --> B{"options.eType"}
B -->|Delete| C["Remove entries and update qSize/listRootEntry"]
B -->|Rename| D["Rename mapFileEntry and listRootEntry entries"]
B -->|Add| E["Insert entry and update qSize/listRootEntry"]
C --> F["Update qComressSize"]
D --> F
E --> F
F --> G["Return PFT_Nomral"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="3rdparty/libzipplugin/libzipplugin.cpp" line_range="647-657" />
<code_context>
+ }
+ }
+ } else { // 重命名文件
+ stArchiveData.mapFileEntry.remove(entry.strFullPath); //在map中重命名该文件
+ QString strPath = QFileInfo(entry.strFullPath).path();
+ if(strPath == "." || strPath.isEmpty() || strPath.isNull()) {
+ strAlias = entry.strAlias;
+ } else {
+ strAlias = strPath + QDir::separator() + entry.strAlias;
+ }
+ FileEntry tmpEntry = entry;
+ tmpEntry.strFullPath = strAlias;
+ stArchiveData.mapFileEntry.insert(strAlias, tmpEntry);
+ // 更新文件夹第一层的数据
+ if (!entry.strFullPath.contains(QLatin1Char('/'))) {
+ for (int i = 0; i < stArchiveData.listRootEntry.count(); i++) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Renaming a file inside a folder inserts the renamed entry into `mapFileEntry` with its new `strFullPath` but leaves `strFileName` copied from the old `entry`. The nested archive view therefore displays the old filename after the rename until the archive is reopened and rebuilt from disk.
**Triggers:** When a file being renamed is not at the archive root.
**Suggested fix:** Set `tmpEntry.strFileName` to `QFileInfo(strAlias).fileName()` before inserting the renamed entry.
</issue_to_address>
### Comment 2
<location path="3rdparty/libzipplugin/libzipplugin.cpp" line_range="688" />
<code_context>
-// m_bAllEntry = true;
-// return minizip_list();
- }
+ // 判断是否追加到第一层数据
+ if (destinationPath.isEmpty() && ((entry.strFullPath.count('/') == 1 && entry.strFullPath.endsWith('/')) || entry.strFullPath.count('/') == 0)) {
+ for (int i = 0; i < stArchiveData.listRootEntry.count(); i++) {
+ if (stArchiveData.listRootEntry.at(i).strFullPath == entry.strFullPath) { // 在第一层数据中找到entry，不添加数据
+ stArchiveData.listRootEntry.removeAt(i);
+ break;
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** The Add path uses the literal `'/'` when deciding whether an entry belongs in `listRootEntry`, while archive paths are normalized with `QDir::separator()`. On Windows, nested entries use backslashes and `count('/')` is zero, so every added nested file or directory is treated as a root entry and appears at the wrong archive level in the refreshed view.
**Triggers:** When adding a folder tree to a ZIP archive on Windows with an empty destination path.
**Suggested fix:** Use `QDir::separator()` consistently, or use a platform-independent archive path separator when counting and testing archive paths.
```suggestion
if (destinationPath.isEmpty() && ((entry.strFullPath.count(QDir::separator()) == 1 && entry.strFullPath.endsWith(QDir::separator())) || entry.strFullPath.count(QDir::separator()) == 0)) {
```
</issue_to_address>
Comment on lines
+647
to
+657
| stArchiveData.mapFileEntry.remove(entry.strFullPath); //在map中重命名该文件 | ||
| QString strPath = QFileInfo(entry.strFullPath).path(); | ||
| if(strPath == "." || strPath.isEmpty() || strPath.isNull()) { | ||
| strAlias = entry.strAlias; | ||
| } else { | ||
| strAlias = strPath + QDir::separator() + entry.strAlias; | ||
| } | ||
| FileEntry tmpEntry = entry; | ||
| tmpEntry.strFullPath = strAlias; | ||
| stArchiveData.mapFileEntry.insert(strAlias, tmpEntry); | ||
| // 更新文件夹第一层的数据 |
There was a problem hiding this comment.
issue (bug_risk): Renaming a file inside a folder inserts the renamed entry into mapFileEntry with its new strFullPath but leaves strFileName copied from the old entry. The nested archive view therefore displays the old filename after the rename until the archive is reopened and rebuilt from disk.
Triggers: When a file being renamed is not at the archive root.
Suggested fix: Set tmpEntry.strFileName to QFileInfo(strAlias).fileName() before inserting the renamed entry.
| // return minizip_list(); | ||
| } | ||
| // 判断是否追加到第一层数据 | ||
| if (destinationPath.isEmpty() && ((entry.strFullPath.count('/') == 1 && entry.strFullPath.endsWith('/')) || entry.strFullPath.count('/') == 0)) { |
There was a problem hiding this comment.
issue (bug_risk): The Add path uses the literal '/' when deciding whether an entry belongs in listRootEntry, while archive paths are normalized with QDir::separator(). On Windows, nested entries use backslashes and count('/') is zero, so every added nested file or directory is treated as a root entry and appears at the wrong archive level in the refreshed view.
Triggers: When adding a folder tree to a ZIP archive on Windows with an empty destination path.
Suggested fix: Use QDir::separator() consistently, or use a platform-independent archive path separator when counting and testing archive paths.
Suggested change
| if (destinationPath.isEmpty() && ((entry.strFullPath.count('/') == 1 && entry.strFullPath.endsWith('/')) || entry.strFullPath.count('/') == 0)) { | |
| if (destinationPath.isEmpty() && ((entry.strFullPath.count(QDir::separator()) == 1 && entry.strFullPath.endsWith(QDir::separator())) || entry.strFullPath.count(QDir::separator()) == 0)) { |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 良好 ✅ 通过 潜在问题：
建议： 在文件重命名的map插入前补充 tmpEntry.strFileName = entry.strAlias; 保持与目录重命名场景一致 2. 代码质量 ✅评价： 良好 ✅ 通过 潜在问题：
建议： 抽取 updateRootEntry(ArchiveData&, const FileEntry&, const QString&) 辅助函数消除重复代码；将 Delete/Rename/Add 操作拆分为独立私有函数提升可读性 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 从磁盘I/O（zip_open/zip_close）改为纯内存操作，性能显著提升，算法复杂度合理 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 代码仅操作内存数据结构（QMap/QList），无外部I/O或用户输入处理，无安全风险 💡 改进建议代码示例// 建议修复：文件重命名时补充 strFileName 更新
// 位置：updateArchiveData() 文件重命名分支，约行115
} else { // 重命名文件
stArchiveData.mapFileEntry.remove(entry.strFullPath);
QString strPath = QFileInfo(entry.strFullPath).path();
if(strPath == "." || strPath.isEmpty() || strPath.isNull()) {
strAlias = entry.strAlias;
} else {
strAlias = strPath + QDir::separator() + entry.strAlias;
}
FileEntry tmpEntry = entry;
tmpEntry.strFullPath = strAlias;
tmpEntry.strFileName = entry.strAlias; // 补充：更新文件名
stArchiveData.mapFileEntry.insert(strAlias, tmpEntry);
// ... 后续根条目更新逻辑
}
// 建议优化：抽取根条目更新辅助函数
void LibzipPlugin::updateRootEntry(ArchiveData &data, const QString &oldPath,
const FileEntry &newEntry)
{
for (int i = 0; i < data.listRootEntry.count(); i++) {
if (data.listRootEntry.at(i).strFullPath == oldPath) {
data.listRootEntry.removeAt(i);
data.listRootEntry.append(newEntry);
break;
}
}
}本报告由 AI 代码审查工具自动生成 |
Contributor
|
TAG Bot New tag: 6.5.35 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause Analysis
LibzipPlugin::updateArchiveData()ignored theoptionsparameter (parameter name commented out as/*options*/), cleared all in-memory archive data viaresetArchiveData(), then re-read from disk usingzip_open(). Thezip_open()failure handler was commented out, so when re-reading failed, the function silently returnedPFT_Nomral(success) with empty/stale data, causing the UI to not refresh after rename operations. In contrast,LibarchivePlugin::updateArchiveData()uses theoptionsparameter to update entries in-memory, which is why tar format refreshes correctly.Key evidence:
3rdparty/libzipplugin/libzipplugin.cpp:556-589(defective implementation) vs3rdparty/libarchive/libarchive/libarchiveplugin.cpp:576-700(correct reference implementation).Fix Approach
Rewrote
LibzipPlugin::updateArchiveData()to mirrorLibarchivePlugin::updateArchiveData()— using theoptionsparameter for in-memory updates across Delete, Rename, and Add operation types. This eliminates the disk re-read fragility entirely and naturally restores proper error handling (no morezip_open()path with commented-out failure handling). Only3rdparty/libzipplugin/libzipplugin.cppis modified, 1 file changed, 141 insertions(+), 24 deletions(-).Change Safety Assessment
Code Safety
zip_openerror handling) was not a historical bug fix product — it was unfinished early development code. This change does not revert any historical fix.singlejob.cpp:545invokes via virtual dispatch; function signature unchanged, return type unchanged. Related PMS bugs (365277, 356233, 353985) are all security/symlink fixes, unrelated to this change.Business Impact Scope
Affects zip format archive file management: rename, delete, and add operations on files inside zip archives. After this fix, zip archives will refresh UI immediately after these operations, consistent with tar format behavior. tar/tar.gz and other formats are unaffected (only libzip plugin modified).
Verification Suggestion
Focus regression testing on zip format: rename/delete/add for both files and multi-level folders. Confirm tar format operations remain unaffected.
根因分析
LibzipPlugin::updateArchiveData()忽略options参数（参数名被注释为/*options*/），通过resetArchiveData()清空全部内存数据后使用zip_open()从磁盘重读。zip_open()失败处理被注释掉，重读失败时函数静默返回PFT_Nomral（成功），内存数据为空或过时，导致重命名后 UI 无法正确刷新。而LibarchivePlugin::updateArchiveData()利用options参数在内存中更新条目，因此 tar 格式能正确刷新。关键证据：
3rdparty/libzipplugin/libzipplugin.cpp:556-589（缺陷实现）对比3rdparty/libarchive/libarchive/libarchiveplugin.cpp:576-700（正确参考实现）。修复方案
参照
LibarchivePlugin::updateArchiveData()重写LibzipPlugin::updateArchiveData()，使用options参数对 Delete、Rename、Add 三种操作类型做内存级更新，消除磁盘重读脆弱性，自然恢复错误处理（不再有zip_open()路径及注释掉的失败处理）。仅修改3rdparty/libzipplugin/libzipplugin.cpp，1 文件改动，141 行新增，24 行删除。改动安全评估
代码安全评估
zip_open错误处理）不是历史 bug 修复产物，而是早期开发遗留的未完成代码，本次修复不撤销任何历史修复。singlejob.cpp:545通过虚函数分派调用，函数签名不变，返回类型不变。关联 PMS bug（365277、356233、353985）均为安全/符号链接修复，与本次改动无关。业务影响范围
影响 zip 格式压缩包文件管理：zip 压缩包内文件的重命名、删除、追加操作。修复后 zip 压缩包在这些操作后 UI 即时刷新，与 tar 格式行为一致。tar/tar.gz 等格式不受影响（仅修改 libzip 插件）。
验证建议
重点回归测试 zip 格式下文件和多层文件夹的重命名/删除/追加操作，确认 tar 格式操作不受影响。
Summary by Sourcery
Keep ZIP archive metadata synchronized in memory after file management operations.
Bug Fixes:
Enhancements: