Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for unexpected C0 inside CSI sequence #158

Closed
o-sdn-o opened this issue Jan 17, 2022 · 4 comments · Fixed by #159
Closed

Support for unexpected C0 inside CSI sequence #158

o-sdn-o opened this issue Jan 17, 2022 · 4 comments · Fixed by #159
Assignees
Labels
bug Something isn't working Terminal Built-in terminal emulator

Comments

@o-sdn-o
Copy link
Collaborator

o-sdn-o commented Jan 17, 2022

repro:

  • Interpret the following sequence CSI \r 34 m or CSI 34 \r m as \r CSI 34 m
@o-sdn-o o-sdn-o added the bug Something isn't working label Jan 17, 2022
@o-sdn-o o-sdn-o self-assigned this Jan 17, 2022
@ghost
Copy link

ghost commented Jan 18, 2022

Have you seen the Paul Flo Williams parser? It might be useful for you. It covers everything except one sequence from VT52 submode (direct cursor position).

@o-sdn-o
Copy link
Collaborator Author

o-sdn-o commented Jan 18, 2022

Have you seen the Paul Flo Williams parser?

Thanks for the awesome link! This helped a lot in understanding how to deal with control characters like C0 inside sequences. It seems to me that I once saw this document a long time ago, but did not give it due attention.

I understand that if you're going to emulate a real terminal, you should match all observable behavior described in this document, but there are several nuances dictated by realities:

In my implementation, the receiving buffer accumulates data until it sees a completed sequence and a valid (not broken codepoint) UTF-8 sequence at the end, and only then passes this data to the parser. In my case, the sequence CSI 3 ; 1 CSI 2 J will be executed differently than in a compatible parser, and I have not yet decided whether such compatibility should be supported here.

@ghost
Copy link

ghost commented Jan 18, 2022

A couple comments:

  • UTF-8 processing as per the Unicode spec should happen before all VT processing, e.g. "decode everything, then apply VT parser". So the 8-bit C1 controls will be decoded from 3-byte UTF-8 sequences. Unfortunately X10 encoded mouse protocol (which no one uses anymore, we all use UTF or SGR mode) expects mouse position coordinates to be in 8-bit bytes, so positions outside 160, 64 can corrupt the UTF-8 stream.

  • C0's can go into OSC sequences no problem, but ASCII BEL (0x07) is typically used to terminate OSC sequences, along with ST. I don't know why that got started, but now tons of terminals use BEL as their preferred OSC terminator. 🤷‍♀️ But otherwise you should be able to pass CSI's and C0's through OSC OK.

  • Don't forget APC. It's used by Kitty image protocol, so at least be able to discard it for a clean display.

  • If you're passing negative numbers for CSI inside your own parser, that's fine. But negative numbers inside CSI for protocols that other terminals will see (e.g. mouse mode 1016) is considered a bug by some. xterm is doing it, but we do not know if that is deliberate or an oversight.

  • You can also use the semicolon-separated form of SGR 38/48 ("Konsole" encoding), that was at one time a lot more commonly supported out there. But colon-separated is fine too, xterm supports that and many other modern terminals do too.

Glad that doc reference helped. :) I mentioned it because in addition to supporting your own terminal, if you look into others some of them are modeled after that specific parser with very similar state names. It can help having a similar "map" when you are working on bugs for them, or cherry-picking features you'd like for yourself.

(Also happy to see @ismail-yilmaz around. He's got a very nice embeddable terminal of his own. :) )

@o-sdn-o
Copy link
Collaborator Author

o-sdn-o commented Jan 18, 2022

But negative numbers inside CSI for protocols that other terminals will see (e.g. mouse mode 1016) is considered a bug by some. xterm is doing it, but we do not know if that is deliberate or an oversight.

For the record, MidnightCommander's input parser does not discard the entire sequence when it receives a minus sign

https://github.com/MidnightCommander/mc/blob/cbcda8a284ab9e3d5498fb255a5059c9f86c9f46/lib/tty/key.c#L779-L802

and extra characters appear in the input.

I try to only use the minus for those applications that can interpret it correctly (something like DECSET 10060 mode).

@o-sdn-o o-sdn-o mentioned this issue Jan 19, 2022
@o-sdn-o o-sdn-o added the Terminal Built-in terminal emulator label Feb 3, 2022
@o-sdn-o o-sdn-o changed the title [Term] Catch and execute an unexpected C0 inside CSI sequence Support for unexpected C0 inside CSI sequence May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Terminal Built-in terminal emulator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant