Junk is being inserted into the web terminal #3666
Comments
|
For whoever works on this the solution we decided on v1 is to use screen: coder/wsep#25 |
|
Faced this today on f72d8e9 |
|
I think this could be accounted to reconnecting-pty buffer size and not taking terminal escape codes into account. I can easily reproduce this problem (junk in the terminal) by making the text size really small (huge terminal) and opening a second window with the same URL, it will type junk into the session. One potential problem is the escape sequence getting cut off and not turning into an escape, another is that an escape sequence may try to put the terminal into a certain kind of mode that is dependent on another escape sequence that was lost. Thoughts @kylecarbs? The easiest solution I see is to increase buffer size, but that won't guarantee the problem is fixed. Stripping escape sequences also doesn't seem like a good idea, since we won't get identical output. So we'd be left with trying to "render" the terminal in memory and output the result, this way we can calculate the buffer needed. But that seems like a bad idea from a security perspective, and a lot of work. EDIT: I may have been off on cause/how to repro, suddenly I was no longer able to do it, but I found one way that works each time:
Didn't even need to make the font smaller. This may be unique to my setup/dotfiles but interestingly this doesn't happen without the reload/new window. |
|
I made an asciinema recording with a minimal shell setup: https://asciinema.org/a/fwLHGVx1zbIRtpYaB0isQz2mw Uses When we reload the page, we get the initial payload: But after we receive it, we send out the following response: So something in the output triggered a response. I do believe control characters at the edge of the buffer constitutes a problem as well, so we may be dealing with two separate but kind of related issues here. |
|
Yup this matches with my findings when we fixed this on v1, the client is responding to control codes in the buffer asking for its capabilities, cursor positions, things like that. But those responses are no longer expected for one reason or another (maybe the application that asked for them has exited for example) and get interpreted as regular input. I think the only reliable solutions are to actually render the terminal by using a full terminal emulator on the backend (ideally one that produces identical output to xterm.js; https://github.com/coder/v1/issues/12909#issuecomment-1226202016 |
|
Thanks for sharing that @code-asher, makes perfect sense! Your reply gives me the idea that we could just strip out the problematic escape sequences. There are not many that I know of that would be problematic. Furthermore, I believe we can eliminate a slew of problems by making the ring buffer newline aware and evicting whole lines at a time (or let the reader do it, doesn’t matter). |
|
That was actually the approach I first took but I was not sure a limited buffer would work in general; take for example a control character saying to "move down one", this will put the cursor in a different location than the first go around if there was a "move to the center of the screen" control code that has since been evicted from the buffer. To recreate the current cursor state (as just one example) from a history of movement commands you would need the full history which means a buffer of unbounded size. Otherwise I think it would work but I also found I was reading through the spec to make sure I got all the possible codes that could cause an issue and I suddenly realized I was starting down a rabbit hole of implementing a full-blown terminal emulator because I was not confident it could be reliably done by just a regex replacement or something like that and figured a proper parser was the way to go. 😆 https://invisible-island.net/xterm/ctlseqs/ctlseqs.html < I am not sure if this is the entire spec since it does not even mention capabilities. |
|
I do see where your coming from @code-asher and those are valid points. It would be quite elegant to render it server side, I agree. One problem with going the My next question would be, how do we perform terminal updates in a performant manner with SSR? Do we perform some sort of diffing for the currently rendered screen and attempt to deliver intelligent line updates or do we serialize the entire screen all the time? I'm not entirely convinced that the loss of control characters past the circular buffer will be a huge problem for v2 considering the buffer is currently 64 KB in size (which is pretty big). That's because, if we discard based on newlines, we remove many problematic cases of control characters and (hopefully) only a little bit at the start of the scroll-back buffer may be corrupted (if at all). See the following example: The newline resets the cursor position to start-of-row and is thus a pretty good indicator of "we're safe now". Some further mitigations we could do:
We could optionally also enable infinite scroll-back by storing it on the filesystem, but I guess this should be a user or terraform setting. |
|
In v1 we technically have no guarantee either, basically if you install Yeah my concern with lost control characters is mostly just academic at this point; I have not actually experimented with reproducing. I think maybe you could see it if you launch an editor like Vim and leave it running for a while with heavy use, maybe? I think it will not be such a big problem with the regular command prompt, if at all. Trying to replay the history of everything Vim/Emacs/etc has been doing seems fraught with risk to me. |
|
Another thing I tried was not buffering anything that printed in the altscreen (which would cover Vim, Emacs, etc) and then trying to send a SIGWINCH to every child process on reconnect to force it to redraw but unfortunately that did not work as some applications appear to ignore the signal. I also tried sending a resize but some applications will not redraw if the size is the same so I tried resizing smaller once and then back up to the expected size to force a redraw but that also did not work because some applications seem to debounce the resize. I am also not sure all apps even enter altscreen to begin with. |
|
I can reproduce some weirdness if I launch Emacs, split the window, open a file in one and eshell in the other, then run Actually on a related note any application that enters the altscreen starts by sending Really any control character that sets some kind of mode will have this problem, even outside the altscreen. The buffer also explains why when I reconnect sometimes I see flashes of old states (for example I see flashes of previous runs of Emacs as xterm "catches up" writing out the rest of the buffer). |
|
If we are talking about terminal then as a sperate issue. Press any key to reconnect actually doesn't work on terminal. It does reconnect on pressing Tab, Ctrl etc. |
|
If we are adverse to relying on |
@code-asher You're likely on to something there. I'm unable to reproduce any altscreen issues with vim or tmux, but that's most likely due those applications re-rendering on screen resize (which happens on reload). Exploring If we do go down the SSR path, it'd be nice to see a plan for protocol updates/bandwidth utilization. Another option would be for us to have special handling of altscreen (separate buffer?), but obviously the more exceptions we add, the closer we get to implementing a terminal emulator ourselves. 😄 |
|
I recorded a reproduction of the scroll issue with vim if you are curious what it looks like. Here I scroll a bit to show the cursor moving up/down, run a command to generate some output, and finally cut/paste it a bunch to make sure I fill up the buffer. On reload you see some flashing from old buffer contents being rendered and then finally scrolling is permanently broken (well at least until vim is restarted). vim.mp4I bet if we dig into the codes there are other reproductions that can occur due to evictions especially with modes of which there appear to be many.
Hahaha that was my thinking exactly as I was going down the rabbit hole. Every mode we check and every code we strip out gets us a little further. I think
I am not sure how With It is a bit weird since both |
|
#7765 may be related here. |
Problem
Right around the time of a Coderd restart, @Emyrk was connected to the web terminal, lost connection, reconnected, and got a bunch of junk inserted into their terminal:
We poked around in the Web inspector and found that the frontend was sending the content. It seems like there's some kind of replay buffer that's re-sending some terminal escape codes, but whatever had been listening to them is now gone and has no way of acking.
Solution
@deansheather thinks we could just block on the websocket until we're finished rendering.
The text was updated successfully, but these errors were encountered: