diff --git a/tests/test_executenode.py b/tests/test_executenode.py index ea7e022..07cfd52 100644 --- a/tests/test_executenode.py +++ b/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 diff --git a/zfs_autobackup/CmdPipe.py b/zfs_autobackup/CmdPipe.py index fbbeb89..fc64edb 100644 --- a/zfs_autobackup/CmdPipe.py +++ b/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) diff --git a/zfs_autobackup/ExecuteNode.py b/zfs_autobackup/ExecuteNode.py index 5d067f0..3595465 100644 --- a/zfs_autobackup/ExecuteNode.py +++ b/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: diff --git a/zfs_autobackup/ZfsDataset.py b/zfs_autobackup/ZfsDataset.py index 0c4f4b7..9f63974 100644 --- a/zfs_autobackup/ZfsDataset.py +++ b/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