Skip to content

feat(util): support full Unicode with UTF-8 byte escaping#558

Merged
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
ComixHe:master
Apr 27, 2026
Merged

feat(util): support full Unicode with UTF-8 byte escaping#558
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
ComixHe:master

Conversation

@ComixHe
Copy link
Copy Markdown
Contributor

@ComixHe ComixHe commented Apr 27, 2026

Desktop name and object path specs require ASCII-only. Now escapes all UTF-8 bytes to support Chinese, Japanese, Korean, etc.

Bug: PMS-357251

Desktop name and object path specs require ASCII-only.
Now escapes all UTF-8 bytes to support Chinese, Japanese, Korean,
etc.

Bug: PMS-357251

Signed-off-by: ComixHe <heyuming@deepin.org>
@ComixHe ComixHe requested a review from BLumia April 27, 2026 07:35
Signed-off-by: ComixHe <heyuming@deepin.org>
@ComixHe ComixHe requested a review from 18202781743 April 27, 2026 08:01
@deepin-ci-robot
Copy link
Copy Markdown
Contributor

deepin pr auto review

这段代码主要进行了一次代码重构,将头文件中的内联函数实现移到了源文件中,并对部分函数实现进行了优化。以下是对该代码的详细审查意见:

1. 语法逻辑审查

优点:

  • 代码将实现从头文件移至源文件,符合良好的代码组织原则,有助于减少编译依赖和编译时间。
  • 函数签名保持一致,并添加了 noexcept 标记,表明这些函数不会抛出异常,这是一个好的实践。
  • escapeToObjectPath 函数新增了 QByteArray 重载,并让 QString 版本调用它,实现了代码复用。

问题和改进建议:

  • 版权年份更新:头文件中的版权年份从 2023 更新到 2026,但新创建的 dutil.cpp 文件也使用了 2026 年。通常版权年份应使用当前年份或包含代码实际编写和修改的年份范围。建议确认 2026 是否为预期年份。
  • escapeToObjectPath 函数逻辑
    • 新实现中,if (std::isalnum(byte) != 0 || byte == '/') 这一行,std::isalnum 的参数类型是 int,且要求参数值必须是 unsigned charEOF。虽然这里进行了 static_cast<unsigned char>,但最好确保 byte 的值在 unsigned char 范围内(0-255)。
    • 注释提到 "a valid dbus object path component only allows '[A-Z][a-z][0-9]_'",但代码中保留了 / 字符。需要确认这是否符合 D-Bus 对象路径规范,或者是否有特殊用途。
    • ret.reserve(str.size() * 3) 预留空间是合理的,因为每个非字母数字字符可能被替换为 3 个字符(_ + 两位十六进制)。
  • unescapeFromObjectPath 函数逻辑
    • if (i <= str.length() - 3 && str.at(i) == u'_') 这里的边界检查是正确的,但 str.length() 返回 qsizetype(通常是 ssize_t),而 i 也是 qsizetype,所以比较是安全的。
    • str.mid(i + 1, 2).toUShort(&ok, 16) 这里将两个字符转换为 unsigned short,然后转换为 unsigned char。如果十六进制字符串表示的值超过 255(如 "_FF"),toUShort 会成功,但转换为 unsigned char 时会发生截断。虽然 D-Bus 对象路径通常不会包含这样的值,但最好添加检查:
      if (ok && byte <= 255) {
          ret.append(static_cast<char>(byte));
          i += 3;
          continue;
      }
  • getAppIdFromAbsolutePathgetAbsolutePathFromAppId 函数
    • 这两个函数的逻辑与原实现基本一致,但移到了源文件中。逻辑上没有明显问题,但可以进一步优化性能(见下文)。

2. 代码质量审查

优点:

  • 代码结构清晰,函数职责单一。
  • 使用了 noexcept 标记,提高了代码的可靠性和性能(避免异常处理开销)。
  • 新增了 QByteArray 重载的 escapeToObjectPath,提高了灵活性。

问题和改进建议:

  • 魔法字符串
    • static QString desktopSuffix{u8".desktop"};"applications" 是硬编码的字符串。建议将这些定义为常量或宏,以提高可维护性:
      static constexpr const char* DESKTOP_SUFFIX = u8".desktop";
      static constexpr const char* APPLICATIONS_DIR = "applications";
  • QStringQByteArray 的混用
    • escapeToObjectPath 中,QString 版本调用 str.toUtf8(),然后调用 QByteArray 版本。这是合理的,但需要注意 toUtf8() 可能会失败(对于无效的 Unicode 字符),尽管 Qt 通常会替换无效字符。
  • unescapeFromObjectPath 的性能
    • str.mid(i + 1, 2) 会创建一个临时 QString,这在循环中可能影响性能。可以直接访问字符:
      if (i + 2 < str.length() && str.at(i) == u'_') {
          bool ok{false};
          QString hexStr = QString(str.at(i + 1)) + str.at(i + 2);
          auto byte = static_cast<unsigned char>(hexStr.toUShort(&ok, 16));
          if (ok) {
              ret.append(static_cast<char>(byte));
              i += 3;
              continue;
          }
      }
      但这样仍然会创建临时 QString。更高效的方法是手动解析十六进制:
      if (i + 2 < str.length() && str.at(i) == u'_') {
          QChar c1 = str.at(i + 1);
          QChar c2 = str.at(i + 2);
          if (isHexDigit(c1) && isHexDigit(c2)) {
              unsigned char byte = (hexValue(c1) << 4) | hexValue(c2);
              ret.append(static_cast<char>(byte));
              i += 3;
              continue;
          }
      }
      其中 isHexDigithexValue 是辅助函数。

3. 代码性能审查

优点:

  • escapeToObjectPath 使用了 reserve 预分配内存,减少了动态分配的开销。
  • unescapeFromObjectPath 也使用了 reserve

问题和改进建议:

  • getAppIdFromAbsolutePathgetAbsolutePathFromAppId 的性能
    • getAppIdFromAbsolutePath 中,path.chopped(desktopSuffix.size()) 会创建一个新的 QString,然后 split 也会创建多个临时字符串。可以考虑使用 QStringView 或迭代器来避免临时对象的创建。
    • getAbsolutePathFromAppId 中,appId.split('-', Qt::SkipEmptyParts) 会创建一个 QStringList,然后循环中多次调用 QStringList::joinQDir::exists。如果 appDirscomponents 很大,可能会有性能问题。可以考虑缓存结果或使用更高效的数据结构。
  • escapeToObjectPath 的性能
    • QString::number(byte, 16).rightJustified(2, u'0').toLower() 会创建多个临时 QString。可以优化为:
      const char hexDigits[] = "0123456789abcdef";
      ret.append(u'_');
      ret.append(hexDigits[byte >> 4]);
      ret.append(hexDigits[byte & 0x0F]);

4. 代码安全审查

问题和改进建议:

  • unescapeFromObjectPath 的安全性
    • 如果输入的 str 包含无效的十六进制序列(如 "_G1"),toUShort 会失败,但代码会继续处理下一个字符。这是合理的,但需要确保不会导致缓冲区溢出或越界访问。
    • 如果 str 包含非常长的序列(如大量的 _ 后跟非十六进制字符),ret.reserve(str.length()) 可能不足以容纳解码后的数据(虽然通常解码后的数据不会比原始字符串长)。但更安全的做法是:
      ret.reserve(str.length() / 3 * 2); // 假设大部分是转义序列
  • escapeToObjectPath 的安全性
    • 如果 str 包含大量的非字母数字字符,ret.reserve(str.size() * 3) 可能会分配过多的内存。可以考虑限制最大长度或分批处理。
  • getAppIdFromAbsolutePathgetAbsolutePathFromAppId 的安全性
    • 这些函数依赖于 QStandardPaths::standardLocations 和文件系统操作,需要确保路径是可信的。如果输入的 pathappId 来自不可信来源,可能会导致路径遍历攻击。建议添加路径验证或规范化。

5. 其他建议

  • 测试覆盖
    • 建议为这些函数添加单元测试,特别是边界情况(如空字符串、无效十六进制序列、超长字符串等)。
  • 文档
    • 建议为这些函数添加详细的文档,说明其用途、参数、返回值和可能的异常情况(虽然标记为 noexcept)。
  • 兼容性
    • 注释提到 "for compatibility with existing applications, we escape all unicode to avoid breakage",建议明确这种兼容性 hack 的预期移除时间或条件。

总结

这次重构总体上是积极的,提高了代码的组织性和可维护性。但在性能和安全性方面仍有优化空间,特别是字符串处理和路径操作部分。建议根据实际使用场景进行进一步的优化和测试。

@deepin-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, ComixHe

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

@ComixHe
Copy link
Copy Markdown
Contributor Author

ComixHe commented Apr 27, 2026

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented Apr 27, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 18eaf24 into linuxdeepin:master Apr 27, 2026
17 of 19 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