Skip to content

Conversation

@caixr23
Copy link
Contributor

@caixr23 caixr23 commented Nov 11, 2025

Modified the secret agent to properly handle multiple concurrent password requests by using unique call IDs instead of connection path + setting name combinations. The previous implementation would incorrectly cancel duplicate requests when multiple connections needed passwords simultaneously.

Key changes:

  1. Added m_callNextId counter to generate unique call identifiers
  2. Implemented nextId() method to create unique hexadecimal call IDs
  3. Removed duplicate request detection that prevented concurrent processing
  4. Simplified processNext() method with clearer control flow
  5. Updated CancelGetSecrets to match requests by connection path and setting name instead of callId

Log: Fixed issue where multiple network connections could not request passwords simultaneously

Influence:

  1. Verify password prompts appear correctly for each connection
  2. Test canceling individual password requests without affecting others
  3. Verify password storage and retrieval works correctly for concurrent connections
  4. Test VPN connections requiring authentication during other network operations

fix: 修复并发密码请求处理问题

修改了密钥代理程序,通过使用唯一的调用ID而不是连接路径+设置名称组合来正
确处理多个并发密码请求。之前的实现在多个连接需要同时输入密码时会错误地取
消重复请求。

主要变更:

  1. 添加m_callNextId计数器来生成唯一的调用标识符
  2. 实现nextId()方法创建唯一的十六进制调用ID
  3. 移除了阻止并发处理的重复请求检测逻辑
  4. 简化了processNext()方法,使控制流更清晰
  5. 更新CancelGetSecrets以通过连接路径和设置名称匹配请求而不是callId

Log: 修复了多个网络连接无法同时请求密码的问题

Influence:

  1. 验证每个连接的密码提示是否正确显示
  2. 测试取消单个密码请求而不影响其他请求
  3. 验证并发连接的密码存储和检索功能是否正常
  4. 测试其他网络操作期间需要认证的VPN连接

PMS: BUG-321517

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.

Sorry @caixr23, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Modified the secret agent to properly handle multiple concurrent
password requests by using unique call IDs instead of connection path +
setting name combinations. The previous implementation would incorrectly
cancel duplicate requests when multiple connections needed passwords
simultaneously.

Key changes:
1. Added m_callNextId counter to generate unique call identifiers
2. Implemented nextId() method to create unique hexadecimal call IDs
3. Removed duplicate request detection that prevented concurrent
processing
4. Simplified processNext() method with clearer control flow
5. Updated CancelGetSecrets to match requests by connection path and
setting name instead of callId

Log: Fixed issue where multiple network connections could not request
passwords simultaneously

Influence:
1. Verify password prompts appear correctly for each connection
2. Test canceling individual password requests without affecting others
3. Verify password storage and retrieval works correctly for concurrent
connections
5. Test VPN connections requiring authentication during other network
operations

fix: 修复并发密码请求处理问题

修改了密钥代理程序,通过使用唯一的调用ID而不是连接路径+设置名称组合来正
确处理多个并发密码请求。之前的实现在多个连接需要同时输入密码时会错误地取
消重复请求。

主要变更:
1. 添加m_callNextId计数器来生成唯一的调用标识符
2. 实现nextId()方法创建唯一的十六进制调用ID
3. 移除了阻止并发处理的重复请求检测逻辑
4. 简化了processNext()方法,使控制流更清晰
5. 更新CancelGetSecrets以通过连接路径和设置名称匹配请求而不是callId

Log: 修复了多个网络连接无法同时请求密码的问题

Influence:
1. 验证每个连接的密码提示是否正确显示
2. 测试取消单个密码请求而不影响其他请求
3. 验证并发连接的密码存储和检索功能是否正常
4. 测试其他网络操作期间需要认证的VPN连接

PMS: BUG-321517
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个代码变更进行详细的审查和分析:

  1. 代码逻辑改进:
  • 原代码使用 connectionPath.path() + settingName 作为 callId,这种方式可能导致 ID 冲突。新代码使用 nextId() 函数生成唯一的十六进制 ID,更加安全可靠。
  • 在 CancelGetSecrets 函数中,原代码使用 callId 进行比较,新代码改为直接比较 settingName 和 connectionPath,这样更加直观且不容易出错。
  1. 代码质量改进:
  • processNext() 函数的重构使代码更加清晰,通过 deleteAfter 变量统一处理删除逻辑,减少了重复代码。
  • 增加了调试日志,方便追踪请求的处理过程。
  1. 代码性能改进:
  • 移除了 GetSecrets 函数中的重复调用检查,这可能提高性能,但也可能带来风险(见安全性部分)。
  • nextId() 使用简单的递增方式生成 ID,性能较好。
  1. 代码安全性改进:
  • 原代码在 GetSecrets 中检查重复调用,这个安全检查被移除了,这可能导致问题。建议保留这个检查或提供其他形式的保护。
  • nextId() 函数虽然使用了 0xFFFFFFFF 掩码,但没有考虑整数溢出的情况。建议添加溢出检查。

建议改进:

  1. 在 GetSecrets 函数中保留重复调用检查,但使用新的 ID 机制:
const QString callId = nextId();
for (const SecretsRequest &request : m_calls) {
    if (request.connectionPath == connectionPath && request.settingName == settingName) {
        qCWarning(DSM()) << "Get secrets was called again! This should not happen, cancelling first call";
        CancelGetSecrets(connectionPath, settingName);
        break;
    }
}
  1. 改进 nextId() 函数,添加溢出检查:
QString NetworkSecretAgent::nextId()
{
    if (m_callNextId == UINT_MAX) {
        m_callNextId = 0;
    }
    return QString::number(0xFFFFFFFF & m_callNextId++, 16);
}
  1. 考虑添加请求超时机制,防止请求堆积:
private:
    QTimer *m_requestTimer;
    static const int REQUEST_TIMEOUT = 30000; // 30 seconds
  1. 建议在 SecretsRequest 结构中添加时间戳,用于追踪请求的生命周期:
struct SecretsRequest {
    // ... 其他成员
    QDateTime timestamp;
};

这些改进将使代码更加健壮和安全,同时保持良好的性能。

@caixr23 caixr23 requested a review from mhduiy November 11, 2025 05:58
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, mhduiy, robertkill

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

@caixr23 caixr23 merged commit 82aa823 into linuxdeepin:master Nov 12, 2025
15 of 17 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