Skip to content

Fix: [fileinfo] Can not delete file in trash.#266

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom
GongHeng2017:202603091656-dev-eagle-fix
Mar 9, 2026
Merged

Fix: [fileinfo] Can not delete file in trash.#266
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom
GongHeng2017:202603091656-dev-eagle-fix

Conversation

@GongHeng2017
Copy link
Contributor

-- The path error in trash, so can not delete it.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-352359.htm

-- The path error in trash, so can not delete it.

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

deepin pr auto review

这段代码修改的主要目的是在构建子文件/目录的 URL 时,保留原始 URL 的 scheme(协议)和 host(主机),而不是像修改前那样强制将其转换为本地文件 URL (fromLocalFile)。这对于支持非本地文件系统(如网络挂载、云存储、SFTP 等)非常重要。

以下是对该代码修改的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 改进点:原代码 QUrl::fromLocalFile(path) 会强制将 URL 的 scheme 设为 file,这对于非本地文件系统是错误的。新代码通过 QUrl nextUrl = url; nextUrl.setPath(path); 复制了原始 URL 的属性(如 scheme、host、query 等),仅修改了路径,逻辑更加严谨,符合跨协议文件访问的需求。
  • 潜在问题
    • 路径拼接逻辑:代码中 path 的构建逻辑是 dirPath + "/" + fileName。如果 dirPath 本身包含非标准字符(如连续的斜杠 //)或者 fileName 包含路径分隔符(如 sub/file),setPath 可能会产生非预期的路径结构。虽然 Qt 的 QUrl 通常能规范化路径,但最好确保 fileName 是纯净的文件名。
    • UserInfo 逻辑:代码继续保留了 if (url.userInfo().startsWith("originPath::")) 的逻辑。这看起来是一种通过 userInfo 字段传递额外元数据的 hack 方式(例如在挂载点中保留原始路径)。如果 fileName 包含特殊字符,直接拼接到 userInfo 中可能会导致 URL 解析问题或安全风险(见下文)。

2. 代码质量

  • 可读性:代码逻辑清晰,注释 // 保留原始 URL 的 scheme 和 host,而不是假定为本地文件 准确解释了修改意图,有助于后续维护。
  • 健壮性:直接使用 url 作为模板创建 nextUrl 是一种很好的做法,因为它保留了所有原始上下文。但是,需要确保 url 本身是有效的。如果传入的 url 是空的或格式错误的,setPath 可能无法按预期工作。

3. 代码性能

  • 性能影响:新旧代码在性能上几乎没有差异。QUrl 的赋值操作是浅拷贝(隐式共享),开销很小。setPath 和字符串拼接操作也是标准操作。
  • 建议:字符串拼接 dirPath + "/" + QString(fileName) 在高频调用下(如遍历大量文件)可能会产生频繁的内存分配。如果性能是关键瓶颈,可以考虑使用 QStringBuilder(通过包含 <QStringBuilder> 并使用 QT_USE_QSTRINGBUILDER 宏)或预分配内存,但在此处通常不是必要优化。

4. 代码安全

  • 路径遍历风险
    • 如果 fileName 来自外部输入(如用户输入或不受信任的文件系统),且包含 ../ 序列,拼接后的 path 可能会指向目录之外的位置。
    • 建议:在拼接前,应检查 fileName 是否包含路径遍历字符(如 /\),或者使用 QFileInfo 进行规范化,确保生成的路径仍在预期范围内。
  • URL 注入风险
    • nextUrl.setUserInfo(url.userInfo() + ...) 这一行直接将 fileName 拼接到 userInfo 中。如果 fileName 包含 URL 特殊字符(如 @, :, /),可能会导致 URL 结构被破坏,甚至被解析为不同的 host 或 scheme。
    • 建议:如果必须将 fileName 放入 userInfo,应对其进行 URL 编码(例如使用 QString::toUtf8().toPercentEncoding()),但这可能会破坏下游解析逻辑。更好的做法是重新设计元数据传递机制,避免在 userInfo 中拼接复杂字符串。

总结与建议

这段代码修改在逻辑上是正确且必要的,修复了非本地文件系统支持的问题。为了进一步提高代码质量,建议进行以下改进:

  1. 输入验证:确保 fileName 不包含路径分隔符,防止路径遍历。
  2. URL 安全:如果 fileName 必须拼接到 userInfo,请确保下游解析逻辑能正确处理特殊字符,或考虑使用更安全的元数据传递方式。
  3. 路径规范化:在调用 setPath 之前,可以使用 QDir::cleanPath(path) 清理路径中的冗余分隔符和 ./..,确保路径格式规范。

改进后的代码示例(仅供参考):

QUrl DEnumeratorPrivate::buildUrl(const QUrl &url, const char *fileName)
{
    QString path;
    const QString &dirPath = url.path();
    const QString &fileStr = QString::fromUtf8(fileName); // 假设 fileName 是 UTF-8 编码

    // 检查 fileName 是否包含路径分隔符,防止路径遍历
    if (fileStr.contains('/') || fileStr.contains('\\')) {
        qWarning() << "Invalid file name with path separators:" << fileStr;
        return QUrl(); // 返回无效 URL 或进行错误处理
    }

    if (dirPath.isEmpty() || dirPath == "/") {
        path = "/" + fileStr;
    } else {
        path = dirPath.endsWith('/') ? dirPath + fileStr : dirPath + "/" + fileStr;
    }

    // 规范化路径,去除冗余的 / 和 .
    path = QDir::cleanPath(path);

    // 保留原始 URL 的 scheme 和 host
    QUrl nextUrl = url;
    nextUrl.setPath(path);

    if (url.userInfo().startsWith("originPath::")) {
        // 警告:直接拼接可能存在安全风险,确保 fileName 不包含特殊字符
        nextUrl.setUserInfo(url.userInfo() + "/" + fileStr);
    }

    return nextUrl;
}

这个改进版本增加了输入验证和路径规范化,提高了代码的健壮性和安全性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, Johnson-zs, 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
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Mar 9, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 203d828 into linuxdeepin:develop/eagle Mar 9, 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.

5 participants