diff options
| -rw-r--r-- | dev-requirements.txt | 3 | ||||
| -rw-r--r-- | paramiko/__init__.py | 7 | ||||
| -rw-r--r-- | paramiko/channel.py | 38 | ||||
| -rw-r--r-- | paramiko/client.py | 2 | ||||
| -rw-r--r-- | sites/www/changelog.rst | 13 | ||||
| -rw-r--r-- | tests/test_channelfile.py | 31 | ||||
| -rw-r--r-- | tests/test_client.py | 2 |
7 files changed, 84 insertions, 12 deletions
diff --git a/dev-requirements.txt b/dev-requirements.txt index 9a7be339..f4f84748 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,8 +1,7 @@ # Invocations for common project tasks invoke>=1.0,<2.0 invocations>=1.2.0,<2.0 -# NOTE: pytest-relaxed currently only works with pytest >=3, <3.3 -pytest==4.6.3 +pytest==4.4.2 pytest-relaxed==1.1.5 # pytest-xdist for test dir watching and the inv guard task pytest-xdist==1.28.0 diff --git a/paramiko/__init__.py b/paramiko/__init__.py index 6afc7a61..8ac52579 100644 --- a/paramiko/__init__.py +++ b/paramiko/__init__.py @@ -29,7 +29,12 @@ from paramiko.client import ( ) 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 f2aaf26a..72f65012 100644 --- a/paramiko/channel.py +++ b/paramiko/channel.py @@ -889,12 +889,30 @@ class Channel(ClosingContextManager): client, it only makes sense to open this file for reading. For a server, it only makes sense to open this file for writing. - :return: `.ChannelFile` object which can be used for Python file I/O. + :returns: + `.ChannelStderrFile` object which can be used for Python file I/O. .. versionadded:: 1.1 """ 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 @@ -1348,9 +1366,27 @@ class ChannelFile(BufferedFile): class ChannelStderrFile(ChannelFile): + """ + A file-like wrapper around `.Channel` stderr. + + See `Channel.makefile_stderr` for details. + """ + def _read(self, size): return self.channel.recv_stderr(size) def _write(self, data): self.channel.sendall_stderr(data) return len(data) + + +class ChannelStdinFile(ChannelFile): + """ + A file-like wrapper around `.Channel` stdin. + + See `Channel.makefile_stdin` for details. + """ + + def close(self): + super(ChannelStdinFile, self).close() + self.channel.shutdown_write() diff --git a/paramiko/client.py b/paramiko/client.py index 6bf479d4..a47efbfe 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -503,7 +503,7 @@ class SSHClient(ClosingContextManager): 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 41c50ce5..8838f5aa 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,19 @@ Changelog ========= +- :bug:`322 major` `SSHClient.exec_command + <paramiko.client.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 lead to issues (such as hangs) 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. + + Thanks to Brandon Rhodes for the report & steps to reproduce. - :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 ffcbea3f..4448fdfb 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): - ChannelFile(Channel(None)) + 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): - ChannelFile(Channel(None), mode="w", bufsize=25) + 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() @@ -39,3 +43,18 @@ class TestChannelStderrFile(object): cf = ChannelStderrFile(chan, mode="w") 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 26de2d37..c46c383b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -213,7 +213,7 @@ class ClientTest(unittest.TestCase): # 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) |