Merge branch 'master' into 471-int

1 files changed, 100 insertions, 72 deletions

diff --git a/paramiko/sftp_client.py b/paramiko/sftp_client.py
index 2979544f..cf7785cf 100644
--- a/paramiko/sftp_client.py
+++ b/paramiko/sftp_client.py
@@ -28,13 +28,17 @@ from paramiko import util
 from paramiko.channel import Channel
 from paramiko.message import Message
 from paramiko.common import INFO, DEBUG, o777
-from paramiko.py3compat import bytestring, b, u, long, string_types, bytes_types
-from paramiko.sftp import BaseSFTP, CMD_OPENDIR, CMD_HANDLE, SFTPError, CMD_READDIR, \
-    CMD_NAME, CMD_CLOSE, SFTP_FLAG_READ, SFTP_FLAG_WRITE, SFTP_FLAG_CREATE, \
-    SFTP_FLAG_TRUNC, SFTP_FLAG_APPEND, SFTP_FLAG_EXCL, CMD_OPEN, CMD_REMOVE, \
-    CMD_RENAME, CMD_MKDIR, CMD_RMDIR, CMD_STAT, CMD_ATTRS, CMD_LSTAT, \
-    CMD_SYMLINK, CMD_SETSTAT, CMD_READLINK, CMD_REALPATH, CMD_STATUS, CMD_EXTENDED, SFTP_OK, \
-    SFTP_EOF, SFTP_NO_SUCH_FILE, SFTP_PERMISSION_DENIED
+from paramiko.py3compat import (
+    bytestring, b, u, long, string_types, bytes_types,
+)
+from paramiko.sftp import (
+    BaseSFTP, CMD_OPENDIR, CMD_HANDLE, SFTPError, CMD_READDIR, CMD_NAME,
+    CMD_CLOSE, SFTP_FLAG_READ, SFTP_FLAG_WRITE, SFTP_FLAG_CREATE,
+    SFTP_FLAG_TRUNC, SFTP_FLAG_APPEND, SFTP_FLAG_EXCL, CMD_OPEN, CMD_REMOVE,
+    CMD_RENAME, CMD_MKDIR, CMD_RMDIR, CMD_STAT, CMD_ATTRS, CMD_LSTAT,
+    CMD_SYMLINK, CMD_SETSTAT, CMD_READLINK, CMD_REALPATH, CMD_STATUS,
+    CMD_EXTENDED, SFTP_OK, SFTP_EOF, SFTP_NO_SUCH_FILE, SFTP_PERMISSION_DENIED,
+)
 
 from paramiko.sftp_attr import SFTPAttributes
 from paramiko.ssh_exception import SSHException
@@ -56,6 +60,7 @@ def _to_unicode(s):
         except UnicodeError:
             return s
 
+
 b_slash = b'/'
 
 
@@ -78,8 +83,8 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
         :param .Channel sock: an open `.Channel` using the ``"sftp"`` subsystem
 
-        :raises SSHException: if there's an exception while negotiating
-            sftp
+        :raises:
+            `.SSHException` -- if there's an exception while negotiating sftp
         """
         BaseSFTP.__init__(self)
         self.sock = sock
@@ -93,13 +98,16 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         if type(sock) is Channel:
             # override default logger
             transport = self.sock.get_transport()
-            self.logger = util.get_logger(transport.get_log_channel() + '.sftp')
+            self.logger = util.get_logger(
+                transport.get_log_channel() + '.sftp')
             self.ultra_debug = transport.get_hexdump()
         try:
             server_version = self._send_version()
         except EOFError:
             raise SSHException('EOF during negotiation')
-        self._log(INFO, 'Opened sftp connection (server version %d)' % server_version)
+        self._log(
+            INFO,
+            'Opened sftp connection (server version %d)' % server_version)
 
     @classmethod
     def from_transport(cls, t, window_size=None, max_packet_size=None):
@@ -111,7 +119,8 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         OpenSSH and should work adequately for both files transfers and
         interactive sessions.
 
-        :param .Transport t: an open `.Transport` which is already authenticated
+        :param .Transport t: an open `.Transport` which is already
+            authenticated
         :param int window_size:
             optional window size for the `.SFTPClient` session.
         :param int max_packet_size:
@@ -136,9 +145,12 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
             for m in msg:
                 self._log(level, m, *args)
         else:
-            # escape '%' in msg (they could come from file or directory names) before logging
-            msg = msg.replace('%','%%')
-            super(SFTPClient, self)._log(level, "[chan %s] " + msg, *([self.sock.get_name()] + list(args)))
+            # escape '%' in msg (they could come from file or directory names)
+            # before logging
+            msg = msg.replace('%', '%%')
+            super(SFTPClient, self)._log(
+                level,
+                "[chan %s] " + msg, *([self.sock.get_name()] + list(args)))
 
     def close(self):
         """
@@ -160,7 +172,8 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
     def listdir(self, path='.'):
         """
-        Return a list containing the names of the entries in the given ``path``.
+        Return a list containing the names of the entries in the given
+        ``path``.
 
         The list is in arbitrary order.  It does not include the special
         entries ``'.'`` and ``'..'`` even if they are present in the folder.
@@ -223,7 +236,7 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         ``read_aheads``, an integer controlling how many
         ``SSH_FXP_READDIR`` requests are made to the server. The default of 50
         should suffice for most file listings as each request/response cycle
-        may contain multiple files (dependant on server implementation.)
+        may contain multiple files (dependent on server implementation.)
 
         .. versionadded:: 1.15
         """
@@ -308,7 +321,7 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         :param int bufsize: desired buffering (-1 = default buffer size)
         :return: an `.SFTPFile` object representing the open file
 
-        :raises IOError: if the file could not be opened.
+        :raises: ``IOError`` -- if the file could not be opened.
         """
         filename = self._adjust_cwd(filename)
         self._log(DEBUG, 'open(%r, %r)' % (filename, mode))
@@ -328,7 +341,9 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         if t != CMD_HANDLE:
             raise SFTPError('Expected handle')
         handle = msg.get_binary()
-        self._log(DEBUG, 'open(%r, %r) -> %s' % (filename, mode, hexlify(handle)))
+        self._log(
+            DEBUG,
+            'open(%r, %r) -> %s' % (filename, mode, hexlify(handle)))
         return SFTPFile(self, handle, mode, bufsize)
 
     # Python continues to vacillate about "open" vs "file"...
@@ -341,7 +356,7 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
         :param str path: path (absolute or relative) of the file to remove
 
-        :raises IOError: if the path refers to a folder (directory)
+        :raises: ``IOError`` -- if the path refers to a folder (directory)
         """
         path = self._adjust_cwd(path)
         self._log(DEBUG, 'remove(%r)' % path)
@@ -356,7 +371,8 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         :param str oldpath: existing name of the file or folder
         :param str newpath: new name for the file or folder, must not exist already
 
-        :raises IOError: if ``newpath`` is a folder, or something else goes
+        :raises:
+            ``IOError`` -- if ``newpath`` is a folder, or something else goes
             wrong
         """
         oldpath = self._adjust_cwd(oldpath)
@@ -497,12 +513,12 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
     def utime(self, path, times):
         """
-        Set the access and modified times of the file specified by ``path``.  If
-        ``times`` is ``None``, then the file's access and modified times are set
-        to the current time.  Otherwise, ``times`` must be a 2-tuple of numbers,
-        of the form ``(atime, mtime)``, which is used to set the access and
-        modified times, respectively.  This bizarre API is mimicked from Python
-        for the sake of consistency -- I apologize.
+        Set the access and modified times of the file specified by ``path``.
+        If ``times`` is ``None``, then the file's access and modified times
+        are set to the current time.  Otherwise, ``times`` must be a 2-tuple
+        of numbers, of the form ``(atime, mtime)``, which is used to set the
+        access and modified times, respectively.  This bizarre API is mimicked
+        from Python for the sake of consistency -- I apologize.
 
         :param str path: path of the file to modify
         :param tuple times:
@@ -524,8 +540,7 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         method on Python file objects.
 
         :param str path: path of the file to modify
-        :param size: the new size of the file
-        :type size: int or long
+        :param int size: the new size of the file
         """
         path = self._adjust_cwd(path)
         self._log(DEBUG, 'truncate(%r, %r)' % (path, size))
@@ -564,7 +579,7 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         :param str path: path to be normalized
         :return: normalized form of the given path (as a `str`)
 
-        :raises IOError: if the path can't be resolved on the server
+        :raises: ``IOError`` -- if the path can't be resolved on the server
         """
         path = self._adjust_cwd(path)
         self._log(DEBUG, 'normalize(%r)' % path)
@@ -587,7 +602,8 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
         :param str path: new current working directory
 
-        :raises IOError: if the requested path doesn't exist on the server
+        :raises:
+            ``IOError`` -- if the requested path doesn't exist on the server
 
         .. versionadded:: 1.4
         """
@@ -595,7 +611,8 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
             self._cwd = None
             return
         if not stat.S_ISDIR(self.stat(path).st_mode):
-            raise SFTPError(errno.ENOTDIR, "%s: %s" % (os.strerror(errno.ENOTDIR), path))
+            raise SFTPError(
+                errno.ENOTDIR, "%s: %s" % (os.strerror(errno.ENOTDIR), path))
         self._cwd = b(self.normalize(path))
 
     def getcwd(self):
@@ -606,8 +623,21 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
         .. versionadded:: 1.4
         """
+        # TODO: make class initialize with self._cwd set to self.normalize('.')
         return self._cwd and u(self._cwd)
 
+    def _transfer_with_callback(self, reader, writer, file_size, callback):
+        size = 0
+        while True:
+            data = reader.read(32768)
+            writer.write(data)
+            size += len(data)
+            if len(data) == 0:
+                break
+            if callback is not None:
+                callback(size, file_size)
+        return size
+
     def putfo(self, fl, remotepath, file_size=0, callback=None, confirm=True):
         """
         Copy the contents of an open file object (``fl``) to the SFTP server as
@@ -616,7 +646,7 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
         The SFTP operations use pipelining for speed.
 
-        :param file fl: opened file or file-like object to copy
+        :param fl: opened file or file-like object to copy
         :param str remotepath: the destination path on the SFTP server
         :param int file_size:
             optional size parameter passed to callback. If none is specified,
@@ -637,19 +667,14 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         """
         with self.file(remotepath, 'wb') as fr:
             fr.set_pipelined(True)
-            size = 0
-            while True:
-                data = fl.read(32768)
-                fr.write(data)
-                size += len(data)
-                if callback is not None:
-                    callback(size, file_size)
-                if len(data) == 0:
-                    break
+            size = self._transfer_with_callback(
+                reader=fl, writer=fr, file_size=file_size, callback=callback
+            )
         if confirm:
             s = self.stat(remotepath)
             if s.st_size != size:
-                raise IOError('size mismatch in put!  %d != %d' % (s.st_size, size))
+                raise IOError(
+                    'size mismatch in put!  %d != %d' % (s.st_size, size))
         else:
             s = SFTPAttributes()
         return s
@@ -673,7 +698,8 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
             whether to do a stat() on the file afterwards to confirm the file
             size
 
-        :return: an `.SFTPAttributes` object containing attributes about the given file
+        :return: an `.SFTPAttributes` object containing attributes about the
+            given file
 
         .. versionadded:: 1.4
         .. versionchanged:: 1.7.4
@@ -702,19 +728,12 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
         .. versionadded:: 1.10
         """
+        file_size = self.stat(remotepath).st_size
         with self.open(remotepath, 'rb') as fr:
-            file_size = self.stat(remotepath).st_size
-            fr.prefetch()
-            size = 0
-            while True:
-                data = fr.read(32768)
-                fl.write(data)
-                size += len(data)
-                if callback is not None:
-                    callback(size, file_size)
-                if len(data) == 0:
-                    break
-        return size
+            fr.prefetch(file_size)
+            return self._transfer_with_callback(
+                reader=fr, writer=fl, file_size=file_size, callback=callback
+            )
 
     def get(self, remotepath, localpath, callback=None):
         """
@@ -732,14 +751,14 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         .. versionchanged:: 1.7.4
             Added the ``callback`` param
         """
-        file_size = self.stat(remotepath).st_size
         with open(localpath, 'wb') as fl:
             size = self.getfo(remotepath, fl, callback)
         s = os.stat(localpath)
         if s.st_size != size:
-            raise IOError('size mismatch in get!  %d != %d' % (s.st_size, size))
+            raise IOError(
+                'size mismatch in get!  %d != %d' % (s.st_size, size))
 
-    ###  internals...
+    # ...internals...
 
     def _request(self, t, *arg):
         num = self._async_request(type(None), t, *arg)
@@ -761,13 +780,14 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
                 elif isinstance(item, SFTPAttributes):
                     item._pack(msg)
                 else:
-                    raise Exception('unknown type for %r type %r' % (item, type(item)))
+                    raise Exception(
+                        'unknown type for %r type %r' % (item, type(item)))
             num = self.request_number
             self._expecting[num] = fileobj
-            self._send_packet(t, msg)
             self.request_number += 1
         finally:
             self._lock.release()
+        self._send_packet(t, msg)
         return num
 
     def _read_response(self, waitfor=None):
@@ -778,21 +798,29 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
                 raise SSHException('Server connection dropped: %s' % str(e))
             msg = Message(data)
             num = msg.get_int()
-            if num not in self._expecting:
-                # might be response for a file that was closed before responses came back
-                self._log(DEBUG, 'Unexpected response #%d' % (num,))
-                if waitfor is None:
-                    # just doing a single check
-                    break
-                continue
-            fileobj = self._expecting[num]
-            del self._expecting[num]
+            self._lock.acquire()
+            try:
+                if num not in self._expecting:
+                    # might be response for a file that was closed before
+                    # responses came back
+                    self._log(DEBUG, 'Unexpected response #%d' % (num,))
+                    if waitfor is None:
+                        # just doing a single check
+                        break
+                    continue
+                fileobj = self._expecting[num]
+                del self._expecting[num]
+            finally:
+                self._lock.release()
             if num == waitfor:
                 # synchronous
                 if t == CMD_STATUS:
                     self._convert_status(msg)
                 return t, msg
-            if fileobj is not type(None):
+
+            # can not rewrite this to deal with E721, either as a None check
+            # nor as not an instance of None or NoneType
+            if fileobj is not type(None):  # noqa
                 fileobj._async_response(t, msg, num)
             if waitfor is None:
                 # just doing a single check
@@ -840,6 +868,6 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
 
 class SFTP(SFTPClient):
     """
-    An alias for `.SFTPClient` for backwards compatability.
+    An alias for `.SFTPClient` for backwards compatibility.
     """
     pass