Update client

- add new PuttySSHVendor class
- update SubprocessSSHVendor
- add new tests for SubprocessSSHVendors and PuttySSHVendor

dulwich/client.py

@@ -1091,7 +1091,7 @@ class SSHVendor(object):
         :param command: Command to run (as argv array)
         :param username: Optional ame of user to log in as
         :param port: Optional SSH port to use
-        :param password: Optional ssh password for login
+        :param password: Optional ssh password for login or private key
         :param key_filename: Optional path to private keyfile
         """
         raise NotImplementedError(self.run_command)
@@ -1109,38 +1109,55 @@ class SubprocessSSHVendor(SSHVendor):
 
     def run_command(self, host, command, username=None, port=None,
                     password=None, key_filename=None):
+
+        if password:
+            raise NotImplementedError(
+                "You can't set password or passphrase for ssh key "
+                "with SubprocessSSHVendor, use ParamikoSSHVendor instead"
+            )
+
+        args = ['ssh', '-x']
+
+        if port:
+            args.extend(['-p', str(port)])
+
+        if key_filename:
+            args.extend(['-i', str(key_filename)])
+
+        if username:
+            host = '%s@%s' % (username, host)
+        if host.startswith('-'):
+            raise StrangeHostname(hostname=host)
+        args.append(host)
+
+        proc = subprocess.Popen(args + [command], bufsize=0,
+                                stdin=subprocess.PIPE,
+                                stdout=subprocess.PIPE)
+        return SubprocessWrapper(proc)
+
+
+class PuttySSHVendor(SSHVendor):
+    """SSH vendor that shells out to the local 'putty' command."""
+
+    def run_command(self, host, command, username=None, port=None,
+                    password=None, key_filename=None):
+
         if password and key_filename:
             raise NotImplementedError(
-                "You can't set passphrase for ssh key with "
-                "SubprocessSSHVendor, use ParamikoSSHVendor instead"
+                "You can't set passphrase for ssh key "
+                "with PuttySSHVendor, use ParamikoSSHVendor instead"
             )
 
         if sys.platform == 'win32':
             args = ['putty.exe', '-ssh']
-            if password:
-                args.extend(['-pw', str(password)])
         else:
-            if password:
-                try:
-                    subprocess.call(["sshpass"])
-                except OSError as e:
-                    import warnings
-                    warnings.warn(
-                        "You need sshpass for using password option, "
-                        "you can use ssh-add "
-                        "or private key without password "
-                        "or ParamikoSSHVendor"
-                    )
-                    raise e
-                args = ['sshpass', '-p', str(password)]
-            else:
-                args = ['ssh', '-x']
+            args = ['putty', '-ssh']
+
+        if password:
+            args.extend(['-pw', str(password)])
 
         if port:
-            if sys.platform == 'win32':
-                args.extend(['-P', str(port)])
-            else:
-                args.extend(['-p', str(port)])
+            args.extend(['-P', str(port)])
 
         if key_filename:
             args.extend(['-i', str(key_filename)])

dulwich/tests/test_client.py

@@ -23,6 +23,7 @@ import base64
 import sys
 import shutil
 import tempfile
+import mock
 
 try:
     from urllib import quote as urlquote
@@ -51,6 +52,7 @@ from dulwich.client import (
     SendPackError,
     StrangeHostname,
     SubprocessSSHVendor,
+    PuttySSHVendor,
     UpdateRefsError,
     default_urllib3_manager,
     get_transport_and_path,
@@ -630,6 +632,8 @@ class TestSSHVendor(object):
         self.command = ""
         self.username = None
         self.port = None
+        self.password = None
+        self.key_filename = None
 
     def run_command(self, host, command, username=None, port=None,
                     password=None, key_filename=None):
@@ -637,6 +641,8 @@ class TestSSHVendor(object):
         self.command = command
         self.username = username
         self.port = port
+        self.password = password
+        self.key_filename = key_filename
 
         class Subprocess:
             pass
@@ -974,3 +980,91 @@ class SubprocessSSHVendorTests(TestCase):
         vendor = SubprocessSSHVendor()
         self.assertRaises(StrangeHostname, vendor.run_command, '--weird-host',
                           'git-clone-url')
+
+    def test_run_command_password(self):
+        vendor = SubprocessSSHVendor()
+        self.assertRaises(NotImplementedError, vendor.run_command, 'host',
+                          'git-clone-url', password='12345')
+
+    def test_run_command_password_and_privkey(self):
+        vendor = SubprocessSSHVendor()
+        self.assertRaises(NotImplementedError, vendor.run_command,
+                          'host', 'git-clone-url',
+                          password='12345', key_filename='/tmp/id_rsa')
+
+    @mock.patch('dulwich.client.subprocess.Popen')
+    def test_run_command_with_port_username_and_privkey(self, mocked_popen):
+        expected = ['ssh', '-x', '-p', '2200',
+                    '-i', '/tmp/id_rsa', 'user@host', 'git-clone-url']
+
+        mocked_popen.return_value.stdout = BytesIO(b"stdout")
+        mocked_popen.return_value.stderr = BytesIO(b"stderr")
+        mocked_popen.return_value.wait.return_value = False
+        mocked_popen.return_value.returncode = 0
+        mocked_popen.return_value.communicate.return_value = ('Running', '')
+
+        vendor = SubprocessSSHVendor()
+        vendor.run_command('host', 'git-clone-url',
+                           username='user', port='2200',
+                           key_filename='/tmp/id_rsa')
+
+        args, kwargs = mocked_popen.call_args
+        self.assertListEqual(expected, args[0])
+
+
+class PuttySSHVendorTests(TestCase):
+
+    def test_run_command_dashes(self):
+        vendor = PuttySSHVendor()
+        self.assertRaises(StrangeHostname, vendor.run_command, '--weird-host',
+                          'git-clone-url')
+
+    def test_run_command_password_and_privkey(self):
+        vendor = PuttySSHVendor()
+        self.assertRaises(NotImplementedError, vendor.run_command,
+                          'host', 'git-clone-url',
+                          password='12345', key_filename='/tmp/id_rsa')
+
+    @mock.patch('dulwich.client.subprocess.Popen')
+    def test_run_command_password(self, mocked_popen):
+        if sys.platform == 'win32':
+            binary = ['putty.exe', '-ssh']
+        else:
+            binary = ['putty', '-ssh']
+        expected = binary + ['-pw', '12345', 'host', 'git-clone-url']
+
+        mocked_popen.return_value.stdout = BytesIO(b"stdout")
+        mocked_popen.return_value.stderr = BytesIO(b"stderr")
+        mocked_popen.return_value.wait.return_value = False
+        mocked_popen.return_value.returncode = 0
+        mocked_popen.return_value.communicate.return_value = ('Running', '')
+
+        vendor = PuttySSHVendor()
+        vendor.run_command('host', 'git-clone-url', password='12345')
+
+        args, kwargs = mocked_popen.call_args
+        self.assertListEqual(expected, args[0])
+
+    @mock.patch('dulwich.client.subprocess.Popen')
+    def test_run_command_with_port_username_and_privkey(self, mocked_popen):
+        if sys.platform == 'win32':
+            binary = ['putty.exe', '-ssh']
+        else:
+            binary = ['putty', '-ssh']
+        expected = binary + [
+            '-P', '2200', '-i', '/tmp/id_rsa',
+            'user@host', 'git-clone-url']
+
+        mocked_popen.return_value.stdout = BytesIO(b"stdout")
+        mocked_popen.return_value.stderr = BytesIO(b"stderr")
+        mocked_popen.return_value.wait.return_value = False
+        mocked_popen.return_value.returncode = 0
+        mocked_popen.return_value.communicate.return_value = ('Running', '')
+
+        vendor = PuttySSHVendor()
+        vendor.run_command('host', 'git-clone-url',
+                           username='user', port='2200',
+                           key_filename='/tmp/id_rsa')
+
+        args, kwargs = mocked_popen.call_args
+        self.assertListEqual(expected, args[0])