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

vterm hangs when attempting to parse an OSC sequence without a terminator #562

Open
1 of 3 tasks
danschwarz opened this issue Jun 4, 2023 · 4 comments
Open
1 of 3 tasks
Labels

Comments

@danschwarz
Copy link
Contributor

danschwarz commented Jun 4, 2023

Description:

Tested against master branch 2023-04-06:

Affected versions (if applicable)
  • master branch (specify commit)
  • Latest stable version from pypi
  • Other (specify source)
Steps to reproduce (if applicable)

run examples\terminal.py and from within the terminal shell:

echo -e "\e]0;test title\007"

Works correctly to set the terminal title.

But when sent with an illegal character, such as:

echo -e "\e]0;test title\001"

vterm hangs.

Expected/actual outcome

Wikipedia says:

The behavior of the terminal is undefined in the case where a CSI sequence contains any character outside of the range 0x20–0x7E. These illegal characters are either C0 control characters (the range 0–0x1F), DEL (0x7F), or bytes with the high bit set. Possible responses are to ignore the byte, to process it immediately, and furthermore whether to continue with the CSI sequence, to abort it immediately, or to ignore the rest of it.

I've tested with two other terminal programs - Xterm, Windows Terminal so far - and neither hang when presented with illegal characters. the unterminated OSC sequence. I think we can agree that whatever the proper behavior should be, hanging isn't it.

I haven't dug into their source code of those other terminals to see what their parsers are doing in this situation, yet.

My first attempt at fixing this "works" insofar as it prevents a crash, but it causes test_vterm to fail on the following case:

        self.connect_signal('title')
        self.write('\\e]666parsed right?\\e\\te\\e]0;test title\007st1')
        self.expect('test1')
        self.expect_signal('test title')

I don't understand the parser well enough yet to fix this situation in a way that allows the test case to complete successfully.

Input/feedback appreciated.

@danschwarz
Copy link
Contributor Author

danschwarz commented Jun 4, 2023

For what it's worth, I've done some quick testing of the '\\e]666parsed right?\\e\\te\\e]0;test title\007st1' string on Xterm, Windows Terminal, kitty, and foot. Tested by the following shell command:

echo -e "\e]666parsed right?\e\\te\e]0;test title\007st1'

All of the above terminals echo st1 back, rather than test1 as the test_vterm test case expects.

So, I don't know if the test is testing a valid and useful case?

@penguinolog
Copy link
Collaborator

I agree, that this scenario should be tested, since "remote" (for urwid) side can technically send anything. Independent from input, hang is incorrect behavior.

@danschwarz
Copy link
Contributor Author

Aha. This explains how it ought to work: https://vt100.net/emu/dec_ansi_parser

@danschwarz danschwarz changed the title vterm infinite loop when attempting to parse an OSC sequence with illegal characters vterm hangs when attempting to parse an OSC sequence without a terminator Jun 6, 2023
@danschwarz
Copy link
Contributor Author

So I've gained a better understanding of this and read through the parser docs above. The issue here is not parsing illegal characters per se. It's the fact that the OSC sequence isn't terminated by BEL or ESC \ . The parser is waiting for a terminator. You can get out of the hang by typing ^G ; once it gets BEL it will return control to you.

I'm not certain what xterm and other terminal emulators are doing (if anything) to avoid hanging in this situation. Time to read some more code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants