search

shell-pool / shpool

Think tmux, then aim... lower
Apache License 2.0
1.15k stars 20 forks source link

shpool splices the motd in between `clear` control codes #15

Closed ethanpailes closed 4 months ago

ethanpailes commented 4 months ago

What happened

The first shpool prompt gets a bit mangled when in motd = "dump" mode

What I expected to happen

The prompt should look normal

To Reproduce

put motd = "dump" in your config file, then make a new session on a machine with a message of the day.

Version info

0.6.0, but I forgot to update the version command output in the Google internal version of shpool, whoops.

Logs

n/a I know what is going on

ethanpailes commented 4 months ago

I took a look at this and I think I have figured out what is going on. When shpool creates a new shell, it immediately injects a few commands in order to set up the prompt prefix, and to avoid having the setup commands clutter up the shell, we always finish up with a clear command. This will stomp on the motd if we inject the motd immediately, so before showing the motd we scan for the the control codes emitted by clear. In particular, we scan for the command codes for the clear_screen terminfo capability, which the clear binary should be using as well. The problem seems to be that clear is issuing more than just the clear_screen commands. The raw output of clear (encoded as space separated decimal bytes since that is easy to log from rust) is

27 91 72 27 91 50 74 27 91 51 74

but currently, we are only matching on the 27 91 72 27 91 50 74 prefix, since that is the clear_screen commands for most (maybe all?) terminals this is probably pretty standard. This means we are splicing the motd into the output in between these two control codes rather than after both of them as we should be doing. I'm not sure what the 27 91 51 74 control code is yet, I'll have to dig into clear sources to try to figure out out, but once I do this fix should be pretty straightforward.

ethanpailes commented 4 months ago

I grabbed the source for clear with

$ dpkg -S $(which clear)
ncurses-bin: /usr/bin/clear
$ sudo apt-get source ncurses-bin
ethanpailes commented 4 months ago

I don't think it is a string capability. I dumped out all the control codes for all the termini StringCapability values and none of them produce 27 91 51 74. My best guess right now is that it is a NumberCapability that is doing something like resetting the cursor position.

The clear c code is not all that easy to follow.

I guess I can just hard code the suffix, but that feels wrong.

ethanpailes commented 4 months ago

https://stackoverflow.com/questions/37774983/clearing-the-screen-by-printing-a-character discusses the codes that clear generates a bit. I'm still not quite sure what query I should pose to the terminfo database to generate the second code though.

ethanpailes commented 4 months ago

I decided to just give up on finding the principled terminfo query and just fork a clear subprocess for each TERM value to empirically determine the control codes emitted.

It also turned out that the splicing logic was buggy and we would accidentally inject a bunch of trailing zeros in the next chunk after injecting the message of the day.