-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Can xterm attach use socket. io? #1972
Comments
@BengBu-YueZhang Possible, but you will have to provide your own attach functionality. Under the hood the terminal object has a |
@AhanM Is this a question how to fix the remaining issues? Basically you have to make sure that the transport encoding done by socketIO does not mess up the input bytes. Terminal editors typically make heavy usage of control characters and sequences, your screenshot above looks like those get messed up for some reason. |
@jerch Yes I am trying to fix vim and nano output. I am using Also I am fairly certain that socket.io does support binary transport. |
@AhanM And how does the server part look like? |
The backend is a python flask server that uses flask-socketio. I have a worker that reads terminal output in a background thread using a library called Paramiko. Here is the server side code I use to read the terminal output: def read(self) -> None:
""" Attempt to read from the channel stdout and send back to socket client """
logging.info('Worker ' + self.id + ' reading')
try:
data = self.channel.recv(BUF_SIZE)
except Exception as e:
logging.error('Closing worker {} due to exception: {}'.format(self.id, str(e)))
raise e
if not data:
self.close(reason='Channel closed')
logging.error('Closing worker {} due to reason: {}'.format(self.id, 'Channel closed'))
return
else:
data = data
self.sio.emit('message', data) |
@AhanM The important bit here is the way, how you transmit pty chunks to xterm.js |
@jerch So this is a unique case since I am not running a pseudo terminal locally on the server but ssh-ing into another machine (which has an important resource). This is the reason I am using Paramiko and not pty |
@AhanM Does the paramiko component send bytestrings (byte arrays) or UTFxy encoded strings? This is somewhat tricky to get done right with Python3, in Python2 the normal string type would be useable directly as byte container. Does the paramiko part work correctly if you attach it to a local terminal (outputting it directly to gnome-terminal or xterm)? |
I had the same problem as @AhanM and discovered that Socket.io is attaching null characters to each key press. I was able to fix that using str.replace('\0', '') at the end of each string. Discovered this when pressing 'g' (and any other character, respectively) and I was generating this. abs2str: 'g\u0000' This fixed the breaking issue using nano. |
@jzombie do you str.replace in the attach plugin? |
@jzombie: Wow thats awkward, if thats the case I'd call this a bug in SocketIO. Are you sure that you use string transport and not binary? Dont know anything about SocketIO internals, still binary would preserve null termination as part of a message. |
I got xterm to work w/ Socket.io to work by using xterm's write (not using writeUtf8) and using these methods on both the client and server. Socket.io must be sending a null character with the string, so when converting to a string, I added str.replace('\0', '') to the end of it. Haven't thoroughly tested but I can now type horizontally in nano, and use the hot keys as they should be.
|
@jzombie In which direction (pty --> xterm.js vs. xterm.js --> pty) does that apply? Also note that |
I did it on both ends. Taking keypresses, running them through str2ab, and running ab2str on the server. And then running str2ab from the server, and ab2str on the client. Just using xterm.write(), not writeUtf8() here. |
Yeah we currently have no way to deliver raw bytes from xterm.js to the backend. #2362 solves this, with that it should be possible to run with socketIO directly in binary mode (the \0 should vanish with this). |
Not sure why you have so many problems to set this up, I basically hacked socketIO into our demo with a few lines of code and it works without any issues. Here is the diff to the current master: diff --git a/demo/client.ts b/demo/client.ts
index 040292e4..f9a909bd 100644
--- a/demo/client.ts
+++ b/demo/client.ts
@@ -37,6 +37,7 @@ export interface IWindowWithTerminal extends Window {
WebglAddon?: typeof WebglAddon;
}
declare let window: IWindowWithTerminal;
+declare let io: any;
let term;
let fitAddon: FitAddon;
@@ -157,10 +158,15 @@ function createTerminal(): void {
res.text().then((processId) => {
pid = processId;
socketURL += processId;
- socket = new WebSocket(socketURL);
- socket.onopen = runRealTerminal;
- socket.onclose = runFakeTerminal;
- socket.onerror = runFakeTerminal;
+ // socket = new WebSocket(socketURL);
+ // socket.onopen = runRealTerminal;
+ // socket.onclose = runFakeTerminal;
+ // socket.onerror = runFakeTerminal;
+
+ const ioSocket = io('localhost:3001');
+ ioSocket.emit('attach', processId);
+ ioSocket.on('data', data => term.write(data));
+ term.onData(data => ioSocket.emit('data', data));
});
});
}, 0);
@@ -172,7 +178,7 @@ function runRealTerminal(): void {
* To run it with UTF8 binary transport, swap comment on
* the lines below. (Must also be switched in server.js)
*/
- term.loadAddon(new AttachAddon(socket));
+ // term.loadAddon(new AttachAddon(socket));
// term.loadAddon(new AttachAddon(socket, {inputUtf8: true}));
term._initialized = true;
diff --git a/demo/index.html b/demo/index.html
index ef8f3891..1de9a6e7 100644
--- a/demo/index.html
+++ b/demo/index.html
@@ -7,6 +7,7 @@
<link rel="stylesheet" href="/style.css" />
<script src="https://cdnjs.cloudflare.com/ajax/libs/es6-promise/4.1.1/es6-promise.auto.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/fetch/1.0.0/fetch.min.js"></script>
+ <script src="socket.io.js"></script>
</head>
<body>
<h1 style="color: #2D2E2C">xterm.js: A terminal for the <em style="color: #5DA5D5">web</em></h1>
diff --git a/demo/server.js b/demo/server.js
index 8955431b..aa806505 100644
--- a/demo/server.js
+++ b/demo/server.js
@@ -13,6 +13,8 @@ const USE_BINARY_UTF8 = false;
function startServer() {
var app = express();
expressWs(app);
+ var http = require('http').createServer(app);
+ var io = require('socket.io')(http);
var terminals = {},
logs = {};
@@ -36,6 +38,10 @@ function startServer() {
res.sendFile(__dirname + '/dist/client-bundle.js');
});
+ app.get('/socket.io.js', function(req, res){
+ res.sendFile(__dirname + '/socket.io.js');
+ });
+
app.post('/terminals', function (req, res) {
const env = Object.assign({}, process.env);
env['COLORTERM'] = 'truecolor';
@@ -71,6 +77,20 @@ function startServer() {
res.end();
});
+ io.on('connection', function(socket) {
+ // console.log(socket);
+ console.log('a user connected');
+ socket.on('disconnect', function(){
+ console.log('user disconnected');
+ });
+ socket.on('attach', pid => {
+ console.log('attaching to terminal', pid);
+ var term = terminals[parseInt(pid)];
+ term.on('data', data => socket.emit('data', data));
+ socket.on('data', data => term.write(data));
+ });
+ });
+
app.ws('/terminals/:pid', function (ws, req) {
var term = terminals[parseInt(req.params.pid)];
console.log('Connected to terminal ' + term.pid);
@@ -135,6 +155,10 @@ function startServer() {
console.log('App listening to http://127.0.0.1:' + port);
app.listen(port, host);
+
+ http.listen(3001, function(){
+ console.log('listening on *:3001');
+ });
} Note that this not well integrated or cleaned up, also had to use another port to get it quickly working. But it should show enough basics to get something similar to the attach addon done. Also I dont see any '\0' at the end of a message? |
@jerch it's because you're not using the |
Ofc I am not using attach addon, as it is hardcoded against WebSocket. It has to be changed to socketIO semantics. But how does that answer my question? Using both the standard attach addon and a socketIO socket at once makes no sense. |
@jerch if you read my original post I explained how I modified the |
@AhanM Ah sorry forgot about your post above, and my answer was more directed to @jzombie. Basically your changes to attach addon look good, there are just a few things i dont know if they will work:
In general maybe its easier to stick with string transport for now, with #2362 it will be much easier with binary (like raw utf8). Feel free to test the PR if you want to mess with binary transport. Edit: Do you also see those null termination garbage @jzombie mentioned? |
@jerch I literally just found this code in other places and kept putting things together until it worked. The online tutorials I found used Uint16Arrays and my keyboard presses were all jacked up when trying that. Took a lot of trial and error. |
And then modified it accordingly when I discovered the issue. Here's the original thread: https://stackoverflow.com/questions/6965107/converting-between-strings-and-arraybuffers/11058858#11058858 |
Even though it was originally written for a different task at hand, I've gotten it to work in a Docker instance running xterm in a GUI. However, someone said they did not have these problems with a Mac when running vi in it. Somehow the problems only affected me in the Linux version. |
And I never saw the \0000 until I did an echo myself on the server, showing key presses coming from the client. a\0000, b\0000, c\0000 |
@jzombie Ah ok. The Uint16Array stuff you found is to convert JS string into UTF16 codepoints. But that will not work here, as websockets (yes socketIO still uses websockets underneath) dont use/know UTF16 at all, they use UTF8 for string data, and raw bytes for Buffer/Uint8Arrays. Also both ends - server part and client part (attach addon) have to agree whether they send string data or binary (raw utf8 byte) data. So with sebsockets you have basically these two options:
// browser send
websocket.send('some string')
// server receive
wsSocket.on('message', data => ...) // data will be a JS string type automatically
// server send
wsSocket.send('some string data')
// browser receive
websocket.onmessage = (data) => ... // data will be a JS string type automatically
// browser send
websocket.send(Uint8Array | ArrayBuffer)
// server receive
wsSocket.on('message', data => ...) // data will be a Buffer type
// server send
wsSocket.send(Buffer.from(...))
// browser receive
websocket.binaryType = 'arraybuffer';
websocket.onmessage = (data) => ... // data will be an ArrayBuffer type With socketIO, as far I got it, its even easier, as it will handle the type switches automatically (no need to mess with it beforehand). So just make sure that when you send a binary type you are ready to receive binary too. |
I actually am sending it as an object, with other data, too. |
Yes that not null termination as I thought, its UTF16 padding bytes. UTF16 is a 2 byte encoding and not the right thing here. |
So if you dont do the ab2str / str2ab stuff on both ends, it does not work? (Just putting string in on both ends, and reading strings back). Edit: Also debug the type and content of the data chunks there. Do you alter the sockets (on both ends) with some other flags? Like binary and such? |
If strings on both end dont work for you, you could see if binary works, but not with UTF16 arrays. Use TextEncoder to create well formed UTF-8 data on browser side like this: const te = new TextEncoder();
const byteChunk = te.encoder('string data');
...
socket.emit('data', byteChunk); This should send the string data as utf8 bytes, which you can feed directly to the pty: socketChannel.on('data', (data) => {
ptyProcess.write(data);
}); But ofc this implies that the host running the pty (and the slave app) has an UTF-8 locale set. Other encodings are currently not supported, again #2362 will fix this. |
This is good advice. I will experiment it w/ it later. |
So I tried w/ TextEncoder and noticed it was creating an object, which was not directly passible to pty. However, I used ArrayBuffer w/ a string length of the input str (not * 2), and that solved the issue w/ the null character. Again, as I said, I have no clue what I'm doing, but this works if using these functions on both sides, using str2ab on the emit, and ab2str on the receive. Passing directly to xterm.write and not xterm.writeUtf8.
|
@jzombie Your UTF-16 typed array hack works by accident, dont use that. Your new "converter" is a binary mode converter, it will strip the high byte of the UTF16 codepoint, thus not work either for unicode chars (works only for ASCII since UTF16 shares ASCII with most other 8bit encodings). I gave you the example how to do it with TextEncoder. It creates an encoder instance, the real data conversion happens with I suggest you to make a full stop here as going with your current approach will lead to errors later on. Put everything back to strings:
Next trace the chunk content at the server part and in browser and see where they differ. Basic idea: what was put in left should come out unaltered right and vice versa. Come back with this info, then I might be able to help you. |
Yes, Uint8Array is a single byte per position, thus can only store 0-255. Uint16Array is 16bit, thus 2 bytes at memory level.
No. UTF-16 is a concept, that represents unicode codepoints as 2 consecutive bytes in memory. The typed arrays are just views on memory with different access levels, a Uint8Array can access single bytes read as
As you can see, both typed arrays can read UTF16, but with Uint16Array the access is more convenient as a codepoint directly map to an index access. With Uint8Array you'd have to grab 2 positions and SHIFT + ADD them to get the actual codepoint back. I think you need some better understanding of data representation/byte level to get this done. |
At least in my implementation, this is what's happening. Notice, the client is emitting a Uint8Array w/ the TextEncoder.
On the server, I get this:
However, this is likely due to other aspects of my implementation, and others may not get the same results. |
Oh lol, that
|
Again, when I try just sending the data, verbatim (by sticking w/ strings), it doesn't work for me. This is likely due to my implementation. |
Can you grab the data and post here with all set to strings? |
Will do in a second, but please keep in mind this silly detail: I'm containing everything in an object before transmitting, to more or less be able to run multiple xterms over the same connection, without changing the event names in all of the instances. Each object is constructed like { evtName, evtData }, and emitted with a common event identifier unique per xterm instance. That aspect works, though the implementation is a hack. |
Ah well embedding it into another object might cause the JSON conversion. To distingish terminals you could register the listener under separate urls that contain some identifier instead. This way you can keep the events clean from other data. |
It "seems" to work when just passing strings via my implementation, bypassing the ab2str, TextEncoder, etc. However, I get an: Uncaught RangeError: Maximum call stack size exceeded I believe that's a library internal to Socket.io if I'm not mistaken. I am not using it personally. |
Line 50 is 'if (Object.prototype.hasOwnProperty.call(obj, key) && hasBinary(obj[key])) {'.... This is utilized internally somewhere, but not directly by me:
|
I'm running an entire API on that single socket connection, and wanted to experiment with running 2-way communications on top of an existing connection, as a "tunnel" (not really, but sorta). Just more or less as an experiment more than anything practical. With the str2ab conversion, I had no problems doing that. |
Maybe dont send the Uint8Array directly, but its arraybuffer? That would be |
Ah, good point. Could be. |
So yes, that seems to work, even in this object hack. |
But watch out, in nodejs the arraybuffer is sometimes not aligned with the Buffer on top, thus on the pty side its better to stick with Buffer itself. |
Thanks for all of your help in making my code more efficient. |
Sounds like its working now? |
It's working for sure. |
Sweet, good luck with your project 😸 |
Thanks! |
node-pty emits an object when hitting the spacebar, I just noticed, as opposed to a string for alphanumeric characters. Going to go back to working w/ the ab2str methods for now and put this on the backburner so I can address some other issues. I literally was able to work with multiple terminal shells, in multiple interactive programs, at the same time w/ the previous approach. Still this has been very helpful. |
Seriously - your str2ab way is totally broken, dont use that. Just because you have progressed with it further doesnt mean its right thing to do it. For a deeper understanding of JS strings this might be helpful: https://mathiasbynens.be/notes/javascript-encoding |
Thanks, again @jerch. I'm referencing this as an issue so that I can come back to it. |
this is what I do..
|
I don't want to use websocket directly
The text was updated successfully, but these errors were encountered: