diff --git a/tests/test_executenode.py b/tests/test_executenode.py index 07cfd52..05ec8ce 100644 --- a/tests/test_executenode.py +++ b/tests/test_executenode.py @@ -28,7 +28,7 @@ class TestExecuteNode(unittest2.TestCase): #escaping test with self.subTest("escape test"): - s="><`'\"@&$()$bla\\/.* !#test _+-={}[]| ${bla} $bla" + s="><`'\"@&$()$bla\\/.* !#test _+-={}[]|${bla} $bla" self.assertEqual(node.run(["echo",s]), [s]) #return std err as well, trigger stderr by listing something non existing @@ -51,6 +51,15 @@ class TestExecuteNode(unittest2.TestCase): with self.subTest("stdin process with inp=None (shouldn't hang)"): self.assertEqual(node.run(["cat"]), []) + # let the system do the piping with an unescaped |: + with self.subTest("system piping test"): + + #first make sure the actual | character is still properly escaped: + self.assertEqual(node.run(["echo","|"]), ["|"]) + + #now pipe + self.assertEqual(node.run(["echo", "abc", node.PIPE, "tr", "a", "A" ]), ["Abc"]) + def test_basics_local(self): node=ExecuteNode(debug_output=True) self.basics(node) diff --git a/zfs_autobackup/CmdPipe.py b/zfs_autobackup/CmdPipe.py index 30c661a..0853a3b 100644 --- a/zfs_autobackup/CmdPipe.py +++ b/zfs_autobackup/CmdPipe.py @@ -24,7 +24,7 @@ class CmdPipe: self._should_execute = True def add(self, cmd, readonly=False, stderr_handler=None, exit_handler=None, shell=False): - """adds a command to pipe""" + """adds a command to pipe. called has to make sure its properly escaped.""" self.items.append({ 'cmd': cmd, diff --git a/zfs_autobackup/ExecuteNode.py b/zfs_autobackup/ExecuteNode.py index 4cb8e18..ca1e965 100644 --- a/zfs_autobackup/ExecuteNode.py +++ b/zfs_autobackup/ExecuteNode.py @@ -13,9 +13,12 @@ except ImportError: class ExecuteError(Exception): pass + class ExecuteNode(LogStub): """an endpoint to execute local or remote commands via ssh""" + PIPE=1 + def __init__(self, ssh_config=None, ssh_to=None, readonly=False, debug_output=False): """ssh_config: custom ssh config ssh_to: server you want to ssh to. none means local @@ -46,14 +49,14 @@ class ExecuteNode(LogStub): else: self.error("STDERR > " + line.rstrip()) - # def _parse_stderr_pipe(self, line, hide_errors): - # """parse stderr from pipe input process. can be overridden in subclass""" - # if hide_errors: - # self.debug("STDERR|> " + line.rstrip()) - # else: - # self.error("STDERR|> " + line.rstrip()) + def __quote(self, cmd): + """return quoted version of command. if it has value PIPE it will add an actual | """ + if cmd==self.PIPE: + return('|') + else: + return(cmd_quote(cmd)) - def _shell_cmd(self, cmd): + def __shell_cmd(self, cmd): """prefix specified ssh shell to command and escape shell characters""" ret=[] @@ -67,7 +70,7 @@ class ExecuteNode(LogStub): ret.append(self.ssh_to) - ret.append(" ".join(map(cmd_quote, cmd))) + ret.append(" ".join(map(self.__quote, cmd))) return ret @@ -82,7 +85,8 @@ class ExecuteNode(LogStub): 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. + and the rest are parameters. use ExecuteNode.PIPE to add an unescaped | + (if you want to use system piping instead of python piping) :param pipe: return CmdPipe instead of executing it. :param inp: Can be None, a string or a CmdPipe that was previously returned. :param tab_split: split tabbed files in output into a list @@ -122,7 +126,7 @@ class ExecuteNode(LogStub): raise (ExecuteError("Command '{}' returned exit code {} (valid codes: {})".format(" ".join(cmd), exit_code, valid_exitcodes))) #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()) + 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: