diff --git a/paramiko/__init__.py b/paramiko/__init__.py index 6afc7a610..2b9d88956 100644 --- a/paramiko/__init__.py +++ b/paramiko/__init__.py @@ -29,7 +29,7 @@ ) from paramiko.auth_handler import AuthHandler from paramiko.ssh_gss import GSSAuth, GSS_AUTH_AVAILABLE, GSS_EXCEPTIONS -from paramiko.channel import Channel, ChannelFile, ChannelStderrFile +from paramiko.channel import Channel, ChannelFile, ChannelStderrFile, ChannelStdinFile from paramiko.ssh_exception import ( SSHException, PasswordRequiredException, diff --git a/paramiko/channel.py b/paramiko/channel.py index 2687e6fcc..d7938b198 100644 --- a/paramiko/channel.py +++ b/paramiko/channel.py @@ -896,6 +896,23 @@ def makefile_stderr(self, *params): """ return ChannelStderrFile(*([self] + list(params))) + def makefile_stdin(self, *params): + """ + Return a file-like object associated with this channel's stdin + stream. + + The optional ``mode`` and ``bufsize`` arguments are interpreted the + same way as by the built-in ``file()`` function in Python. For a + client, it only makes sense to open this file for writing. For a + server, it only makes sense to open this file for reading. + + :returns: + `.ChannelStdinFile` object which can be used for Python file I/O. + + .. versionadded:: 2.6 + """ + return ChannelStdinFile(*([self] + list(params))) + def fileno(self): """ Returns an OS-level file descriptor which can be used for polling, but @@ -1355,3 +1372,9 @@ def _read(self, size): def _write(self, data): self.channel.sendall_stderr(data) return len(data) + + +class ChannelStdinFile(ChannelFile): + def close(self): + super(ChannelFile, self).close() + self.channel.shutdown_write() diff --git a/paramiko/client.py b/paramiko/client.py index 6bf479d46..a47efbfec 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -503,7 +503,7 @@ def exec_command( if environment: chan.update_environment(environment) chan.exec_command(command) - stdin = chan.makefile("wb", bufsize) + stdin = chan.makefile_stdin("wb", bufsize) stdout = chan.makefile("r", bufsize) stderr = chan.makefile_stderr("r", bufsize) return stdin, stdout, stderr diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 41c50ce5a..abd73b06f 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,17 @@ Changelog ========= +- :bug:`322 major` `SSHClient.exec_command + ` previously returned a naive + `~paramiko.channel.ChannelFile` object for its ``stdin`` value; such objects + don't know to properly shut down the remote end's stdin when they + ``.close()``. This leads to issues when running remote commands that read + from stdin. + + A new subclass, `~paramiko.channel.ChannelStdinFile`, has been created which + closes remote stdin when it itself is closed. + `~paramiko.client.SSHClient.exec_command` has been updated to use that class + for its ``stdin`` return value. - :release:`2.5.0 <2019-06-09>` - :feature:`1233` (also :issue:`1229`, :issue:`1332`) Add support for encrypt-then-MAC (ETM) schemes (``hmac-sha2-256-etm@openssh.com``, diff --git a/tests/test_channelfile.py b/tests/test_channelfile.py index 6faf8bf8d..707a49c0a 100644 --- a/tests/test_channelfile.py +++ b/tests/test_channelfile.py @@ -1,32 +1,36 @@ from mock import patch, MagicMock -from paramiko import Channel, ChannelFile, ChannelStderrFile +from paramiko import Channel, ChannelFile, ChannelStderrFile, ChannelStdinFile -class TestChannelFile(object): +class ChannelFileBase(object): @patch("paramiko.channel.ChannelFile._set_mode") def test_defaults_to_unbuffered_reading(self, setmode): - cf = ChannelFile(Channel(None)) + cf = self.klass(Channel(None)) setmode.assert_called_once_with("r", -1) @patch("paramiko.channel.ChannelFile._set_mode") def test_can_override_mode_and_bufsize(self, setmode): - cf = ChannelFile(Channel(None), mode="w", bufsize=25) + cf = self.klass(Channel(None), mode="w", bufsize=25) setmode.assert_called_once_with("w", 25) def test_read_recvs_from_channel(self): chan = MagicMock() - cf = ChannelFile(chan) + cf = self.klass(chan) cf.read(100) chan.recv.assert_called_once_with(100) def test_write_calls_channel_sendall(self): chan = MagicMock() - cf = ChannelFile(chan, mode="w") + cf = self.klass(chan, mode="w") cf.write("ohai") chan.sendall.assert_called_once_with(b"ohai") +class TestChannelFile(ChannelFileBase): + klass = ChannelFile + + class TestChannelStderrFile(object): def test_read_calls_channel_recv_stderr(self): chan = MagicMock() @@ -40,3 +44,17 @@ def test_write_calls_channel_sendall(self): cf.write("ohai") chan.sendall_stderr.assert_called_once_with(b"ohai") + +class TestChannelStdinFile(ChannelFileBase): + klass = ChannelStdinFile + + def test_close_calls_channel_shutdown_write(self): + chan = MagicMock() + cf = ChannelStdinFile(chan, mode="wb") + cf.flush = MagicMock() + cf.close() + # Sanity check that we still call BufferedFile.close() + cf.flush.assert_called_once_with() + assert cf._closed is True + # Actual point of test + chan.shutdown_write.assert_called_once_with() diff --git a/tests/test_client.py b/tests/test_client.py index 26de2d37b..c46c383b2 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -213,7 +213,7 @@ def _test_connection(self, **kwargs): # Nobody else tests the API of exec_command so let's do it here for # now. :weary: - assert isinstance(stdin, paramiko.ChannelFile) + assert isinstance(stdin, paramiko.ChannelStdinFile) assert isinstance(stdout, paramiko.ChannelFile) assert isinstance(stderr, paramiko.ChannelStderrFile)