fix: remove empty files left when split-volume encrypted extraction f…#360
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom Feb 2, 2026
Merged
Conversation
…ails (e.g. wrong password) Log: as title Bug: https://pms.uniontech.com/bug-view-343249.html
Reviewer's GuideAdds a cleanup routine that, on failed full-volume extractions (e.g., wrong password on split encrypted archives), removes empty files and then prunes empty directories in the target path, and ensures progress is set to 100% before aborting when line handling fails. Sequence diagram for extraction failure cleanup and progress updatesequenceDiagram
actor User
participant CliInterface
participant ArchiveProcess
participant FileSystem
User->>CliInterface: startExtraction()
CliInterface->>ArchiveProcess: start()
loop processOutput
ArchiveProcess-->>CliInterface: stdout line
CliInterface->>CliInterface: readStdout(handleAll)
CliInterface->>CliInterface: handleLine(line, m_workStatus)
alt handleLine fails
CliInterface->>CliInterface: signalprogress(100)
CliInterface->>ArchiveProcess: killProcess()
note over CliInterface: break
end
end
ArchiveProcess-->>CliInterface: finished(exitCode != 0)
CliInterface->>CliInterface: extractProcessFinished(exitCode, exitStatus)
alt full extraction and target path set
CliInterface->>CliInterface: removeExtractedFilesOnFailure(strTargetPath, m_files)
CliInterface->>FileSystem: remove empty files in target path
CliInterface->>FileSystem: prune empty directories in target path
end
CliInterface-->>User: report extraction failure
Updated class diagram for CliInterface extraction failure handlingclassDiagram
class CliInterface {
+bool moveExtractTempFilesToDest(QList_FileEntry files, ExtractionOptions options)
+void removeExtractedFilesOnFailure(QString strTargetPath, QList_FileEntry entries)
+bool handleLongNameExtract(QList_FileEntry files)
+void readStdout(bool handleAll)
+void extractProcessFinished(int exitCode, QProcess_ExitStatus exitStatus)
-ExtractionOptions m_extractOptions
-QList_FileEntry m_files
-WorkStatus m_workStatus
-bool m_listEmptyLines
}
class ExtractionOptions {
+bool bAllExtract
+QString strTargetPath
+QString strDestination
}
class FileEntry {
+QString strFullPath
+bool isDirectory
}
class DataManager {
+static DataManager get_instance()
+ArchiveData archiveData()
}
class ArchiveData {
+QMap_Int_FileEntry mapFileEntry
}
CliInterface --> ExtractionOptions
CliInterface --> FileEntry
DataManager --> ArchiveData
ArchiveData --> FileEntry
Flow diagram for removeExtractedFilesOnFailure cleanup routineflowchart TD
A[Start removeExtractedFilesOnFailure] --> B{entries is empty?}
B -- Yes --> C[entries = ArchiveData.mapFileEntry.values]
B -- No --> D[listToRemove = entries]
C --> E{listToRemove is empty?}
D --> E{listToRemove is empty?}
E -- Yes --> Z[Return]
E -- No --> F[Create QDir targetDir from strTargetPath]
F --> G{targetDir exists?}
G -- No --> Z
G -- Yes --> H[Build paths list from listToRemove
relPath = entry.strFullPath
trim trailing slash
skip empty relPath
store absolute path and isDirectory]
H --> I[For each path with isDirectory == false
if file exists and size == 0
remove file]
I --> J[removed = false]
J --> K[For each path with isDirectory == true
if directory exists and isEmpty
removeRecursively
set removed = true]
K --> L{removed is true?}
L -- Yes --> J
L -- No --> Z
Z[End removeExtractedFilesOnFailure]
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 1 issue, and left some high level feedback:
- In removeExtractedFilesOnFailure, consider building a set of unique directory paths (and possibly sorting them deepest-first) instead of repeatedly scanning the full paths list in a do/while loop, to avoid redundant checks on large archives.
- When removing directories and files, you may want to explicitly handle symlinks (e.g., using QFileInfo::isSymLink) to avoid unintentionally following or deleting outside the intended extraction tree.
- The logic for determining which entries to clean up falls back to all archive entries when the provided list is empty; consider documenting or enforcing whether this behavior is always safe for partial extractions or mixed operations to avoid over-deletion in edge cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In removeExtractedFilesOnFailure, consider building a set of unique directory paths (and possibly sorting them deepest-first) instead of repeatedly scanning the full paths list in a do/while loop, to avoid redundant checks on large archives.
- When removing directories and files, you may want to explicitly handle symlinks (e.g., using QFileInfo::isSymLink) to avoid unintentionally following or deleting outside the intended extraction tree.
- The logic for determining which entries to clean up falls back to all archive entries when the provided list is empty; consider documenting or enforcing whether this behavior is always safe for partial extractions or mixed operations to avoid over-deletion in edge cases.
## Individual Comments
### Comment 1
<location> `3rdparty/interface/archiveinterface/cliinterface.cpp:1130-1132` </location>
<code_context>
+
+ for (const auto &p : paths) {
+ const QString &path = p.first;
+ if (!p.second) { // 文件
+ QFileInfo fi(path);
+ if (fi.exists() && fi.isFile() && fi.size() == 0) {
+ QFile::remove(path);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Deleting all zero-size files on failure may remove legitimately empty files from the archive.
This will delete all zero-sized files in `paths`, including ones that are intentionally empty and were successfully written before the failure. To avoid removing valid files, consider tracking which files were actually created/modified during the failed extraction (e.g., via a temp name/extension, a process-local list, or a timestamp window) instead of relying solely on `size() == 0`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1130
to
+1132
| if (!p.second) { // 文件 | ||
| QFileInfo fi(path); | ||
| if (fi.exists() && fi.isFile() && fi.size() == 0) { |
There was a problem hiding this comment.
issue (bug_risk): Deleting all zero-size files on failure may remove legitimately empty files from the archive.
This will delete all zero-sized files in paths, including ones that are intentionally empty and were successfully written before the failure. To avoid removing valid files, consider tracking which files were actually created/modified during the failed extraction (e.g., via a temp name/extension, a process-local list, or a timestamp window) instead of relying solely on size() == 0.
deepin pr auto review这段代码新增了一个 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
修改后的代码建议（结合部分建议）：void CliInterface::removeExtractedFilesOnFailure(const QString &strTargetPath, const QList<FileEntry> &entries)
{
QList<FileEntry> entriesToProcess = entries;
if (entriesToProcess.isEmpty()) {
// 防御性检查：确保单例和数据有效
if (DataManager::get_instance().isValid()) {
entriesToProcess = DataManager::get_instance().archiveData().mapFileEntry.values();
}
}
if (entriesToProcess.isEmpty()) {
return;
}
QDir targetDir(strTargetPath);
if (!targetDir.exists()) {
return;
}
// 使用 QPair 存储 (绝对路径, 是否是目录)
QList<QPair<QString, bool>> fileSystemPaths;
QString targetAbsPath = targetDir.absolutePath();
for (const FileEntry &entry : entriesToProcess) {
QString relPath = entry.strFullPath;
if (relPath.endsWith(QLatin1Char('/'))) {
relPath.chop(1);
}
if (relPath.isEmpty()) {
continue;
}
QString absPath = targetDir.absoluteFilePath(relPath);
// 安全检查：防止路径遍历攻击
if (!absPath.startsWith(targetAbsPath)) {
continue;
}
fileSystemPaths.append(qMakePair(absPath, entry.isDirectory));
}
// 1. 清理残留文件（当前逻辑：仅清理大小为0的文件）
for (const auto &pair : fileSystemPaths) {
if (!pair.second) { // 是文件
QFileInfo fi(pair.first);
// 仅当文件存在、确实是文件且大小为0时删除
if (fi.exists() && fi.isFile() && fi.size() == 0) {
QFile::remove(pair.first);
}
}
}
// 2. 清理空目录
// 为了性能优化，可以考虑对目录按深度排序后删除，这里保持原有循环逻辑但使用 rmdir
bool removed;
do {
removed = false;
for (const auto &pair : fileSystemPaths) {
if (pair.second) { // 是目录
QDir dir(pair.first);
// rmdir 仅删除空目录，更安全
if (dir.exists() && dir.isEmpty()) {
if (dir.rmdir()) {
removed = true;
}
}
}
}
} while (removed);
}总结这段代码主要解决了在解压失败（特别是分卷加密包密码错误）时清理空文件的问题。整体逻辑清晰，但在**安全性（路径遍历）和健壮性（文件清理条件）**方面可以进一步加强。建议采纳上述关于路径验证和目录删除方式的建议。 |
max-lvs
approved these changes
Feb 2, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, max-lvs 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 |
Contributor
Author
|
/merge |
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
3 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.
…ails (e.g. wrong password)
Log: as title
Bug: https://pms.uniontech.com/bug-view-343249.html
Summary by Sourcery
Clean up artifacts created during failed archive extraction operations and ensure progress is reported when aborting processing.
Bug Fixes: