Merge branch '1.15' into 1.16

3 files changed, 23 insertions, 20 deletions

diff --git a/paramiko/sftp_client.py b/paramiko/sftp_client.py
index 57225558..daaae3ef 100644
--- a/paramiko/sftp_client.py
+++ b/paramiko/sftp_client.py
@@ -592,6 +592,18 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         # 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
@@ -621,15 +633,9 @@ 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:
@@ -689,16 +695,10 @@ class SFTPClient(BaseSFTP, ClosingContextManager):
         file_size = self.stat(remotepath).st_size
         with self.open(remotepath, 'rb') as fr:
             fr.prefetch(file_size)
+            return self._transfer_with_callback(
+                reader=fr, writer=fl, file_size=file_size, callback=callback
+            )
 
-            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
 
     def get(self, remotepath, localpath, callback=None):
diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst
index 072607ee..69b65360 100644
--- a/sites/www/changelog.rst
+++ b/sites/www/changelog.rst
@@ -2,6 +2,9 @@
 Changelog
 =========
 
+* :bug:`632` Fix logic bug in the SFTP client's callback-calling functionality;
+  previously there was a chance the given callback would fire twice at the end
+  of a transfer. Thanks to ``@ab9-er`` for catch & original patch.
 * :support:`612` Identify & work around a race condition in the test for
   handshake timeouts, which was causing frequent test failures for a subset of
   contributors as well as Travis-CI (usually, but not always, limited to Python
diff --git a/tests/test_sftp.py b/tests/test_sftp.py
index 131b8abf..ff146ade 100755
--- a/tests/test_sftp.py
+++ b/tests/test_sftp.py
@@ -610,7 +610,7 @@ class SFTPTest (unittest.TestCase):
 
         with sftp.open(FOLDER + '/bunny.txt', 'rb') as f:
             self.assertEqual(text, f.read(128))
-        self.assertEqual((41, 41), saved_progress[-1])
+        self.assertEqual([(41, 41)], saved_progress)
 
         os.unlink(localname)
         fd, localname = mkstemp()
@@ -620,7 +620,7 @@ class SFTPTest (unittest.TestCase):
 
         with open(localname, 'rb') as f:
             self.assertEqual(text, f.read(128))
-        self.assertEqual((41, 41), saved_progress[-1])
+        self.assertEqual([(41, 41)], saved_progress)
 
         os.unlink(localname)
         sftp.unlink(FOLDER + '/bunny.txt')