chore: replace the connectivity check type

[ut003640]

used libcurl to check the connectivity

Log:replace the connectivity check type
Influence: 任务栏图标显示的状态是否正常
PMS: BUG-331389

[sourcery-ai]

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

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

deepin pr auto review

Git Diff 代码审查报告

整体评估

这次提交对网络连通性检测模块进行了重要重构，主要变化包括：

1. 将QProcess实现的HTTP请求替换为libcurl实现
2. 引入了多线程处理网络检测逻辑
3. 添加了新的配置项来控制超时时间和检测间隔
4. 重构了ConnectivityChecker类的层次结构

详细审查

1. 语法与逻辑

1.1 正面改进

• 引入libcurl替代QProcess是合理的改进，提供了更可控的HTTP请求功能
• 使用单独的线程(QThread)处理网络检测，避免阻塞主线程
• 添加了防抖机制(m_pendingCheckTimer)减少频繁请求
• 新增配置项使超时时间可配置，提高了灵活性

1.2 潜在问题

1. 在connectivitychecker.cpp中：

   • LocalConnectionvityChecker构造函数中使用了QMetaObject::invokeMethod来调用initConnectivityChecker，这可能导致初始化延迟
   • StatusChecker::stop()方法只设置了标志位m_isStop，但没有立即停止正在进行的HTTP请求

2. 在httpmanager.cpp中：

   • HttpManager::get方法中创建的HttpReply对象没有指定父对象，可能导致内存泄漏
   • write_data回调函数使用了静态函数，这在多线程环境下可能需要同步机制

3. 在settingconfig.cpp中：

   • 新增的配置项读取没有进行有效性检查，如httpRequestTimeout和httpConnectTimeout可能为负值

2. 代码质量

2.1 正面改进

• 代码结构更加清晰，将网络检测逻辑分离到StatusChecker类中
• 使用了更现代的C++特性，如explicit关键字
• 添加了适当的日志输出，便于调试

2.2 可改进之处

1. 命名一致性：

   • 类名LocalConnectionvityChecker中的拼写错误(Connectionvity应为Connectivity)应统一修正
   • 变量名m_pendingCheck和m_isStop可以更明确地表示其用途

2. 代码注释：

   • 部分新增代码缺少注释，如HttpManager::get方法中的各个curl选项设置
   • 防抖机制的实现逻辑应该有更详细的注释说明

3. 资源管理：

   • HttpReply对象的生命周期管理不够明确
   • CURL句柄在错误情况下可能未正确释放

3. 代码性能

3.1 正面改进

• 使用libcurl替代QProcess减少了进程创建开销
• 多线程处理避免了阻塞主线程
• 防抖机制减少了不必要的网络请求

3.2 性能考虑

1. 在connectivitychecker.cpp中：

   • 每次检测都会创建新的CURL句柄，可以考虑复用
   • realStartCheck方法中对每个URL都会创建新的HttpManager实例，可能造成不必要的开销

2. 在httpmanager.cpp中：

   • setsockopt调用可能在每次请求时重复执行，可以优化

4. 代码安全

4.1 正面改进

• 添加了超时配置，防止长时间阻塞
• 使用CURLOPT_NOSIGNAL避免多线程环境下的信号问题

4.2 安全考虑

1. 在httpmanager.cpp中：

   • 没有对URL进行验证，可能存在SSRF(服务器端请求伪造)风险
   • write_data回调函数中直接追加数据到缓冲区，没有大小限制，可能导致缓冲区溢出

2. 在connectivitychecker.cpp中：

   • 线程间通信使用信号槽机制，但缺少对m_isStop标志位的原子操作保护
   • StatusChecker的析构函数中调用stop()可能不够安全，因为线程可能仍在运行

3. 在settingconfig.cpp中：

   • 配置项读取没有进行范围检查，可能导致不合理的值被使用

改进建议

1. 修复拼写错误：将LocalConnectionvityChecker重命名为LocalConnectivityChecker

2. 改进资源管理：

   // 在httpmanager.cpp中
   HttpReply *HttpManager::get(const QString &url)
   {
       // ... 现有代码 ...
       if (curlRes != CURLE_OK) {
           curl_easy_cleanup(curl); // 确保在错误情况下也释放资源
           // ... 错误处理 ...
       }
       // ... 现有代码 ...
   }

3. 添加配置项验证：

   // 在settingconfig.cpp中
   int SettingConfig::httpRequestTimeout() const
   {
       int timeout = m_httpRequestTimeout;
       return timeout > 0 ? timeout : 15; // 提供合理的默认值
   }

4. 改进线程安全：

   // 在connectivitychecker.cpp中
   void StatusChecker::stop()
   {
       std::lock_guard<std::mutex> lock(m_mutex); // 添加互斥锁保护
       m_isStop = true;
       // 取消正在进行的HTTP请求
   }

5. 添加URL验证：

   // 在httpmanager.cpp中
   HttpReply *HttpManager::get(const QString &url)
   {
       // 添加URL验证
       if (!isValidUrl(url)) {
           HttpReply *reply = new HttpReply(this);
           reply->setErrorMessage("Invalid URL");
           return reply;
       }
       // ... 现有代码 ...
   }

6. 改进缓冲区管理：

   // 在httpmanager.cpp中
   static size_t write_data(char *data, size_t size, size_t nmemb, std::string *buffer)
   {
       size_t total_size = size * nmemb;
       if (buffer && buffer->size() + total_size <= MAX_BUFFER_SIZE) { // 添加大小限制
           buffer->append(data, total_size);
           return total_size;
       }
       return 0; // 超过限制返回0，中止传输
   }

总结

这次提交对网络连通性检测模块进行了重要重构，整体方向是正确的，主要改进了性能和可配置性。但在代码安全性、资源管理和线程安全方面还有一些需要改进的地方。建议在合并前进行上述改进，特别是资源管理和线程安全方面的修改，以避免潜在的内存泄漏和竞态条件问题。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, ut003640

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

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

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

[ut003640]

/merge

[deepin-bot]

This pr cannot be merged! (status: unstable)

[ut003640]

/forcemerge

[deepin-bot]

This pr force merged! (status: unstable)