run everything in either local shell (shell=true), or remote shell (ssh). this it to allow external shell piping

tests/test_executenode.py

@@ -26,9 +26,9 @@ class TestExecuteNode(unittest2.TestCase):
         with self.subTest("multiline tabsplit"):
             self.assertEqual(node.run(["echo","l1c1\tl1c2\nl2c1\tl2c2"], tab_split=True), [['l1c1', 'l1c2'], ['l2c1', 'l2c2']])
 
-        #escaping test (shouldnt be a problem locally, single quotes can be a problem remote via ssh)
+        #escaping test
         with self.subTest("escape test"):
-            s="><`'\"@&$()$bla\\/.*!#test _+-={}[]|"
+            s="><`'\"@&$()$bla\\/.* !#test _+-={}[]| ${bla} $bla"
             self.assertEqual(node.run(["echo",s]), [s])
 
         #return std err as well, trigger stderr by listing something non existing

zfs_autobackup/CmdPipe.py

@@ -1,6 +1,7 @@
 import subprocess
 import os
 import select
+import shlex
 
 class CmdPipe:
     """a pipe of one or more commands. also takes care of utf-8 encoding/decoding and line based parsing"""
@@ -17,31 +18,32 @@ class CmdPipe:
         self.readonly = readonly
         self._should_execute = True
 
-    def add(self, cmd, readonly=False, stderr_handler=None, exit_handler=None):
+    def add(self, cmd, readonly=False, stderr_handler=None, exit_handler=None, shell=False):
         """adds a command to pipe"""
 
         self.items.append({
             'cmd': cmd,
             'stderr_handler': stderr_handler,
-            'exit_handler': exit_handler
+            'exit_handler': exit_handler,
+            'shell': shell
         })
 
         if not readonly and self.readonly:
             self._should_execute = False
 
     def __str__(self):
-        """transform into oneliner for debugging and testing """
+        """transform into oneliner for debugging and testing. this should generate a copy-pastable string for in a console """
 
-        #just one command?
-        if len(self.items)==1:
-            return " ".join(self.items[0]['cmd'])
-
-        #an actual pipe
         ret = ""
         for item in self.items:
             if ret:
                 ret = ret + " | "
-            ret = ret + "(" + " ".join(item['cmd']) + ")"
+            if item['shell']:
+                #its already copy pastable for a shell:
+                ret = ret + "(" + " ".join(item['cmd']) + ")"
+            else:
+                #make it copy-pastable, will make a mess of quotes sometimes, but is correct
+                ret = ret + "(" + shlex.join(item['cmd']) + ")"
 
         return ret
 
@@ -69,7 +71,7 @@ class CmdPipe:
                 encoded_cmd.append(arg.encode('utf-8'))
 
             item['process'] = subprocess.Popen(encoded_cmd, env=os.environ, stdout=subprocess.PIPE, stdin=stdin,
-                                               stderr=subprocess.PIPE)
+                                               stderr=subprocess.PIPE, shell=item['shell'])
 
             selectors.append(item['process'].stderr)
 

zfs_autobackup/ExecuteNode.py

@@ -1,7 +1,7 @@
 import os
 import select
 import subprocess
-
+import shlex
 from zfs_autobackup.CmdPipe import CmdPipe
 from zfs_autobackup.LogStub import LogStub
 
@@ -48,30 +48,23 @@ class ExecuteNode(LogStub):
     #     else:
     #         self.error("STDERR|> " + line.rstrip())
 
-    def _remote_cmd(self, cmd):
-        """transforms cmd in correct form for remote over ssh, if needed"""
+    def _shell_cmd(self, cmd):
+        """prefix specified ssh shell to command and escape shell characters"""
 
-        # use ssh?
-        if self.ssh_to is not None:
-            encoded_cmd = []
-            encoded_cmd.append("ssh")
+        ret=[]
+
+        #add remote shell
+        if not self.is_local():
+            ret=["ssh"]
 
             if self.ssh_config is not None:
-                encoded_cmd.extend(["-F", self.ssh_config])
+                ret.extend(["-F", self.ssh_config])
 
-            encoded_cmd.append(self.ssh_to)
+            ret.append(self.ssh_to)
 
-            for arg in cmd:
-                # add single quotes for remote commands to support spaces and other weird stuff (remote commands are
-                # executed in a shell) and escape existing single quotes (bash needs ' to end the quoted string,
-                # then a \' for the actual quote and then another ' to start a new quoted string) (and then python
-                # needs the double \ to get a single \)
-                encoded_cmd.append(("'" + arg.replace("'", "'\\''") + "'"))
-
-            return encoded_cmd
-        else:
-            return(cmd)
+        ret.append(shlex.join(cmd))
 
+        return ret
 
     def is_local(self):
         return self.ssh_to is None
@@ -81,6 +74,8 @@ class ExecuteNode(LogStub):
             return_stderr=False, pipe=False):
         """run a command on the node , checks output and parses/handle output and returns it
 
+        Either uses a local shell (sh -c) or remote shell (ssh) to execute the command. Therefore the command can have stuff like actual pipes in it, if you dont want to use pipe=True to pipe stuff.
+
         :param cmd: the actual command, should be a list, where the first item is the command
                     and the rest are parameters.
         :param pipe: return CmdPipe instead of executing it.
@@ -121,9 +116,8 @@ class ExecuteNode(LogStub):
             if (valid_exitcodes != []) and (exit_code not in valid_exitcodes):
              raise (ExecuteError("Command '{}' returned exit code {} (valid codes: {})".format(" ".join(cmd), exit_code, valid_exitcodes)))
 
-        # add command to pipe
-        encoded_cmd = self._remote_cmd(cmd)
-        p.add(cmd=encoded_cmd, readonly=readonly, stderr_handler=stderr_handler, exit_handler=exit_handler)
+        #add shell command and handlers to pipe
+        p.add(cmd=self._shell_cmd(cmd), readonly=readonly, stderr_handler=stderr_handler, exit_handler=exit_handler, shell=self.is_local())
 
         # return pipe instead of executing?
         if pipe:

zfs_autobackup/ZfsDataset.py

@@ -557,17 +557,17 @@ class ZfsDataset:
             cmd.append(self.name)
 
         # #add custom output pipes?
-        #local so do our own piping
-        if self.zfs_node.is_local():
-            output_pipe = self.zfs_node.run(cmd, pipe=True, readonly=True)
-            for pipe_cmd in output_pipes:
-                output_pipe=self.zfs_node.run(pipe_cmd.split(" "), inp=output_pipe, pipe=True, readonly=False)
-        #remote, so add with actual | and let remote shell handle it
-        else:
-            for pipe_cmd in output_pipes:
-                cmd.append("|")
-                cmd.extend(pipe_cmd.split(" "))
-            output_pipe = self.zfs_node.run(cmd, pipe=True, readonly=True)
+        # #local so do our own piping
+        # if self.zfs_node.is_local():
+        #     output_pipe = self.zfs_node.run(cmd, pipe=True, readonly=True)
+        #     for pipe_cmd in output_pipes:
+        #         output_pipe=self.zfs_node.run(pipe_cmd.split(" "), inp=output_pipe, pipe=True, readonly=False)
+        # #remote, so add with actual | and let remote shell handle it
+        # else:
+        #     for pipe_cmd in output_pipes:
+        #         cmd.append("|")
+        #         cmd.extend(pipe_cmd.split(" "))
+        output_pipe = self.zfs_node.run(cmd, pipe=True, readonly=True)
 
         return output_pipe