Skip to content

fix: can not preview picture by double-click#346

Merged
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
LiHua000:master
May 12, 2026
Merged

fix: can not preview picture by double-click#346
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
LiHua000:master

Conversation

@LiHua000
Copy link
Copy Markdown
Contributor

@LiHua000 LiHua000 commented May 9, 2026

Image src was changed to file:// URL in commit 438764b,but viewPicture did not strip the file:// prefix, causing QImageReader to fail silently.

提交438764b将编辑区图片src改为file://绝对URL,
viewPicture未剥离该前缀导致QImageReader静默失败。

Log: 修复双击图片无法预览的问题

Bug: https://pms.uniontech.com/bug-view-354073.html

Image src was changed to file:// URL in commit 438764b,but viewPicture did not strip the file:// prefix, causing QImageReader to fail silently.

提交438764b将编辑区图片src改为file://绝对URL,
viewPicture未剥离该前缀导致QImageReader静默失败。

Log: 修复双击图片无法预览的问题

Bug: https://pms.uniontech.com/bug-view-354073.html

Influence: 修复后双击编辑区图片可正常打开预览。
@LiHua000 LiHua000 force-pushed the master branch 2 times, most recently from 0211108 to 8e1f4f2 Compare May 11, 2026 08:58
Commit 438764b changed getHtml() to store image src as relative
paths (images/xxx.png), but getFullHtml() regex still only matched
absolute paths, preventing base64 conversion. Update regex to match
both formats and resolve relative paths via AppData location.

Log: fix export image bug

Bug: https://pms.uniontech.com/bug-view-354159.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。这次修改的主要目的是兼容相对路径与绝对路径的图片引用,并增加路径遍历安全防护,整体思路非常正确,尤其是在处理用户输入和文件路径时加入安全校验是非常值得肯定的做法。

不过,在语法逻辑、代码质量、性能和安全细节上,还有一些可以改进和优化的空间。以下是我的详细审查意见:

1. 代码安全 - 路径遍历防护存在绕过风险

问题分析:
vnoteitem.cpp 中,你使用了 QDir::cleanPath() 来规范化路径,并使用 absolutePath.startsWith(appDataBase) 来防止路径遍历。但这存在一个经典的前缀碰撞安全漏洞。

假设:

  • appDataBase = "/home/user/.config/App"
  • 攻击者构造的路径 = "/home/user/.config/AppDataEvil/images/hack.png"

此时 absolutePath.startsWith(appDataBase) 会返回 true,从而绕过安全检查,导致程序读取到 AppData 目录外同层级名为 AppDataEvil 的文件夹中的恶意文件。

改进意见:
确保路径不仅以 appDataBase 开头,而且后面紧跟着路径分隔符 /,这样才能确认它是在 appDataBase 目录内部

// 修改前:
if (!absolutePath.startsWith(appDataBase)) {

// 修改后:
if (!absolutePath.startsWith(appDataBase + "/")) {
    // ...
}

2. 语法逻辑 - 正则表达式匹配逻辑隐患

问题分析:
vnoteitem.cpp 中,新的正则表达式为:

QRegExp rxPath("(/[^/\\s]+)+/images/[\\w\\-]+\\.[a-z]{3,4}|images/[\\w\\-]+\\.[a-z]{3,4}");
  1. 匹配范围不一致:左侧(绝对路径)允许路径中包含空格等字符(因为外层是 (/[^/\\s]+)+,排除了斜杠和空白,但允许其他字符),而右侧(相对路径)只允许 [\\w\\-](字母数字下划线连字符)。如果相对路径包含空格或其他合法字符(如中文目录名、加号等),将无法匹配。
  2. rxPath.setMinimal(false) 的影响:由于使用了最大匹配(贪婪模式),当HTML中出现类似 <img src="images/xxx.png"> some text <img src="images/yyy.png"> 时,正则可能会跨越多个标签,将中间的文本也包含进匹配结果中。

改进意见:
建议将相对路径的字符类放宽,与绝对路径保持一致,或者使用更严谨的匹配边界。另外,强烈建议将过时的 QRegExp 迁移到现代的 QRegularExpression(Qt 5.4+ 引入,Qt 6 已移除 QRegExp),它支持更简洁的非贪婪匹配和更严格的边界控制。

如果暂时必须保留 QRegExp,建议改进正则并添加单词边界或引号边界限制,防止贪婪匹配越界:

// 改进后的正则:相对路径部分允许更多合法字符,并限制在引号或标签边界内
// 注意:实际应用中最好根据HTML的src属性值是用单引号还是双引号包裹来精确界定
QRegExp rxPath("(/[^/\\s]+)+/images/[^/\\s]+\\.[a-z]{3,4}|images/[^/\\s]+\\.[a-z]{3,4}");

3. 代码性能 - 静态变量初始化与文件系统I/O

问题分析:

static const QString appDataBase = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation);

使用 static const 是个好习惯,避免了每次调用都去查询路径。但需要注意,QStandardPaths::writableLocation 在某些极端情况下(如程序运行期间用户环境变量发生改变,虽然罕见)可能返回不同结果,不过对于AppData来说通常不变,这里使用 static 是合理的。

另外,QDir::cleanPath() 本身不访问文件系统,只是字符串操作,性能很好。但后续的 Utils::pictureToBase64(absolutePath, base64) 会进行磁盘I/O,如果在主线程中循环处理大量图片,可能会导致UI卡顿。

改进意见:

  • 性能方面,当前逻辑在单次循环中是可以接受的,但如果未来需要批量导出,建议将 getFullHtml() 放在子线程执行。
  • 建议在路径校验通过后、执行文件读取前,增加一次文件是否存在的检查,避免不必要的文件打开操作(pictureToBase64 内部如果没做判断,会导致报错或异常处理开销):
if (!absolutePath.startsWith(appDataBase + "/")) {
    html.append(imgLabel);
} else if (!QFile::exists(absolutePath)) { // 新增:提前规避无效的磁盘I/O
    html.append(imgLabel);
} else {
    // ... 执行 base64 转换
}

4. 代码质量 - URL 解析不够健壮

问题分析:
webrichtexteditor.cpp 中:

QString path = filePath.startsWith("file://") ? filePath.mid(7) : filePath;

这里硬编码了 mid(7) 来去掉 file:// 前缀。这在大多数Linux环境下没问题,但在Windows平台下,标准的 file:// URL 格式通常是 file:///C:/Users/...(带有三个斜杠),如果只去掉7个字符,会留下 /C:/Users/...,这可能导致后续的文件读取失败。此外,URL中可能包含百分号编码(如空格被编码为 %20)。

改进意见:
使用 Qt 提供的专门处理 URL 的类 QUrl 来解析,这样不仅跨平台兼容性更好,还能自动处理 URL 解码:

// 修改前:
QString path = filePath.startsWith("file://") ? filePath.mid(7) : filePath;

// 修改后:
#include <QUrl>

QUrl url(filePath);
QString path = url.isLocalFile() ? url.toLocalFile() : filePath;

总结与最终修改建议代码片段

综合以上意见,针对核心修改部分,我建议的代码如下:

vnoteitem.cpp:

// 匹配图片路径的正则表达式:兼容绝对路径和相对路径
// 注意:放宽了相对路径的字符限制,防止中文/空格路径失效
QRegExp rxPath("(/[^/\\s]+)+/images/[^/\\s]+\\.[a-z]{3,4}|images/[^/\\s]+\\.[a-z]{3,4}");
rxPath.setMinimal(false); // 最大匹配
// AppData基准路径
static const QString appDataBase = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation);

// ... (循环内部)
QString matchedPath = rxPath.cap(0);
QString absolutePath = matchedPath.startsWith("/")
    ? matchedPath
    : appDataBase + "/" + matchedPath;
absolutePath = QDir::cleanPath(absolutePath);

// 路径遍历防护:规范化后路径必须在AppData目录下(追加"/"防止前缀碰撞)
if (!absolutePath.startsWith(appDataBase + "/") || !QFile::exists(absolutePath)) {
    html.append(imgLabel);
} else {
    QString base64 = "";
    if (!Utils::pictureToBase64(absolutePath, base64)) {
        html.append(imgLabel);
    } else {
        html.append(imgLabel.mid(0, last))
            .append(base64)
            .append(imgLabel.mid(last + rxPath.matchedLength(), imgLabel.size() - last - rxPath.matchedLength()));
    }
}

webrichtexteditor.cpp:

#include <QUrl>

void WebRichTextEditor::viewPicture(const QString &filePath)
{
    if (imgView == nullptr) {
        imgView = new ImageViewerDialog(this);
    }
    // 使用 QUrl 安全解析本地文件路径,兼容跨平台和 URL 编码
    QUrl url(filePath);
    QString path = url.isLocalFile() ? url.toLocalFile() : filePath;
    imgView->open(path);
}

希望这些审查意见对你有所帮助!如果有任何疑问,欢迎随时提问。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, 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

@LiHua000
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 12, 2026

This pr cannot be merged! (status: unstable)

@LiHua000
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 12, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 6d56672 into linuxdeepin:master May 12, 2026
19 of 21 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