Skip to content

fix: Specify plugin type for zip file handling in ArchiveManager#364

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/snipefrom
dengzhongyuan365-dev:pzip-cpp
Feb 10, 2026
Merged

fix: Specify plugin type for zip file handling in ArchiveManager#364
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/snipefrom
dengzhongyuan365-dev:pzip-cpp

Conversation

@dengzhongyuan365-dev
Copy link
Contributor

@dengzhongyuan365-dev dengzhongyuan365-dev commented Feb 10, 2026

Updated the ArchiveManager to explicitly assign the libzip plugin when adding zip files, addressing compatibility issues with the pzip plugin. Enhanced logging to include the plugin type used during the interface creation.

bug: https://pms.uniontech.com/bug-view-350661.html

Summary by Sourcery

Ensure zip archives use a compatible plugin when appending files and improve visibility into plugin selection during interface creation.

Bug Fixes:

  • Force use of the libzip plugin when appending to zip archives to avoid compatibility issues with the pzip plugin.

Enhancements:

  • Log the selected plugin type when creating the archive interface for adding files.

Updated the ArchiveManager to explicitly assign the libzip plugin when adding zip files, addressing compatibility issues with the pzip plugin. Enhanced logging to include the plugin type used during the interface creation.

bug: https://pms.uniontech.com/bug-view-350661.html
@sourcery-ai
Copy link

sourcery-ai bot commented Feb 10, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

ArchiveManager now explicitly selects the libzip plugin when appending to ZIP archives and logs the chosen plugin type when creating the archive interface, avoiding compatibility issues with the pzip plugin.

Sequence diagram for ArchiveManager addFiles with explicit plugin selection

sequenceDiagram
    actor Caller
    participant ArchiveManager
    participant EventLogUtils
    participant UiTools
    participant MimeDetector as determineMimeType
    participant CustomMimeType
    participant ArchiveInterface

    Caller->>ArchiveManager: addFiles(strArchiveFullPath, fileList)
    ArchiveManager->>EventLogUtils: writeLogs(eventObject)
    ArchiveManager->>MimeDetector: determineMimeType(strArchiveFullPath)
    MimeDetector-->>ArchiveManager: CustomMimeType
    ArchiveManager->>CustomMimeType: name()
    CustomMimeType-->>ArchiveManager: "application/zip" or other
    alt is_zip
        ArchiveManager->>ArchiveManager: eType = APT_Libzip
    else not_zip
        ArchiveManager->>ArchiveManager: eType = APT_Auto
    end
    ArchiveManager->>UiTools: createInterface(strArchiveFullPath, true, eType)
    UiTools-->>ArchiveManager: m_pTempInterface: ArchiveInterface
    alt interface_created
        ArchiveManager->>ArchiveInterface: create AddJob and append files
    else interface_not_created
        ArchiveManager->>ArchiveManager: handle error
    end
    ArchiveManager-->>Caller: return success or failure
Loading

Class diagram for ArchiveManager plugin selection and UiTools interface creation

classDiagram
    class ArchiveManager {
        +bool addFiles(QString strArchiveFullPath, QList~FileInfo~ fileList)
        -ArchiveInterface* m_pTempInterface
    }

    class UiTools {
        +ArchiveInterface* createInterface(QString archiveFullPath, bool appendMode, AssignPluginType pluginType)
    }

    class AssignPluginType {
        <<enumeration>>
        +APT_Auto
        +APT_Libzip
    }

    class CustomMimeType {
        +QString name()
    }

    class ArchiveInterface {
        +void appendFiles(QList~FileInfo~ fileList)
    }

    ArchiveManager --> UiTools : uses
    ArchiveManager --> CustomMimeType : uses
    ArchiveManager --> AssignPluginType : selects
    UiTools --> AssignPluginType : configures
    ArchiveManager --> ArchiveInterface : creates
    UiTools --> ArchiveInterface : returns
Loading

File-Level Changes

Change Details Files
Explicitly select libzip plugin for ZIP archives when adding files and log the plugin type used.
  • Introduce a plugin type variable defaulting to automatic selection before creating the archive interface.
  • Determine the MIME type of the target archive path and, if it is application/zip, override the plugin type to use the libzip plugin.
  • Enhance debug logging to print the plugin type used when creating the interface for adding files.
  • Update the createInterface call to accept and pass the explicit plugin type parameter.
src/source/archivemanager/archivemanager.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要是为了解决 ZIP 文件追加文件时的兼容性问题。下面是对这段代码的详细审查和改进建议:

1. 代码逻辑审查

当前逻辑:
代码通过检查文件 MIME 类型是否为 "application/zip" 来决定是否强制使用 UiTools::APT_Libzip 插件。这是一个针对 pzip 插件不支持 ZIP 追加操作的临时解决方案。

潜在问题:

  • MIME 类型检测的准确性determineMimeType 函数的实现未展示,但如果它仅依赖文件扩展名(如 .zip),可能会误判。例如,某些文件可能扩展名不是 .zip 但实际是 ZIP 格式(如 JAR、ODT 等)。
  • 硬编码字符串QLatin1String("application/zip") 是硬编码的,如果 MIME 类型定义变更或需要支持其他格式,维护成本较高。

改进建议:

  • 如果 determineMimeType 仅检查扩展名,建议改为检查文件头(Magic Number)以确保更准确的格式识别。
  • 将 MIME 类型常量提取到类的静态成员或全局常量中,便于维护。

2. 代码质量审查

优点:

  • 添加了清晰的注释说明 workaround 的原因,便于后续维护者理解。
  • 使用了 QLatin1String 优化字符串比较。

改进建议:

  • 变量命名eType 可以改为更具描述性的名称,如 pluginType
  • 代码复用:如果其他地方也需要判断 ZIP 格式并选择插件,建议将此逻辑提取为独立函数。

3. 代码性能审查

当前性能:

  • MIME 类型检测可能涉及文件 I/O(如果检查文件头),但这是必要的开销。
  • 字符串比较使用了高效的 QLatin1String,性能良好。

改进建议:

  • 如果 determineMimeType 实现较慢(如检查大文件),可以考虑缓存结果,尤其是当同一文件会被多次操作时。

4. 代码安全审查

潜在问题:

  • 路径遍历风险strArchiveFullPath 是用户提供的路径,需确保其合法性(如不包含 ../ 等路径遍历字符)。
  • 插件类型注入:虽然 eType 是内部变量,但如果 UiTools::createInterface 的第三个参数可被外部控制,需验证其合法性。

改进建议:

  • 在调用 determineMimeType 前,验证 strArchiveFullPath 的合法性(如使用 QFileInfo::canonicalPath())。
  • 确保 UiTools::createInterfaceeType 参数有严格的边界检查。

5. 改进后的代码示例

bool ArchiveManager::addFiles(const QString &strArchiveFullPath, const QList<FileData> &files) {
    // 日志记录
    QVariantMap obj = {
        {"describe", QString("Add File to package: ") + strArchiveFullPath}
    };
    EventLogUtils::get().writeLogs(obj);

    // 安全检查
    QString normalizedPath = QFileInfo(strArchiveFullPath).canonicalFilePath();
    if (normalizedPath.isEmpty()) {
        qWarning() << "Invalid archive path:" << strArchiveFullPath;
        return false;
    }

    // 插件类型选择
    UiTools::AssignPluginType pluginType = UiTools::APT_Auto;
    CustomMimeType mimeType = determineMimeType(normalizedPath);
    if (isZipFormat(mimeType)) { // 提取为独立函数
        pluginType = UiTools::APT_Libzip;
    }

    qDebug() << "Creating interface for adding files, plugin type:" << pluginType;
    m_pTempInterface = UiTools::createInterface(normalizedPath, true, pluginType);

    if (m_pTempInterface) {
        qDebug() << "Creating AddJob";
        // ... 后续代码
    }

    return true;
}

// 辅助函数(可放在类的私有部分或工具类中)
bool ArchiveManager::isZipFormat(const CustomMimeType &mimeType) {
    static const QLatin1String zipMime("application/zip");
    return mimeType.name() == zipMime;
}

6. 总结

  • 语法逻辑:当前代码逻辑正确,但可以通过更健壮的 MIME 类型检测和路径验证来增强可靠性。
  • 代码质量:注释清晰,但可以通过提取常量和函数复用来提高可维护性。
  • 代码性能:当前性能可接受,但若 MIME 检测较慢,可考虑缓存。
  • 代码安全:需增加路径验证和插件类型检查,防止潜在的安全风险。

以上改进建议可以根据实际项目需求选择性采纳。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The plugin selection checks only for an exact application/zip MIME name; consider using a more robust check (e.g., inherits or handling common aliases) so zip-like archives are consistently routed to libzip when needed.
  • If determineMimeType is already used elsewhere for the same path, consider reusing or caching the result to avoid redundant MIME detection work when adding files.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The plugin selection checks only for an exact `application/zip` MIME name; consider using a more robust check (e.g., `inherits` or handling common aliases) so zip-like archives are consistently routed to `libzip` when needed.
- If `determineMimeType` is already used elsewhere for the same path, consider reusing or caching the result to avoid redundant MIME detection work when adding files.

## Individual Comments

### Comment 1
<location> `src/source/archivemanager/archivemanager.cpp:150-158` </location>
<code_context>
+    // workaround:
+    // pzip 仅支持新建压缩,目前先将zip 追加时显式指定使用 libzip 插件
+    UiTools::AssignPluginType eType = UiTools::APT_Auto;
+    CustomMimeType mimeType = determineMimeType(strArchiveFullPath);
+    if (mimeType.name() == QLatin1String("application/zip")) {
+        eType = UiTools::APT_Libzip;
+    }
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Consider handling additional ZIP MIME types or using a more robust ZIP detection.

Currently only `application/zip` is considered, but ZIPs are often reported as `application/x-zip-compressed` or other aliases. To avoid missing valid ZIP archives, consider normalizing MIME types and checking a small set of known ZIP variants, or falling back to an extension-based `.zip` check when MIME detection doesn’t match exactly.

```suggestion
    // workaround:
    // pzip 仅支持新建压缩,目前先将zip 追加时显式指定使用 libzip 插件
    UiTools::AssignPluginType eType = UiTools::APT_Auto;
    CustomMimeType mimeType = determineMimeType(strArchiveFullPath);

    // 兼容更多 ZIP MIME 类型,并在 MIME 识别失败时回退到扩展名判断
    const QString mimeName = mimeType.name().toLower();
    const bool isZipMime =
        mimeName == QLatin1String("application/zip") ||
        mimeName == QLatin1String("application/x-zip-compressed") ||
        mimeName == QLatin1String("application/x-zip") ||
        mimeName == QLatin1String("application/x-compressed-zip");
    const bool isZipByExt = strArchiveFullPath.endsWith(QLatin1String(".zip"), Qt::CaseInsensitive);

    if (isZipMime || isZipByExt) {
        eType = UiTools::APT_Libzip;
    }

    qDebug() << "Creating interface for adding files, plugin type:" << eType;
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dengzhongyuan365-dev
Copy link
Contributor Author

/forcemerge

@deepin-bot deepin-bot bot merged commit f11772e into linuxdeepin:develop/snipe Feb 10, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants