From c981a61eabbcc57046de9d8e6381b7b31b347fcd Mon Sep 17 00:00:00 2001 From: nixon Date: Sun, 20 Oct 2019 00:36:22 -0500 Subject: [PATCH] fix single quote escape for append, contains revert back to fabric1 version of `_escape_for_regex` to handle single quotes and other special characters. also escaped double quotes in filename and added testcases. refs #34 --- patchwork/files.py | 51 ++++++++++++++++++--------- tests/files.py | 86 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 112 insertions(+), 25 deletions(-) diff --git a/patchwork/files.py b/patchwork/files.py index 1e68512..a6ea6a4 100644 --- a/patchwork/files.py +++ b/patchwork/files.py @@ -2,8 +2,6 @@ Tools for file and directory management. """ -import re - from invoke.vendor import six from .util import set_runner @@ -25,12 +23,16 @@ def directory(c, runner, path, user=None, group=None, mode=None): :param str mode: ``chmod`` compatible mode string to apply to the directory. """ - runner("mkdir -p {}".format(path)) + path = path.replace(r'"', r'\"') + runner('mkdir -p "{}"'.format(path)) if user is not None: group = group or user - runner("chown {}:{} {}".format(user, group, path)) + user = user.replace(r'"', r'\"') + group = group.replace(r'"', r'\"') + runner('chown "{}:{}" "{}"'.format(user, group, path)) if mode is not None: - runner("chmod {} {}".format(mode, path)) + mode = mode.replace(r'"', r'\"') + runner('chmod "{}" "{}"'.format(mode, path)) @set_runner @@ -43,7 +45,7 @@ def exists(c, runner, path): :param str path: Path to check for existence. """ - cmd = 'test -e "$(echo {})"'.format(path) + cmd = 'test -e "$(echo {})"'.format(path.replace(r'"', r'\"')) return runner(cmd, hide=True, warn=True).ok @@ -126,17 +128,34 @@ def append(c, runner, filename, text, partial=False, escape=True): and contains(c, filename, regex, escape=False, runner=runner) ): continue - line = line.replace("'", r"'\\''") if escape else line - runner("echo '{}' >> {}".format(line, filename)) + line = line.replace("'", r"'\''") if escape else line + filename = filename.replace(r'"', r'\"') + runner("echo '{}' >> \"{}\"".format(line, filename)) def _escape_for_regex(text): """Escape ``text`` to allow literal matching using egrep""" - regex = re.escape(text) - # Seems like double escaping is needed for \ - regex = regex.replace("\\\\", "\\\\\\") - # Triple-escaping seems to be required for $ signs - regex = regex.replace(r"\$", r"\\\$") - # Whereas single quotes should not be escaped - regex = regex.replace(r"\'", "'") - return regex + re_specials = '\\^$|(){}[]*+?.' + sh_specials = '\\$`"' + re_chars = [] + sh_chars = [] + + # Removes newline characters. + # egrep does not support multi-line patterns, so they should not be used + # with these functions. But, this might as well remain consistent with + # its original behavior regarding newlines, which was to escape them, + # causing the shell to ignore/omit them. + + for c in text: + if c == '\n': + continue + if c in re_specials: + re_chars.append('\\') + re_chars.append(c) + + for c in re_chars: + if c in sh_specials: + sh_chars.append('\\') + sh_chars.append(c) + + return ''.join(sh_chars) diff --git a/tests/files.py b/tests/files.py index ded49ae..7822fe5 100644 --- a/tests/files.py +++ b/tests/files.py @@ -1,6 +1,6 @@ -from mock import call +from mock import call, patch -from patchwork.files import directory +from patchwork.files import directory, append, contains class files: @@ -9,26 +9,94 @@ class directory_: def base_case_creates_dir_with_dash_p(self, cxn): directory(cxn, "/some/dir") - cxn.run.assert_called_once_with("mkdir -p /some/dir") + cxn.run.assert_called_once_with('mkdir -p "/some/dir"') + + def creates_dir_with_double_quotes(self, cxn): + directory(cxn, '/some/double"quote') + cxn.run.assert_called_once_with('mkdir -p "/some/double\\"quote"') def user_sets_owner_and_group(self, cxn): directory(cxn, "/some/dir", user="user") - cxn.run.assert_any_call("chown user:user /some/dir") + cxn.run.assert_any_call('chown "user:user" "/some/dir"') def group_may_be_given_to_change_group(self, cxn): directory(cxn, "/some/dir", user="user", group="admins") - cxn.run.assert_any_call("chown user:admins /some/dir") + cxn.run.assert_any_call('chown "user:admins" "/some/dir"') def mode_adds_a_chmod(self, cxn): directory(cxn, "/some/dir", mode="0700") - cxn.run.assert_any_call("chmod 0700 /some/dir") + cxn.run.assert_any_call('chmod "0700" "/some/dir"') def all_args_in_play(self, cxn): directory( cxn, "/some/dir", user="user", group="admins", mode="0700" ) assert cxn.run.call_args_list == [ - call("mkdir -p /some/dir"), - call("chown user:admins /some/dir"), - call("chmod 0700 /some/dir"), + call('mkdir -p "/some/dir"'), + call('chown "user:admins" "/some/dir"'), + call('chmod "0700" "/some/dir"'), + ] + + def contains_issue34(self, cxn): + contains(cxn, "/some/file", "'*'") + cxn.run.assert_any_call( + r'''egrep "'\\*'" "/some/file"''', + hide=True, warn=True + ) + + def contains_trailing_dollar(self, cxn): + contains(cxn, "/some/file", "trailing $") + cxn.run.assert_called_once_with( + r'egrep "trailing \\\$" "/some/file"', + hide=True, warn=True + ) + + def contains_trailing_backslash(self, cxn): + contains(cxn, "/some/file", "trailing \\") + cxn.run.assert_called_once_with( + r'egrep "trailing \\\\" "/some/file"', + hide=True, warn=True + ) + + @patch('patchwork.files.contains') + def append_ending_single_quote(self, m, cxn): + m.return_value = False + append(cxn, "/some/file", "alias l='ls -rtl'") + cxn.run.assert_any_call( + 'echo \'alias l=\'\\\'\'ls -rtl\'\\\'\'\' >> "/some/file"' + ) + + @patch('patchwork.files.contains') + def append_trailing_dollar(self, m, cxn): + m.return_value = False + append(cxn, "/some/file", "trailing $") + cxn.run.assert_any_call( + r'''echo 'trailing $' >> "/some/file"''', + ) + + @patch('patchwork.files.contains') + def append_trailing_backslash(self, m, cxn): + m.return_value = False + append(cxn, "/some/file", "trailing \\") + + assert cxn.run.call_args_list == [ + call('test -e "$(echo /some/file)"', hide=True, warn=True), + call(r'''echo 'trailing \' >> "/some/file"''') ] + + + @patch('patchwork.files.contains') + def append_trailing_backtick(self, m, cxn): + m.return_value = False + append(cxn, "/some/file", "trailing `") + cxn.run.assert_any_call( + r'''echo 'trailing `' >> "/some/file"''' + ) + + @patch('patchwork.files.contains') + def append_issue_34(self, m, cxn): + m.return_value = False + append(cxn, "/some/file", "listen_addresses='*'", escape=True) + cxn.run.assert_any_call( + 'echo \'listen_addresses=\'\\\'\'*\'\\\'\'\' >> "/some/file"' + )