Skip to content

fix(emulation): handle DECRQM mode queries to fix spurious 'pp' in vim#516

Merged
lzwind merged 1 commit intolinuxdeepin:masterfrom
JWWTSL:fix/decrqm-mode-query
Apr 16, 2026
Merged

fix(emulation): handle DECRQM mode queries to fix spurious 'pp' in vim#516
lzwind merged 1 commit intolinuxdeepin:masterfrom
JWWTSL:fix/decrqm-mode-query

Conversation

@JWWTSL
Copy link
Copy Markdown
Contributor

@JWWTSL JWWTSL commented Apr 15, 2026

Absorb the '$' CSI intermediate byte in the tokenizer and dispatch the following 'p' final byte through the existing TY_CSI_PR/TY_CSI_PS paths. Add reportAnsiMode() and reportDecMode() helpers to send proper DECRPM replies for the ANSI and DEC modes already tracked by the emulation.

This fixes garbage characters (e.g. 'pp') appearing in the top-left corner when launching vim, which was caused by unhandled DECRQM queries falling through to reportDecodingError().

Ref: lxqt/qtermwidget#628

@JWWTSL JWWTSL force-pushed the fix/decrqm-mode-query branch 2 times, most recently from 09892fd to 0496967 Compare April 16, 2026 03:22
Absorb the '$' CSI intermediate byte in the tokenizer and dispatch the
following 'p' final byte through the existing TY_CSI_PR/TY_CSI_PS paths.
Add reportAnsiMode() and reportDecMode() helpers to send proper DECRPM
replies for the ANSI and DEC modes already tracked by the emulation.

This fixes garbage characters (e.g. 'pp') appearing in the top-left
corner when launching vim, which was caused by unhandled DECRQM queries
falling through to reportDecodingError().

Ref: lxqt/qtermwidget#628

pms: bug-356801
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码主要是为终端模拟器添加了 DECRQM(请求模式)和 DECRPM(报告模式)的支持,这是 VT100/VT102 终端标准的一部分,用于查询和报告终端的当前设置状态。

以下是对该代码的详细审查意见:

1. 语法逻辑

  • eec('$') 的副作用与潜在 Bug

    • 问题:在 receiveChar 中添加了 if (eec('$')) { return; }。正如代码注释中 WARNING 所指出的,这会"吸收所有 CSI 序列中的 '$',而不仅仅是 DECRQM"。
    • 分析:在 VT100/VT300 系列标准中,$ 是一个中间字符,用于区分 DECRQM(请求模式)与其他序列(如 DECCRA - 矩形复制,DECSERA - 选择性擦除矩形)。目前的实现逻辑是:一旦在 CSI 序列中遇到 $,就立即返回并等待下一个字符。这意味着如果终端接收到 CSI ... $ p(DECRQM),它会被正确处理。但是,如果接收到 CSI ... $ v(DECCRA),它也会在 $ 处暂停,等待 v。由于后续的 processToken 逻辑似乎并没有专门处理带有 $ 中间字符的 DECCRA 等序列(除非 lec 逻辑能处理,但通常 lec 处理的是特定的定长序列),这可能导致这些序列被部分解析或被忽略。
    • 风险:如果应用程序发送了使用 $ 作为中间字符但不是 DECRQM 的序列(例如某些高级终端特性),该终端模拟器可能无法正确执行,或者仅仅是"吞掉"了 $ 而导致后续解析错乱。
    • 建议:这是针对该特定 tokenizer 架构的一种权衡。如果该终端模拟器主要目标是兼容主流应用(如 bash, vim, tmux),它们很少发送 DECCRA 等高级 DEC 图形序列,那么这种实现是可以接受的。但如果追求完整的 VT 兼容性,应该改进 tokenizer 的状态机,明确区分"等待中间字符"和"等待最终字符"的状态,而不是简单地拦截特定字符。
  • reportAnsiModereportDecMode 的缓冲区大小

    • 问题:这两个函数使用 char tmp[32] 作为缓冲区。
    • 分析:格式化字符串为 "\033[%d;%d$y""\033[?%d;%d$y"
      • mode 参数最大值在代码中为 2004。
      • status 参数为 1, 2 或 4。
      • 最大长度计算:\033 (1) + [? (2) + 2004 (4) + ; (1) + 4 (1) + $y (2) + \0 (1) = 12 字节。
    • 结论:32 字节的缓冲区非常安全,没有溢出风险。snprintf 的检查也是良好的防御性编程实践。

2. 代码质量

  • 注释详细:代码中关于 DECRQM 的注释非常清晰,解释了 $ 的处理方式以及 TY_CSI_PSTY_CSI_PR 的生成逻辑,这大大提高了可维护性。
  • 一致性:新增的模式报告代码(case TY_CSI_PR('p', ...))与现有的 processToken 风格保持一致。
  • 硬编码状态值:在 reportAnsiMode 调用中,对于不支持的模式(如 KAM, HEM),直接传入了硬编码的返回值(如 reportAnsiMode(2, 2))。虽然符合标准(2 表示 Mode Not Recognized/Not Set),但建议定义一个常量,例如 const int MODE_NOT_RECOGNIZED = 2;const int MODE_PERMANENTLY_RESET = 4;,以提高代码可读性。

3. 代码性能

  • 字符串格式化reportAnsiModereportDecMode 每次调用都会执行 snprintf。考虑到这些函数通常只在主机(如 shell 应用程序)显式查询时触发,频率极低,因此性能开销可以忽略不计。
  • Tokenizer 分支:在 receiveChar 中增加了一个 if (eec('$')) 判断。这是在字符处理的主循环中,但对于非 CSI 序列或 CSI 序列中不包含 $ 的情况,这只是一个简单的字符比较,性能影响微乎其微。

4. 代码安全

  • 缓冲区溢出防护:使用了 snprintf 并检查返回值 r >= sz,这是防止缓冲区溢出的正确做法,值得肯定。
  • 输入验证processToken 中的 case 语句覆盖了特定的模式 ID。如果终端接收到一个未在 switch 中列出的 DECRQM 查询(例如 CSI ? 9999 $ p),它将落入 default 分支(假设有 default,或者直接跳出 switch)。这意味着终端将忽略该查询而不做任何响应。根据 VT 标准,对于无法识别的模式,终端应该返回 "Not recognized" (通常是 CSI ? Pd ; 0 $ y 或 CSI ? Pd ; 3 $ y,具体取决于实现,标准建议是 3 表示 Invalid,但代码中用 2 表示 reset/not recognized,4 表示 permanently reset)。
    • 改进建议:虽然当前代码对未列出的模式不做响应(忽略),这通常不会导致安全漏洞,但严格遵循标准应该回复一个错误状态。不过,鉴于这是终端模拟器的被动行为,不响应通常比响应错误更安全。

5. 改进建议总结

  1. 处理 $ 中间字符的通用性:虽然当前的 eec('$') hack 对 DECRQM 有效,但它破坏了其他可能使用 $ 的序列。建议长期改进 tokenizer 架构,支持多中间字符的完整解析路径。
  2. 使用常量定义状态码:将 2, 4 等状态码定义为有意义的常量(如 MODE_STATUS_RESET, MODE_STATUS_PERMANENTLY_RESET),便于阅读和维护。
  3. 默认行为:确认 processTokenswitch 语句是否有 default 分支。对于未知的 DECRQM 请求,最好显式地回复一个"未识别"或"不支持"的状态码,而不是静默忽略,这样调试终端程序时会更容易发现问题。

修正后的代码片段示例(针对建议2):

// 在类定义或头文件中定义常量
private:
    static const int MODE_STATUS_RECOGNIZED_SET = 1;
    static const int MODE_STATUS_RECOGNIZED_RESET = 2;
    static const int MODE_STATUS_PERMANENTLY_RESET = 4;
    // ...

// 在 processToken 中使用
case TY_CSI_PS('p', 2) : reportAnsiMode(2, MODE_STATUS_RECOGNIZED_RESET); break;
case TY_CSI_PR('p', 4) : reportDecMode(4, MODE_STATUS_PERMANENTLY_RESET); break;

总体而言,这段代码在功能实现上是正确的,增加了对重要终端特性的支持,且具备基本的防御性编程措施。主要的妥协在于对 $ 字符的通用处理,这在实际应用中通常是可接受的。

@lzwind lzwind merged commit 71b6c7c into linuxdeepin:master Apr 16, 2026
16 checks passed
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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