Skip to content

Conversation

@GongHeng2017
Copy link

@GongHeng2017 GongHeng2017 commented Aug 20, 2025

-- When enter the right password, show password error. -- Not judge the Second password, so add code logic the adjust it.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-329305.html

Summary by Sourcery

Fix password error handling in libzip plugin to properly track and limit password retries

Bug Fixes:

  • Stop extraction and show error after entering wrong password twice for the same archive entry

Enhancements:

  • Introduce lastNeedPasswordIndex to track entries needing password and prevent repeated prompts

-- When enter the right password, show password error.
-- Not judge the Second password, so add code logic the adjust it.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-329305.html
@deepin-ci-robot
Copy link

deepin pr auto review

我对这段代码进行审查,提供以下改进建议:

  1. 代码逻辑和安全性改进:
  • 建议将密码错误处理逻辑提取为单独的函数,提高代码可读性和可维护性
  • 密码验证逻辑中,lastNeedPasswordIndex 的使用可以优化,建议记录尝试次数而不是仅记录最后一次尝试的索引
  • 密码错误处理中,直接关闭archive可能导致资源泄漏,建议使用RAII模式管理资源
  1. 性能优化:
  • 在循环中频繁调用 zip_get_num_entries 可能影响性能,建议在循环开始前获取并存储结果
  • 对于密码验证,可以考虑添加最大尝试次数限制,防止恶意用户无限尝试
  • 密码对话框弹出和等待可能会阻塞主线程,建议考虑异步处理方式
  1. 代码质量改进:
  • 变量命名可以更具描述性,如 qExtractSize 可以改为 totalExtractSize
  • 错误处理逻辑较为复杂,建议使用状态机模式重构
  • 注释不够详细,建议补充说明关键业务逻辑和边界条件处理
  1. 具体代码修改建议:
// 在类中定义最大密码尝试次数
static const int MAX_PASSWORD_ATTEMPTS = 3;

// 提取密码验证逻辑为单独函数
bool LibzipPlugin::handlePasswordError(zip_int64_t currentIndex, int& attemptCount) {
    if (m_eErrorType != ET_WrongPassword && m_eErrorType != ET_NeedPassword) {
        return false;
    }

    // 检查尝试次数
    if (++attemptCount >= MAX_PASSWORD_ATTEMPTS) {
        zip_close(archive);
        m_strPassword = "";
        return false;
    }

    // 如果是重复尝试同一文件且密码错误,直接返回
    if (attemptCount > 1 && lastNeedPasswordIndex == currentIndex) {
        zip_close(archive);
        m_strPassword = "";
        return false;
    }

    PasswordNeededQuery query(strFileName);
    emit signalQuery(&query);
    query.waitForResponse();
    
    if (query.isAccepted()) {
        setPassword(query.password());
        zip_set_default_password(archive, m_strPassword.toUtf8().constData());
        lastNeedPasswordIndex = currentIndex;
        return true;
    }
    
    return false;
}

// 在extractFiles函数中使用
for (zip_int64_t i = 0; i < nofEntries; ++i) {
    if (QThread::currentThread()->isInterruptionRequested()) {
        // ... 现有代码 ...
    }
    
    int passwordAttempts = 0;
    while (true) {
        // ... 现有解压逻辑 ...
        
        if (error) {
            if (!handlePasswordError(i, passwordAttempts)) {
                return PFT_Error;
            }
            continue;  // 密码正确后重试当前文件
        }
        break;
    }
}
  1. 其他建议:
  • 考虑添加日志记录功能,记录密码尝试次数和解压进度
  • 可以添加密码强度验证,提高安全性
  • 建议添加异常处理机制,确保在异常情况下资源能正确释放

这些改进建议旨在提高代码的可维护性、安全性和性能,同时保持原有功能不变。根据实际需求,可以选择实施部分或全部建议。

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 there - I've reviewed your changes and they look great!


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.

@sourcery-ai
Copy link

sourcery-ai bot commented Aug 20, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

In extractFiles, this PR refactors the password prompt flow to track and handle consecutive wrong-password attempts for the same archive entry, preventing false error signals when a correct password is supplied.

Sequence diagram for improved password error handling in extractFiles

sequenceDiagram
    participant Plugin as LibzipPlugin
    participant Archive as zip archive
    participant User as actor User
    participant Query as PasswordNeededQuery

    Plugin->>Archive: Attempt to extract entry[i]
    alt Password required or wrong password
        Plugin->>Query: Emit signalQuery for password
        User->>Query: Enter password
        Query->>Plugin: Return password
        Plugin->>Archive: Set password and retry
        alt Password still wrong for same entry
            Plugin->>Archive: Close archive
            Plugin->>User: Show password error
        else Password correct
            Plugin->>Archive: Continue extraction
        end
    else No password needed
        Plugin->>Archive: Continue extraction
    end
Loading

Class diagram for updated LibzipPlugin password handling

classDiagram
    class LibzipPlugin {
        +extractFiles(files, options)
        -m_eErrorType
        -m_strPassword
        -lastNeedPasswordIndex
    }
    class PasswordNeededQuery {
        +password()
        +waitForResponse()
    }
    LibzipPlugin "1" -- "*" PasswordNeededQuery: emits signalQuery
Loading

File-Level Changes

Change Details Files
Refactor password error handling to detect and block repeated wrong-password inputs
  • Declare and initialize lastNeedPasswordIndex before entry loop
  • Remove standalone ET_WrongPassword error branch
  • Merge ET_WrongPassword and ET_NeedPassword into a single handling block
  • Add check comparing lastNeedPasswordIndex with current index to catch a second failed attempt and return an error
  • Record lastNeedPasswordIndex and decrement loop index after new password input to retry extraction
3rdparty/libzipplugin/libzipplugin.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

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, liyigang1, max-lvs

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

@GongHeng2017
Copy link
Author

/merge

@deepin-bot deepin-bot bot merged commit 2d870ff into linuxdeepin:release/eagle Aug 20, 2025
14 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.

4 participants