From 7131c75d9354a85864d20c61b06ba1d5d5f171b8 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Sat, 29 Aug 2020 03:22:22 +1000 Subject: [PATCH] powershell - fix quoting values (#71411) (#71449) * powershell - fix quoting values * Add ignore for smart quote skip (cherry picked from commit 72a7cb4a2c3036da5e3abb32c50713a262d0c063) --- .../fragments/powershell-fix-quoting.yaml | 2 + lib/ansible/plugins/connection/winrm.py | 2 +- lib/ansible/plugins/shell/powershell.py | 39 +++++++------------ .../targets/win_fetch/meta/main.yml | 2 + .../targets/win_fetch/tasks/main.yml | 23 +++++++++++ test/sanity/ignore.txt | 1 + 6 files changed, 43 insertions(+), 26 deletions(-) create mode 100644 changelogs/fragments/powershell-fix-quoting.yaml create mode 100644 test/integration/targets/win_fetch/meta/main.yml diff --git a/changelogs/fragments/powershell-fix-quoting.yaml b/changelogs/fragments/powershell-fix-quoting.yaml new file mode 100644 index 00000000000..68ffde593c1 --- /dev/null +++ b/changelogs/fragments/powershell-fix-quoting.yaml @@ -0,0 +1,2 @@ +bugfixes: +- powershell - fix escaping of strings that broken modules like fetch when dealing with special chars - https://github.com/ansible/ansible/issues/62781 diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 7f9e3658c66..3a1bc3d4123 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -649,7 +649,7 @@ class Connection(ConnectionBase): while True: try: script = ''' - $path = "%(path)s" + $path = '%(path)s' If (Test-Path -Path $path -PathType Leaf) { $buffer_size = %(buffer_size)d diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index d8c2e836d3c..7eeb4e0928b 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -120,9 +120,9 @@ class ShellModule(ShellBase): def remove(self, path, recurse=False): path = self._escape(self._unquote(path)) if recurse: - return self._encode_script('''Remove-Item "%s" -Force -Recurse;''' % path) + return self._encode_script('''Remove-Item '%s' -Force -Recurse;''' % path) else: - return self._encode_script('''Remove-Item "%s" -Force;''' % path) + return self._encode_script('''Remove-Item '%s' -Force;''' % path) def mkdtemp(self, basefile=None, system=False, mode=None, tmpdir=None): # Windows does not have an equivalent for the system temp files, so @@ -147,15 +147,15 @@ class ShellModule(ShellBase): if user_home_path == '~': script = 'Write-Output (Get-Location).Path' elif user_home_path.startswith('~\\'): - script = 'Write-Output ((Get-Location).Path + "%s")' % self._escape(user_home_path[1:]) + script = "Write-Output ((Get-Location).Path + '%s')" % self._escape(user_home_path[1:]) else: - script = 'Write-Output "%s"' % self._escape(user_home_path) + script = "Write-Output '%s'" % self._escape(user_home_path) return self._encode_script(script) def exists(self, path): path = self._escape(self._unquote(path)) script = ''' - If (Test-Path "%s") + If (Test-Path '%s') { $res = 0; } @@ -163,7 +163,7 @@ class ShellModule(ShellBase): { $res = 1; } - Write-Output "$res"; + Write-Output '$res'; Exit $res; ''' % path return self._encode_script(script) @@ -171,14 +171,14 @@ class ShellModule(ShellBase): def checksum(self, path, *args, **kwargs): path = self._escape(self._unquote(path)) script = ''' - If (Test-Path -PathType Leaf "%(path)s") + If (Test-Path -PathType Leaf '%(path)s') { $sp = new-object -TypeName System.Security.Cryptography.SHA1CryptoServiceProvider; - $fp = [System.IO.File]::Open("%(path)s", [System.IO.Filemode]::Open, [System.IO.FileAccess]::Read); + $fp = [System.IO.File]::Open('%(path)s', [System.IO.Filemode]::Open, [System.IO.FileAccess]::Read); [System.BitConverter]::ToString($sp.ComputeHash($fp)).Replace("-", "").ToLower(); $fp.Dispose(); } - ElseIf (Test-Path -PathType Container "%(path)s") + ElseIf (Test-Path -PathType Container '%(path)s') { Write-Output "3"; } @@ -264,22 +264,11 @@ class ShellModule(ShellBase): return m.group(1) return value - def _escape(self, value, include_vars=False): - '''Return value escaped for use in PowerShell command.''' - # http://www.techotopia.com/index.php/Windows_PowerShell_1.0_String_Quoting_and_Escape_Sequences - # http://stackoverflow.com/questions/764360/a-list-of-string-replacements-in-python - subs = [('\n', '`n'), ('\r', '`r'), ('\t', '`t'), ('\a', '`a'), - ('\b', '`b'), ('\f', '`f'), ('\v', '`v'), ('"', '`"'), - ('\'', '`\''), ('`', '``'), ('\x00', '`0')] - if include_vars: - subs.append(('$', '`$')) - pattern = '|'.join('(%s)' % re.escape(p) for p, s in subs) - substs = [s for p, s in subs] - - def replace(m): - return substs[m.lastindex - 1] - - return re.sub(pattern, replace, value) + def _escape(self, value): + '''Return value escaped for use in PowerShell single quotes.''' + # There are 5 chars that need to be escaped in a single quote. + # https://github.com/PowerShell/PowerShell/blob/b7cb335f03fe2992d0cbd61699de9d9aafa1d7c1/src/System.Management.Automation/engine/parser/CharTraits.cs#L265-L272 + return re.compile(u"(['\u2018\u2019\u201a\u201b])").sub(u'\\1\\1', value) def _encode_script(self, script, as_list=False, strict_mode=True, preserve_rc=True): '''Convert a PowerShell script to a single base64-encoded command.''' diff --git a/test/integration/targets/win_fetch/meta/main.yml b/test/integration/targets/win_fetch/meta/main.yml new file mode 100644 index 00000000000..9f37e96cd90 --- /dev/null +++ b/test/integration/targets/win_fetch/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: +- setup_remote_tmp_dir diff --git a/test/integration/targets/win_fetch/tasks/main.yml b/test/integration/targets/win_fetch/tasks/main.yml index 1c4ff0b142c..78b6fa02c9b 100644 --- a/test/integration/targets/win_fetch/tasks/main.yml +++ b/test/integration/targets/win_fetch/tasks/main.yml @@ -187,3 +187,26 @@ # Doesn't fail anymore, only returns a message. - "fetch_dir is not changed" - "fetch_dir.msg" + +- name: create file with special characters + raw: Set-Content -LiteralPath '{{ remote_tmp_dir }}\abc$not var''quote‘‘' -Value 'abc' + +- name: fetch file with special characters + fetch: + src: '{{ remote_tmp_dir }}\abc$not var''quote‘' + dest: '{{ host_output_dir }}/' + flat: yes + register: fetch_special_file + +- name: get content of fetched file + command: cat {{ (host_output_dir ~ "/abc$not var'quote‘") | quote }} + register: fetch_special_file_actual + delegate_to: localhost + +- name: assert fetch file with special characters + assert: + that: + - fetch_special_file is changed + - fetch_special_file.checksum == '34d4150adc3347f1dd8ce19fdf65b74d971ab602' + - fetch_special_file.dest == host_output_dir + "/abc$not var'quote‘" + - fetch_special_file_actual.stdout == 'abc' diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 2d1ed748e6a..90f5e1fe54b 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -304,6 +304,7 @@ test/integration/targets/want_json_modules_posix/library/helloworld.py future-im test/integration/targets/want_json_modules_posix/library/helloworld.py metaclass-boilerplate test/integration/targets/win_exec_wrapper/library/test_fail.ps1 pslint:PSCustomUseLiteralPath test/integration/targets/win_exec_wrapper/tasks/main.yml no-smart-quotes # We are explicitly testing smart quote support for env vars +test/integration/targets/win_fetch/tasks/main.yml no-smart-quotes # We are explictly testing smart quotes in the file name to fetch test/integration/targets/win_module_utils/library/legacy_only_new_way_win_line_ending.ps1 line-endings # Explicitly tests that we still work with Windows line endings test/integration/targets/win_module_utils/library/legacy_only_old_way_win_line_ending.ps1 line-endings # Explicitly tests that we still work with Windows line endings test/integration/targets/win_script/files/test_script.ps1 pslint:PSAvoidUsingWriteHost # Keep