From 6685603b9cb9d72609d3bfda5d1a7a9392fe153f Mon Sep 17 00:00:00 2001 From: Paul Kapp Date: Sun, 7 Feb 2016 18:10:26 -0500 Subject: ProxyCommand fixes for Python3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simple fix of using unbuffered subprocess.PIPE uncovered a trickier bug where the lack of timeout on initial readline call would pull data from the first real packet, throwing off subsequent reads. Doesn’t seem necessary to preserve a read buffer across recv() calls, especially if it isn’t implemented correctly. --- paramiko/proxy.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/paramiko/proxy.py b/paramiko/proxy.py index ca602c4c..d3ae436f 100644 --- a/paramiko/proxy.py +++ b/paramiko/proxy.py @@ -38,7 +38,7 @@ class ProxyCommand(ClosingContextManager): `.Transport` and `.Packetizer` classes. Using this class instead of a regular socket makes it possible to talk with a Popen'd command that will proxy traffic between the client and a server hosted in another machine. - + Instances of this class may be used as context managers. """ def __init__(self, command_line): @@ -50,9 +50,9 @@ class ProxyCommand(ClosingContextManager): the command that should be executed and used as the proxy. """ self.cmd = shlsplit(command_line) - self.process = Popen(self.cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE) + self.process = Popen(self.cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE, + bufsize=0) self.timeout = None - self.buffer = [] def send(self, content): """ @@ -77,11 +77,12 @@ class ProxyCommand(ClosingContextManager): :param int size: how many chars should be read - :return: the length of the read content, as an `int` + :return: the string of bytes read, which may be shorter than requested """ try: + buffer = b'' start = time.time() - while len(self.buffer) < size: + while len(buffer) < size: select_timeout = None if self.timeout is not None: elapsed = (time.time() - start) @@ -92,16 +93,13 @@ class ProxyCommand(ClosingContextManager): r, w, x = select( [self.process.stdout], [], [], select_timeout) if r and r[0] == self.process.stdout: - b = os.read( - self.process.stdout.fileno(), size - len(self.buffer)) - # Store in class-level buffer for persistence across - # timeouts; this makes us act more like a real socket - # (where timeouts don't actually drop data.) - self.buffer.extend(b) - result = ''.join(self.buffer) - self.buffer = [] - return result + buffer += os.read( + self.process.stdout.fileno(), size - len(buffer)) + return buffer except socket.timeout: + if buffer: + # Don't raise socket.timeout, return partial result instead + return buffer raise # socket.timeout is a subclass of IOError except IOError as e: raise ProxyCommandFailure(' '.join(self.cmd), e.strerror) -- cgit v1.2.3 From e0d6687c9305a8a2cd95093dec638f6fb93a25cb Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 25 Jul 2016 21:03:33 -0700 Subject: Changelog re #673, fixes #673, closes #681 --- sites/www/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 6004a6e7..76e81721 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,10 @@ Changelog ========= +* :bug:`673` (via :issue:`681`) Fix protocol banner read errors + (``SSHException``) which would occasionally pop up when using + ``ProxyCommand`` gatewaying. Thanks to ``@Depado`` for the initial report and + Paul Kapp for the fix. * :bug:`774 (1.16+)` Add a ``_closed`` private attribute to `~paramiko.channel.Channel` objects so that they continue functioning when used as proxy sockets under Python 3 (e.g. as ``direct-tcpip`` gateways for -- cgit v1.2.3