prompt-toolkit / ptpython

A better Python REPL
BSD 3-Clause "New" or "Revised" License
5.23k stars 281 forks source link

PythonRepl over AsyncSSH mixes inputs, corrupts connection #561

Open Menyadar opened 11 months ago

Menyadar commented 11 months ago

I'm encountering strange issue with PythonRepl over ssh. Example ptpython/examples/asyncio-ssh-python-embed.py using ptpython.contrib.asyncssh_repl.py bundled in sources is obsolete (at least from 3.0.28), but it looks like prompt_toolkit.contrib.ssh.server is now the proper way to go (it even looks it was developed from the example).

Using all these as a base for my example (see below for gist), i was able to login over ssh to PythonRepl and everything looks fine, however only until there is another connection. Second connection then hijacks input from first. Writing to first client shows nothing, but is visible in second. As soon as any input is entered, second session fails. first client:

$ ssh 127.0.0.1 -p 8222
>>> dir()
['__builtins__', 'get_ptpython', 'print']

>>> dir()
['_', '_1', '__builtins__', 'get_ptpython', 'print']

second client just after connect:

$ ssh 127.0.0.1 -p 8222
>>> 

Now enter anything to first client, nothing will happen there, but will appear in the second. Pressing enter ends up with corrupted connection, disconnecting second client and leaving first client in unusable state.

# this is the second client
>>> dir()

Bad packet length 749899493.
ssh_dispatch_run_fatal: Connection to 127.0.0.1 port 8222: Connection corrupted

Server output:

INFO:asyncssh:Creating SSH listener on port 8222
INFO:asyncssh:[conn=0] Accepted SSH client connection
INFO:asyncssh:[conn=0]   Local address: 127.0.0.1, port 8222
INFO:asyncssh:[conn=0]   Peer address: 127.0.0.1, port 35100
INFO:asyncssh:[conn=0] Beginning auth for user user
INFO:asyncssh:[conn=0] Auth for user user succeeded
INFO:asyncssh:[conn=0, chan=0] New SSH session requested
INFO:asyncssh:[conn=0, chan=0]   PTY created
INFO:asyncssh:[conn=0, chan=0]   Line editor enabled
INFO:asyncssh:[conn=0, chan=0]   Interactive shell requested
INFO:asyncssh:[conn=0, chan=0] Disabling line editor
INFO:asyncssh:[conn=1] Accepted SSH client connection
INFO:asyncssh:[conn=1]   Local address: 127.0.0.1, port 8222
INFO:asyncssh:[conn=1]   Peer address: 127.0.0.1, port 52500
INFO:asyncssh:[conn=1] Beginning auth for user user
INFO:asyncssh:[conn=1] Auth for user user succeeded
INFO:asyncssh:[conn=1, chan=0] New SSH session requested
INFO:asyncssh:[conn=1, chan=0]   PTY created
INFO:asyncssh:[conn=1, chan=0]   Line editor enabled
INFO:asyncssh:[conn=1, chan=0]   Interactive shell requested
INFO:asyncssh:[conn=1, chan=0] Disabling line editor
INFO:asyncssh:[conn=1, chan=0] Closing channel due to connection close
INFO:asyncssh:[conn=1, chan=0] Channel closed: Connection lost

Raising logging level to DEBUG doesn't show anything interesting either.

Stepping through the code with debugger prevents this corruption, so there is probably even some timing issue, but input is still mixed. When experimenting with writing to each client i've noticed other weird behavior - writing to second client and stepping through execution makes ptpython bottom menu disappear in the first client for a brief moment (mostly when a corouting is being scheduled). Possibly not only inputs, but even outputs are somehow connected.

I've prepared minimal example as a gist: asyncio-ssh-python-embed.py. This expects server key in ssh-keys/ssh_host_rsa_key. It is stripped down to bare minimum, not even passing custom globals (each command then runs in empty dict as globals and locals, triggering error on exit, but it is unrelated to this case). Most probably i'm just omitting some important setting, or not passing correct values somewhere, but examples are scarce for this usecase.

jonathanslenders commented 11 months ago

Thanks for reporting! I don't have a solution right now, but I can reproduce this. It's definitely a bug.

jonathanslenders commented 11 months ago

I should also add that even if we manage to make it work again (I think this used to work), the SSH/telnet functionality of ptpython has its limitations. The main problem being that there's only one sys.stdout or sys.stdin. It's hard to make the REPL think that for one connection, stdout should redirect over there, but for another connection, stdout should go elsewhere. We can patch sys.stdout or replace print by a different function, with thread locals or context vars, but it doesn't work for every case.

Menyadar commented 11 months ago

I think mostly just sys.stdout is really needed, as sys.stdin isn't used as much in interactive sessions. You can look how twisted handles this in conch.manhole. It's rather thin layer over code.InteractiveInterpreter class. Haven't checked too deep how they patch stdout to make print work (but it does, without the need to replace it with custom function). When connected to it, this is how it looks:

>>> dir()
['__builtins__', '__doc__', '__name__']
>>> import sys
>>> sys.stdin
<_io.TextIOWrapper name='<stdin>' mode='r' encoding='utf-8'>
>>> sys.stdout
<twisted.conch.manhole.FileWrapper object at 0x7fcb3e032440>
>>> sys.stdout.write('test')
test
>>> print('test')
test

I can log to this interpreter from different ssh / telnet sessions and it just works without hijacking inputs. Frankly, when i noticed that ssh embed demo, i thought it'd be drop-in replacement, with better REPL experience on top (twisted manhole has some support for deferred / future calls out of the box + basic coloring, but that's it). I've spent some time trying to debug that input mixup expecting some easy to spot place, but came empty-handed.