diff options
| -rw-r--r-- | paramiko/channel.py | 174 | ||||
| -rw-r--r-- | paramiko/packet.py | 10 | ||||
| -rw-r--r-- | paramiko/transport.py | 2 | ||||
| -rw-r--r-- | tests/test_client.py | 2 | ||||
| -rw-r--r-- | tests/test_packetizer.py | 46 |
5 files changed, 141 insertions, 93 deletions
diff --git a/paramiko/channel.py b/paramiko/channel.py index dadfe184..27202a7e 100644 --- a/paramiko/channel.py +++ b/paramiko/channel.py @@ -117,13 +117,13 @@ class Channel (object): self.combine_stderr = False self.exit_status = -1 self.origin_addr = None - + def __del__(self): try: self.close() except: pass - + def __repr__(self): """ Return a string representation of this object, for debugging. @@ -158,7 +158,7 @@ class Channel (object): :param int height: height (in characters) of the terminal screen :param int width_pixels: width (in pixels) of the terminal screen :param int height_pixels: height (in pixels) of the terminal screen - + :raises SSHException: if the request was rejected or the channel was closed """ @@ -183,14 +183,14 @@ class Channel (object): Request an interactive shell session on this channel. If the server allows it, the channel will then be directly connected to the stdin, stdout, and stderr of the shell. - + Normally you would call `get_pty` before this, in which case the shell will operate through the pty, and the channel will be connected to the stdin and stdout of the pty. - + When the shell exits, the channel will be closed and can't be reused. You must open a new channel if you wish to open another shell. - + :raises SSHException: if the request was rejected or the channel was closed """ @@ -209,7 +209,7 @@ class Channel (object): Execute a command on the server. If the server allows it, the channel will then be directly connected to the stdin, stdout, and stderr of the command being executed. - + When the command finishes executing, the channel will be closed and can't be reused. You must open a new channel if you wish to execute another command. @@ -235,7 +235,7 @@ class Channel (object): Request a subsystem on the server (for example, ``sftp``). If the server allows it, the channel will then be directly connected to the requested subsystem. - + When the subsystem finishes, the channel will be closed and can't be reused. @@ -285,14 +285,14 @@ class Channel (object): status. You may use this to poll the process status if you don't want to block in `recv_exit_status`. Note that the server may not return an exit status in some cases (like bad servers). - + :return: ``True`` if `recv_exit_status` will return immediately, else ``False``. .. versionadded:: 1.7.3 """ return self.closed or self.status_event.isSet() - + def recv_exit_status(self): """ Return the exit status from the process on the server. This is @@ -300,9 +300,9 @@ class Channel (object): If the command hasn't finished yet, this method will wait until it does, or until the channel is closed. If no exit status is provided by the server, -1 is returned. - + :return: the exit code (as an `int`) of the process on the server. - + .. versionadded:: 1.2 """ self.status_event.wait() @@ -315,9 +315,9 @@ class Channel (object): really only makes sense in server mode.) Many clients expect to get some sort of status code back from an executed command after it completes. - + :param int status: the exit code of the process - + .. versionadded:: 1.2 """ # in many cases, the channel will not still be open here. @@ -337,13 +337,13 @@ class Channel (object): Request an x11 session on this channel. If the server allows it, further x11 requests can be made from the server to the client, when an x11 application is run in a shell session. - + From RFC4254:: It is RECOMMENDED that the 'x11 authentication cookie' that is sent be a fake, random cookie, and that the cookie be checked and replaced by the real cookie when a connection request is received. - + If you omit the auth_cookie, a new secure random 128-bit value will be generated, used, and returned. You will need to use this value to verify incoming x11 requests and replace them with the actual local @@ -353,9 +353,8 @@ class Channel (object): whenever a new x11 connection arrives. The default handler queues up incoming x11 connections, which may be retrieved using `.Transport.accept`. The handler's calling signature is:: - + handler(channel: Channel, (address: str, port: int)) - :param int screen_number: the x11 screen number (0, 10, etc.) :param str auth_protocol: the name of the X11 authentication method used; if none is given, @@ -438,33 +437,33 @@ class Channel (object): def get_id(self): """ Return the `int` ID # for this channel. - + The channel ID is unique across a `.Transport` and usually a small number. It's also the number passed to `.ServerInterface.check_channel_request` when determining whether to accept a channel request in server mode. """ return self.chanid - + def set_combine_stderr(self, combine): """ Set whether stderr should be combined into stdout on this channel. The default is ``False``, but in some cases it may be convenient to have both streams combined. - + If this is ``False``, and `exec_command` is called (or ``invoke_shell`` with no pty), output to stderr will not show up through the `recv` and `recv_ready` calls. You will have to use `recv_stderr` and `recv_stderr_ready` to get stderr output. - + If this is ``True``, data will never show up via `recv_stderr` or `recv_stderr_ready`. - + :param bool combine: ``True`` if stderr output should be combined into stdout on this channel. :return: the previous setting (a `bool`). - + .. versionadded:: 1.1 """ data = bytes() @@ -573,7 +572,7 @@ class Channel (object): Returns true if data is buffered and ready to be read from this channel. A ``False`` result does not mean that the channel has closed; it means you may need to wait before more data arrives. - + :return: ``True`` if a `recv` call on this channel would immediately return at least one byte; ``False`` otherwise. @@ -589,7 +588,7 @@ class Channel (object): :param int nbytes: maximum number of bytes to read. :return: received data, as a `str` - + :raises socket.timeout: if no data is ready before the timeout set by `settimeout`. """ @@ -615,11 +614,11 @@ class Channel (object): channel's stderr stream. Only channels using `exec_command` or `invoke_shell` without a pty will ever have data on the stderr stream. - + :return: ``True`` if a `recv_stderr` call on this channel would immediately return at least one byte; ``False`` otherwise. - + .. versionadded:: 1.1 """ return self.in_stderr_buffer.read_ready() @@ -635,17 +634,17 @@ class Channel (object): :param int nbytes: maximum number of bytes to read. :return: received data as a `str` - + :raises socket.timeout: if no data is ready before the timeout set by `settimeout`. - + .. versionadded:: 1.1 """ try: out = self.in_stderr_buffer.read(nbytes, self.timeout) except PipeTimeout: raise socket.timeout() - + ack = self._check_add_window(len(out)) # no need to hold the channel lock when sending this if ack > 0: @@ -661,11 +660,11 @@ class Channel (object): """ Returns true if data can be written to this channel without blocking. This means the channel is either closed (so any write attempt would - return immediately) or there is at least one byte of space in the + return immediately) or there is at least one byte of space in the outbound buffer. If there is at least one byte of space in the outbound buffer, a `send` call will succeed immediately and return the number of bytes actually written. - + :return: ``True`` if a `send` call on this channel would immediately succeed or fail @@ -677,7 +676,7 @@ class Channel (object): return self.out_window_size > 0 finally: self.lock.release() - + def send(self, s): """ Send data to the channel. Returns the number of bytes sent, or 0 if @@ -692,23 +691,11 @@ class Channel (object): :raises socket.timeout: if no data could be sent before the timeout set by `settimeout`. """ - size = len(s) - self.lock.acquire() - try: - size = self._wait_for_send_window(size) - if size == 0: - # eof or similar - return 0 - m = Message() - m.add_byte(cMSG_CHANNEL_DATA) - m.add_int(self.remote_chanid) - m.add_string(s[:size]) - finally: - self.lock.release() - # Note: We release self.lock before calling _send_user_message. - # Otherwise, we can deadlock during re-keying. - self.transport._send_user_message(m) - return size + + m = Message() + m.add_byte(cMSG_CHANNEL_DATA) + m.add_int(self.remote_chanid) + return self._send(s, m) def send_stderr(self, s): """ @@ -718,33 +705,22 @@ class Channel (object): stream is closed. Applications are responsible for checking that all data has been sent: if only some of the data was transmitted, the application needs to attempt delivery of the remaining data. - + :param str s: data to send. :return: number of bytes actually sent, as an `int`. - + :raises socket.timeout: if no data could be sent before the timeout set by `settimeout`. - + .. versionadded:: 1.1 """ - size = len(s) - self.lock.acquire() - try: - size = self._wait_for_send_window(size) - if size == 0: - # eof or similar - return 0 - m = Message() - m.add_byte(cMSG_CHANNEL_EXTENDED_DATA) - m.add_int(self.remote_chanid) - m.add_int(1) - m.add_string(s[:size]) - finally: - self.lock.release() - # Note: We release self.lock before calling _send_user_message. - # Otherwise, we can deadlock during re-keying. - self.transport._send_user_message(m) - return size + + m = Message() + m.add_byte(cMSG_CHANNEL_EXTENDED_DATA) + m.add_int(self.remote_chanid) + m.add_int(1) + return self._send(s, m) + def sendall(self, s): """ @@ -765,9 +741,6 @@ class Channel (object): This is irritating, but identically follows Python's API. """ while s: - if self.closed: - # this doesn't seem useful, but it is the documented behavior of Socket - raise socket.error('Socket is closed') sent = self.send(s) s = s[sent:] return None @@ -778,9 +751,9 @@ class Channel (object): results. Unlike `send_stderr`, this method continues to send data from the given string until all data has been sent or an error occurs. Nothing is returned. - + :param str s: data to send to the client as "stderr" output. - + :raises socket.timeout: if sending stalled for longer than the timeout set by `settimeout`. :raises socket.error: @@ -789,8 +762,6 @@ class Channel (object): .. versionadded:: 1.1 """ while s: - if self.closed: - raise socket.error('Socket is closed') sent = self.send_stderr(s) s = s[sent:] return None @@ -810,18 +781,18 @@ class Channel (object): Return a file-like object associated with this channel's stderr stream. Only channels using `exec_command` or `invoke_shell` without a pty will ever have data on the stderr 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 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. .. versionadded:: 1.1 """ return ChannelStderrFile(*([self] + list(params))) - + def fileno(self): """ Returns an OS-level file descriptor which can be used for polling, but @@ -835,7 +806,7 @@ class Channel (object): open at the same time.) :return: an OS-level file descriptor (`int`) - + .. warning:: This method causes channel reads to be slightly less efficient. """ @@ -874,7 +845,7 @@ class Channel (object): self.lock.release() if m is not None: self.transport._send_user_message(m) - + def shutdown_read(self): """ Shutdown the receiving side of this socket, closing the stream in @@ -882,11 +853,11 @@ class Channel (object): channel will fail instantly. This is a convenience method, equivalent to ``shutdown(0)``, for people who don't make it a habit to memorize unix constants from the 1970s. - + .. versionadded:: 1.2 """ self.shutdown(0) - + def shutdown_write(self): """ Shutdown the sending side of this socket, closing the stream in @@ -894,7 +865,7 @@ class Channel (object): channel will fail instantly. This is a convenience method, equivalent to ``shutdown(1)``, for people who don't make it a habit to memorize unix constants from the 1970s. - + .. versionadded:: 1.2 """ self.shutdown(1) @@ -912,14 +883,14 @@ class Channel (object): self.in_window_threshold = window_size // 10 self.in_window_sofar = 0 self._log(DEBUG, 'Max packet in: %d bytes' % max_packet_size) - + def _set_remote_channel(self, chanid, window_size, max_packet_size): self.remote_chanid = chanid self.out_window_size = window_size self.out_max_packet_size = max(max_packet_size, MIN_PACKET_SIZE) self.active = 1 self._log(DEBUG, 'Max packet out: %d bytes' % max_packet_size) - + def _request_success(self, m): self._log(DEBUG, 'Sesch channel %d request ok' % self.chanid) self.event_ready = True @@ -954,7 +925,7 @@ class Channel (object): self._feed(s) else: self.in_stderr_buffer.feed(s) - + def _window_adjust(self, m): nbytes = m.get_int() self.lock.acquire() @@ -1077,6 +1048,25 @@ class Channel (object): ### internals... + def _send(self, s, m): + size = len(s) + self.lock.acquire() + try: + if self.closed: + # this doesn't seem useful, but it is the documented behavior of Socket + raise socket.error('Socket is closed') + size = self._wait_for_send_window(size) + if size == 0: + # eof or similar + return 0 + m.add_string(s[:size]) + finally: + self.lock.release() + # Note: We release self.lock before calling _send_user_message. + # Otherwise, we can deadlock during re-keying. + self.transport._send_user_message(m) + return size + def _log(self, level, msg, *args): self.logger.log(level, "[chan " + self._name + "] " + msg, *args) @@ -1196,7 +1186,7 @@ class Channel (object): if self.ultra_debug: self._log(DEBUG, 'window down to %d' % self.out_window_size) return size - + class ChannelFile (BufferedFile): """ @@ -1236,7 +1226,7 @@ class ChannelStderrFile (ChannelFile): def _read(self, size): return self.channel.recv_stderr(size) - + def _write(self, data): self.channel.sendall_stderr(data) return len(data) diff --git a/paramiko/packet.py b/paramiko/packet.py index e97d92f0..f516ff9b 100644 --- a/paramiko/packet.py +++ b/paramiko/packet.py @@ -231,6 +231,7 @@ class Packetizer (object): def write_all(self, out): self.__keepalive_last = time.time() + iteration_with_zero_as_return_value = 0 while len(out) > 0: retry_write = False try: @@ -254,6 +255,15 @@ class Packetizer (object): n = 0 if self.__closed: n = -1 + else: + if n == 0 and iteration_with_zero_as_return_value > 10: + # We shouldn't retry the write, but we didn't + # manage to send anything over the socket. This might be an + # indication that we have lost contact with the remote side, + # but are yet to receive an EOFError or other socket errors. + # Let's give it some iteration to try and catch up. + n = -1 + iteration_with_zero_as_return_value += 1 if n < 0: raise EOFError() if n == len(out): diff --git a/paramiko/transport.py b/paramiko/transport.py index d3990e54..fff11a1d 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -1257,7 +1257,7 @@ class Transport (threading.Thread): def stop_thread(self): self.active = False self.packetizer.close() - while self.isAlive(): + while self.is_alive() and (self is not threading.current_thread()): self.join(10) ### internals... diff --git a/tests/test_client.py b/tests/test_client.py index 7c094628..33dd9f23 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -262,6 +262,8 @@ class SSHClientTest (unittest.TestCase): """ # Unclear why this is borked on Py3, but it is, and does not seem worth # pursuing at the moment. + # XXX: It's the release of the references to e.g packetizer that fails + # in py3... if not PY2: return threading.Thread(target=self._run).start() diff --git a/tests/test_packetizer.py b/tests/test_packetizer.py index a8c0f973..8faec03c 100644 --- a/tests/test_packetizer.py +++ b/tests/test_packetizer.py @@ -74,3 +74,49 @@ class PacketizerTest (unittest.TestCase): self.assertEqual(100, m.get_int()) self.assertEqual(1, m.get_int()) self.assertEqual(900, m.get_int()) + + def test_3_closed(self): + rsock = LoopSocket() + wsock = LoopSocket() + rsock.link(wsock) + p = Packetizer(wsock) + p.set_log(util.get_logger('paramiko.transport')) + p.set_hexdump(True) + cipher = AES.new(zero_byte * 16, AES.MODE_CBC, x55 * 16) + p.set_outbound_cipher(cipher, 16, sha1, 12, x1f * 20) + + # message has to be at least 16 bytes long, so we'll have at least one + # block of data encrypted that contains zero random padding bytes + m = Message() + m.add_byte(byte_chr(100)) + m.add_int(100) + m.add_int(1) + m.add_int(900) + wsock.send = lambda x: 0 + from functools import wraps + import errno + import os + import signal + + class TimeoutError(Exception): + pass + + def timeout(seconds=1, error_message=os.strerror(errno.ETIME)): + def decorator(func): + def _handle_timeout(signum, frame): + raise TimeoutError(error_message) + + def wrapper(*args, **kwargs): + signal.signal(signal.SIGALRM, _handle_timeout) + signal.alarm(seconds) + try: + result = func(*args, **kwargs) + finally: + signal.alarm(0) + return result + + return wraps(func)(wrapper) + + return decorator + send = timeout()(p.send_message) + self.assertRaises(EOFError, send, m) |