Skip to content

fix: Add dynamic path resolution for htmltopdf#188

Merged
lzwind merged 2 commits intolinuxdeepin:masterfrom
dengzhongyuan365-dev:master
Sep 3, 2025
Merged

fix: Add dynamic path resolution for htmltopdf#188
lzwind merged 2 commits intolinuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev
Copy link
Contributor

fix: Add dynamic path resolution for htmltopdf

Added a new function getHtmlToPdfPath() to dynamically resolve the path for htmltopdf. Updated the htmltopdfCommand in getDocument to use this function instead of a hardcoded path, improving code flexibility and maintainability.

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

Added a new function getHtmlToPdfPath() to dynamically resolve the path for htmltopdf. Updated the htmltopdfCommand in getDocument to use this function instead of a hardcoded path, improving code flexibility and maintainability.

bug: https://pms.uniontech.com/bug-view-332133.html
 - update version to 6.5.36

 log: update version to 6.5.36
@github-actions
Copy link

github-actions bot commented Sep 3, 2025

TAG Bot

TAG: 6.5.36
EXISTED: no
DISTRIBUTION: unstable

@deepin-ci-robot
Copy link

deepin pr auto review

根据提供的git diff,我将对代码进行审查,并提出改进意见:

1. 代码逻辑审查

  • 版本号从6.5.35.1更新到6.5.36.1,符合语义化版本规范
  • changelog中添加了动态路径解析的bug修复说明
  • Model.cpp中添加了htmltopdf工具的动态路径解析功能

2. 代码质量改进建议

  1. 错误处理不完善

    • getHtmlToPdfPath()函数在找不到htmltopdf工具时返回空字符串,但没有调用方处理这个空字符串的情况
    • 建议在调用getHtmlToPdfPath()后检查返回值是否为空,如果为空应该有适当的错误处理
  2. 日志级别使用不当

    • 使用qDebug()输出路径信息在生产环境中可能产生过多日志
    • 建议将路径信息的日志级别改为qInfo()qDebug()仅在调试模式下启用
  3. 代码结构优化

    • getHtmlToPdfPath()函数可以提取为独立的工具类或模块,便于维护和测试
    • 建议添加注释说明动态路径解析的目的和逻辑

3. 代码性能改进建议

  1. 路径检查效率

    • 当前代码进行了两次文件存在性检查,可以考虑合并为一个更灵活的路径检查机制
    • 可以考虑添加缓存机制,避免每次调用都检查文件系统
  2. 字符串操作优化

    • 多次字符串拼接操作可以使用QString的+运算符链式调用优化
    • 考虑使用QStringLiteral处理字面量字符串

4. 代码安全改进建议

  1. 路径安全性

    • 直接拼接用户提供的文件路径可能存在安全风险
    • 建议对输入路径进行规范化处理,防止路径遍历攻击
    • 使用QFileInfo获取绝对路径,避免相对路径带来的安全问题
  2. 命令执行安全

    • 当前代码直接拼接命令字符串并执行,存在命令注入风险
    • 建议使用QProcess的参数列表方式而不是拼接命令字符串
    • 对输入参数进行合法性检查
  3. 资源管理

    • 临时文件的使用需要确保在异常情况下也能正确清理
    • 建议使用RAII模式管理临时文件资源

改进后的代码建议示例:

QString getHtmlToPdfPath() {
    // 预定义的搜索路径
    const QStringList searchPaths = {
        QString(INSTALL_PREFIX) + "/lib/deepin-reader/htmltopdf",
        QLibraryInfo::path(QLibraryInfo::LibrariesPath) + "/deepin-reader/htmltopdf"
    };

    for (const QString& path : searchPaths) {
        QFileInfo fileInfo(path);
        if (fileInfo.exists() && fileInfo.isExecutable()) {
            qInfo() << "Using htmltopdf at:" << path;
            return fileInfo.absoluteFilePath();
        }
    }

    qWarning() << "htmltopdf not found in any of the search paths";
    return QString();
}

// 在调用处添加错误处理
QString htmltopdfPath = getHtmlToPdfPath();
if (htmltopdfPath.isEmpty()) {
    qCritical() << "Failed to find htmltopdf tool";
    return nullptr;
}

// 使用参数列表方式执行命令
QProcess converter2;
converter2.setProgram(htmltopdfPath);
converter2.setArguments({tmpHtmlFilePath, realFilePath});
converter2.setWorkingDirectory(convertedFileDir + "/word");
if (!converter2.start()) {
    qCritical() << "Failed to start htmltopdf process";
    return nullptr;
}

这些改进可以提高代码的健壮性、安全性和可维护性。

@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

@lzwind lzwind merged commit 8e21fb7 into linuxdeepin:master Sep 3, 2025
7 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