carletes / mock-ssh-server

Python mock SSH server for testing purposes
MIT License
56 stars 32 forks source link

stdin of executed command is not closed when using the `ssh` binary as the client #24

Open wosc opened 3 years ago

wosc commented 3 years ago

Since I'm currently trying to write a test harness for some legacy ssh scripts with the basic idea cat long-input-file | ssh remotehost process.sh, I'm really happy that mockssh just recently gained stdin support in 0.9.1. Thank you!

However, it seems to me that paramiko's server.client().execute_command() handles stdin differently than openssh's ssh binary. This example test hangs indefinitely at while self.process.poll():

@pytest.fixture
def server(tmpdir):
    shutil.copy(pkg_resources.resource_filename(
        'mockssh', 'sample-user-key'), tmpdir / 'key')
    os.chmod(tmpdir / 'key', 0o600)  # ssh(1) requires strict permissions
    with mockssh.Server({'sample': pkg_resources.resource_filename(
            'mockssh', 'sample-user-key')}) as server:
        yield server

def test_stdin_using_ssh_client_binary(server, tmpdir):
    proc = subprocess.Popen(
        ['ssh', '-i', tmpdir / 'key', '-p', str(server.port), 
         '-o', 'StrictHostKeyChecking=no',
         'sample@localhost',  'cat'],
        stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    out, err = proc.communicate(b'myinput')
    out, err = out.decode('utf-8'), err.decode('utf-8')
    print(err)
    assert 'myinput' in out

I'm not at all familiar with all the internals at play here, so all I can offer is this somewhat kludgy idea -- which does make the above test case work as expected:

--- mockssh/streaming.py~   2021-10-25 14:03:25.000000000 +0200
+++ mockssh/streaming.py    2021-10-25 14:04:20.000000000 +0200
@@ -23,6 +23,23 @@
                 return

+class InputStream(Stream):
+
+    def __init__(self, fd, read, output):
+        super().__init__(fd, read, output.write, output.flush)
+        self.output = output
+        self.eof = False
+
+    def transfer(self):
+        if self.eof:
+            return b''
+        data = super().transfer()
+        # https://docs.paramiko.org/en/stable/api/channel.html#paramiko.channel.Channel.recv says
+        # "If a string of length zero is returned, the channel stream has closed."
+        if not data:
+            self.eof = True
+            self.output.close()
+        return data
+
+
 class StreamTransfer:
     BUFFER_SIZE = 1024

@@ -35,7 +52,7 @@
         ]

     def ssh_to_process(self, channel, process_stream):
-        return Stream(channel, lambda: channel.recv(self.BUFFER_SIZE), process_stream.write, process_stream.flush)
+        return InputStream(channel, lambda: channel.recv(self.BUFFER_SIZE), process_stream)

     @staticmethod
     def process_to_ssh(process_stream, write_func):
colons commented 2 years ago

i've been trying to use mockssh to test a codebase that sends arbitrary data over stdin over ssh, so i was also very excited to see the addition of stdin support, but got stuck with the hang described here too

i am very much liking this patch you've done