Simplify handling of SSH command invocation.

NEWS

@@ -1,5 +1,12 @@
 0.11.3	UNRELEASED
 
+ BUG FIXES
+
+  * Simplify handling of SSH command invocation.
+    Fixes quoting of paths. Thanks, Thomas Liebetraut. (#384)
+
+  * Fix inconsistent handling of trailing slashes for DictRefsContainer. (#383)
+
 0.11.2	2015-09-18
 
  IMPROVEMENTS

dulwich/client.py

@@ -850,8 +850,7 @@ class SubprocessSSHVendor(SSHVendor):
     """SSH vendor that shells out to the local 'ssh' command."""
 
     def run_command(self, host, command, username=None, port=None):
-        if (type(command) is not list or
-            not all([isinstance(b, bytes) for b in command])):
+        if not isinstance(command, bytes):
             raise TypeError(command)
 
         #FIXME: This has no way to deal with passwords..
@@ -861,7 +860,7 @@ def run_command(self, host, command, username=None, port=None):
         if username is not None:
             host = '%s@%s' % (username, host)
         args.append(host)
-        proc = subprocess.Popen(args + command,
+        proc = subprocess.Popen(args + [command],
                                 stdin=subprocess.PIPE,
                                 stdout=subprocess.PIPE)
         return SubprocessWrapper(proc)
@@ -896,11 +895,7 @@ def __init__(self, host, port=None, username=None, vendor=None, **kwargs):
     def _get_cmd_path(self, cmd):
         cmd = self.alternative_paths.get(cmd, b'git-' + cmd)
         assert isinstance(cmd, bytes)
-        if sys.version_info[:2] <= (2, 6):
-            return shlex.split(cmd)
-        else:
-            # TODO(jelmer): Don't decode/encode here
-            return [x.encode('ascii') for x in shlex.split(cmd.decode('ascii'))]
+        return cmd
 
     def _connect(self, cmd, path):
         if type(cmd) is not bytes:
@@ -909,7 +904,7 @@ def _connect(self, cmd, path):
             raise TypeError(path)
         if path.startswith(b"/~"):
             path = path[1:]
-        argv = self._get_cmd_path(cmd) + [b"'" + path + b"'"]
+        argv = self._get_cmd_path(cmd) + b" '" + path + b"'"
         con = self.ssh_vendor.run_command(
             self.host, argv, port=self.port, username=self.username)
         return (Protocol(con.read, con.write, con.close,

dulwich/contrib/paramiko_vendor.py

@@ -108,49 +108,14 @@ def close(self):
         self.stop_monitoring()
 
 
-# {{{ shell quoting
-
-# Adapted from
-# https://github.com/python/cpython/blob/8cd133c63f156451eb3388b9308734f699f4f1af/Lib/shlex.py#L278
-
-def is_shell_safe(s):
-    import re
-    import sys
-
-    flags = 0
-    if sys.version_info >= (3,):
-        flags = re.ASCII
-
-    unsafe_re = re.compile(br'[^\w@%+=:,./-]', flags)
-
-    return unsafe_re.search(s) is None
-
-
-def shell_quote(s):
-    """Return a shell-escaped version of the byte string *s*."""
-
-    # Unconditionally quotes because that's apparently git's behavior, too,
-    # and some code hosting sites (notably Bitbucket) appear to rely on that.
-
-    if not s:
-        return b"''"
-
-    # use single quotes, and put single quotes into double quotes
-    # the string $'b is then quoted as '$'"'"'b'
-    return b"'" + s.replace(b"'", b"'\"'\"'") + b"'"
-
-# }}}
-
-
 class ParamikoSSHVendor(object):
 
     def __init__(self):
         self.ssh_kwargs = {}
 
     def run_command(self, host, command, username=None, port=None,
                     progress_stderr=None):
-        if (type(command) is not list or
-            not all([isinstance(b, bytes) for b in command])):
+        if not isinstance(command, bytes):
             raise TypeError(command)
         # Paramiko needs an explicit port. None is not valid
         if port is None:
@@ -166,18 +131,8 @@ def run_command(self, host, command, username=None, port=None,
         # Open SSH session
         channel = client.get_transport().open_session()
 
-        # Quote command
-        assert command
-        assert is_shell_safe(command[0])
-
-        quoted_command = (
-            command[0]
-            + b' '
-            + b' '.join(
-                shell_quote(c) for c in command[1:]))
-
         # Run commands
-        channel.exec_command(quoted_command)
+        channel.exec_command(command)
 
         return _ParamikoWrapper(
             client, channel, progress_stderr=progress_stderr)

dulwich/tests/compat/test_client.py

@@ -324,7 +324,7 @@ class TestSSHVendor(object):
 
     @staticmethod
     def run_command(host, command, username=None, port=None):
-        cmd, path = command
+        cmd, path = command.split(b' ')
         cmd = cmd.split(b'-', 1)
         path = path.replace(b"'", b"")
         p = subprocess.Popen(cmd + [path], bufsize=0, env=get_safe_env(), stdin=subprocess.PIPE,

dulwich/tests/test_client.py

@@ -502,8 +502,7 @@ def __init__(self):
         self.port = None
 
     def run_command(self, host, command, username=None, port=None):
-        if (type(command) is not list or
-            not all([isinstance(b, bytes) for b in command])):
+        if not isinstance(command, bytes):
             raise TypeError(command)
 
         self.host = host
@@ -535,19 +534,19 @@ def tearDown(self):
         client.get_ssh_vendor = self.real_vendor
 
     def test_default_command(self):
-        self.assertEqual([b'git-upload-pack'],
+        self.assertEqual(b'git-upload-pack',
                 self.client._get_cmd_path(b'upload-pack'))
 
     def test_alternative_command_path(self):
         self.client.alternative_paths[b'upload-pack'] = (
             b'/usr/lib/git/git-upload-pack')
-        self.assertEqual([b'/usr/lib/git/git-upload-pack'],
+        self.assertEqual(b'/usr/lib/git/git-upload-pack',
             self.client._get_cmd_path(b'upload-pack'))
 
     def test_alternative_command_path_spaces(self):
         self.client.alternative_paths[b'upload-pack'] = (
             b'/usr/lib/git/git-upload-pack -ibla')
-        self.assertEqual([b'/usr/lib/git/git-upload-pack', b'-ibla'],
+        self.assertEqual(b"/usr/lib/git/git-upload-pack -ibla",
             self.client._get_cmd_path(b'upload-pack'))
 
     def test_connect(self):
@@ -560,10 +559,10 @@ def test_connect(self):
         client._connect(b"command", b"/path/to/repo")
         self.assertEqual(b"username", server.username)
         self.assertEqual(1337, server.port)
-        self.assertEqual([b"git-command", b"'/path/to/repo'"], server.command)
+        self.assertEqual(b"git-command '/path/to/repo'", server.command)
 
         client._connect(b"relative-command", b"/~/path/to/repo")
-        self.assertEqual([b"git-relative-command", b"'~/path/to/repo'"],
+        self.assertEqual(b"git-relative-command '~/path/to/repo'",
                           server.command)
 
 

dulwich/tests/test_repository.py

@@ -504,9 +504,7 @@ def test_shell_hook_post_commit(self):
 
     def test_as_dict(self):
         def check(repo):
-            self.assertTrue(repo.refs.as_dict())
-            self.assertTrue(repo.refs.as_dict('refs/tags/'))
-            self.assertTrue(repo.refs.as_dict('refs/heads/'))
+            self.assertEqual(repo.refs.subkeys('refs/tags'), repo.refs.subkeys('refs/tags/'))
             self.assertEqual(repo.refs.as_dict('refs/tags'), repo.refs.as_dict('refs/tags/'))
             self.assertEqual(repo.refs.as_dict('refs/heads'), repo.refs.as_dict('refs/heads/'))