powershell - remove quoting logic (#85781)

Attempts to simplify the PowerShell code and special edge cases that
removes quotes from a value like a src or destination path on Windows
hosts. This should not be needed as paths should not be quoted when it
comes to this section of the code.

ci_complete
pull/85813/head
Jordan Borean 3 months ago committed by GitHub
parent a345a404e0
commit f29d432c69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,4 @@
breaking_changes:
- >-
powershell - Removed code that tried to remote quotes from paths when performing Windows operations like copying
and fetching file. This should not affect normal playbooks unless a value is quoted too many times.

@ -287,14 +287,6 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin):
elif leaf_module_name == 'async_status' and collection_name in rewrite_collection_names: elif leaf_module_name == 'async_status' and collection_name in rewrite_collection_names:
module_name = '%s.%s' % (win_collection, leaf_module_name) module_name = '%s.%s' % (win_collection, leaf_module_name)
# TODO: move this tweak down to the modules, not extensible here
# Remove extra quotes surrounding path parameters before sending to module.
if leaf_module_name in ['win_stat', 'win_file', 'win_copy', 'slurp'] and module_args and \
hasattr(self._connection._shell, '_unquote'):
for key in ('src', 'dest', 'path'):
if key in module_args:
module_args[key] = self._connection._shell._unquote(module_args[key])
result = self._shared_loader_obj.module_loader.find_plugin_with_context(module_name, mod_type, collection_list=self._task.collections) result = self._shared_loader_obj.module_loader.find_plugin_with_context(module_name, mod_type, collection_list=self._task.collections)
if not result.resolved: if not result.resolved:

@ -130,7 +130,6 @@ class ActionModule(ActionBase):
# calculate the destination name # calculate the destination name
if os.path.sep not in self._connection._shell.join_path('a', ''): if os.path.sep not in self._connection._shell.join_path('a', ''):
source = self._connection._shell._unquote(source)
source_local = source.replace('\\', '/') source_local = source.replace('\\', '/')
else: else:
source_local = source source_local = source

@ -489,7 +489,6 @@ class Connection(ConnectionBase):
def put_file(self, in_path: str, out_path: str) -> None: def put_file(self, in_path: str, out_path: str) -> None:
super(Connection, self).put_file(in_path, out_path) super(Connection, self).put_file(in_path, out_path)
out_path = self._shell._unquote(out_path)
display.vvv("PUT %s TO %s" % (in_path, out_path), host=self._psrp_host) display.vvv("PUT %s TO %s" % (in_path, out_path), host=self._psrp_host)
script, in_data = _bootstrap_powershell_script('psrp_put_file.ps1', { script, in_data = _bootstrap_powershell_script('psrp_put_file.ps1', {
@ -549,7 +548,6 @@ class Connection(ConnectionBase):
display.vvv("FETCH %s TO %s" % (in_path, out_path), display.vvv("FETCH %s TO %s" % (in_path, out_path),
host=self._psrp_host) host=self._psrp_host)
in_path = self._shell._unquote(in_path)
out_path = out_path.replace('\\', '/') out_path = out_path.replace('\\', '/')
b_out_path = to_bytes(out_path, errors='surrogate_or_strict') b_out_path = to_bytes(out_path, errors='surrogate_or_strict')

@ -768,7 +768,6 @@ class Connection(ConnectionBase):
def put_file(self, in_path: str, out_path: str) -> None: def put_file(self, in_path: str, out_path: str) -> None:
super(Connection, self).put_file(in_path, out_path) super(Connection, self).put_file(in_path, out_path)
out_path = self._shell._unquote(out_path)
display.vvv('PUT "%s" TO "%s"' % (in_path, out_path), host=self._winrm_host) display.vvv('PUT "%s" TO "%s"' % (in_path, out_path), host=self._winrm_host)
if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')):
raise AnsibleFileNotFound('file or module does not exist: "%s"' % to_native(in_path)) raise AnsibleFileNotFound('file or module does not exist: "%s"' % to_native(in_path))
@ -806,7 +805,6 @@ class Connection(ConnectionBase):
def fetch_file(self, in_path: str, out_path: str) -> None: def fetch_file(self, in_path: str, out_path: str) -> None:
super(Connection, self).fetch_file(in_path, out_path) super(Connection, self).fetch_file(in_path, out_path)
in_path = self._shell._unquote(in_path)
out_path = out_path.replace('\\', '/') out_path = out_path.replace('\\', '/')
# consistent with other connection plugins, we assume the caller has created the target dir # consistent with other connection plugins, we assume the caller has created the target dir
display.vvv('FETCH "%s" TO "%s"' % (in_path, out_path), host=self._winrm_host) display.vvv('FETCH "%s" TO "%s"' % (in_path, out_path), host=self._winrm_host)

@ -192,7 +192,7 @@ class ShellModule(ShellBase):
def join_path(self, *args): def join_path(self, *args):
# use normpath() to remove doubled slashed and convert forward to backslashes # use normpath() to remove doubled slashed and convert forward to backslashes
parts = [ntpath.normpath(self._unquote(arg)) for arg in args] parts = [ntpath.normpath(arg) for arg in args]
# Because ntpath.join treats any component that begins with a backslash as an absolute path, # Because ntpath.join treats any component that begins with a backslash as an absolute path,
# we have to strip slashes from at least the beginning, otherwise join will ignore all previous # we have to strip slashes from at least the beginning, otherwise join will ignore all previous
@ -210,7 +210,6 @@ class ShellModule(ShellBase):
def path_has_trailing_slash(self, path): def path_has_trailing_slash(self, path):
# Allow Windows paths to be specified using either slash. # Allow Windows paths to be specified using either slash.
path = self._unquote(path)
return path.endswith('/') or path.endswith('\\') return path.endswith('/') or path.endswith('\\')
def chmod(self, paths, mode): def chmod(self, paths, mode):
@ -223,11 +222,11 @@ class ShellModule(ShellBase):
raise NotImplementedError('set_user_facl is not implemented for Powershell') raise NotImplementedError('set_user_facl is not implemented for Powershell')
def remove(self, path, recurse=False): def remove(self, path, recurse=False):
path = self._escape(self._unquote(path)) quoted_path = self._escape(path)
if recurse: if recurse:
return self._encode_script("""Remove-Item '%s' -Force -Recurse;""" % path) return self._encode_script("""Remove-Item '%s' -Force -Recurse;""" % quoted_path)
else: else:
return self._encode_script("""Remove-Item '%s' -Force;""" % path) return self._encode_script("""Remove-Item '%s' -Force;""" % quoted_path)
def mkdtemp( def mkdtemp(
self, self,
@ -240,7 +239,6 @@ class ShellModule(ShellBase):
# compatibility in case other action plugins outside Ansible calls this. # compatibility in case other action plugins outside Ansible calls this.
if not basefile: if not basefile:
basefile = self.__class__._generate_temp_dir_name() basefile = self.__class__._generate_temp_dir_name()
basefile = self._escape(self._unquote(basefile))
basetmpdir = self._escape(tmpdir if tmpdir else self.get_option('remote_tmp')) basetmpdir = self._escape(tmpdir if tmpdir else self.get_option('remote_tmp'))
script = f""" script = f"""
@ -263,7 +261,6 @@ class ShellModule(ShellBase):
if not basefile: if not basefile:
basefile = self.__class__._generate_temp_dir_name() basefile = self.__class__._generate_temp_dir_name()
basefile = self._unquote(basefile)
basetmpdir = tmpdir if tmpdir else self.get_option('remote_tmp') basetmpdir = tmpdir if tmpdir else self.get_option('remote_tmp')
script, stdin = _bootstrap_powershell_script("powershell_mkdtemp.ps1", { script, stdin = _bootstrap_powershell_script("powershell_mkdtemp.ps1", {
@ -283,7 +280,6 @@ class ShellModule(ShellBase):
) -> str: ) -> str:
# This is not called in Ansible anymore but it is kept for backwards # This is not called in Ansible anymore but it is kept for backwards
# compatibility in case other actions plugins outside Ansible called this. # compatibility in case other actions plugins outside Ansible called this.
user_home_path = self._unquote(user_home_path)
if user_home_path == '~': if user_home_path == '~':
script = 'Write-Output (Get-Location).Path' script = 'Write-Output (Get-Location).Path'
elif user_home_path.startswith('~\\'): elif user_home_path.startswith('~\\'):
@ -297,7 +293,6 @@ class ShellModule(ShellBase):
user_home_path: str, user_home_path: str,
username: str = '', username: str = '',
) -> _ShellCommand: ) -> _ShellCommand:
user_home_path = self._unquote(user_home_path)
script, stdin = _bootstrap_powershell_script("powershell_expand_user.ps1", { script, stdin = _bootstrap_powershell_script("powershell_expand_user.ps1", {
'Path': user_home_path, 'Path': user_home_path,
}) })
@ -308,7 +303,7 @@ class ShellModule(ShellBase):
) )
def exists(self, path): def exists(self, path):
path = self._escape(self._unquote(path)) path = self._escape(path)
script = """ script = """
If (Test-Path '%s') If (Test-Path '%s')
{ {
@ -329,7 +324,7 @@ class ShellModule(ShellBase):
version="2.23", version="2.23",
help_text="Use `ActionBase._execute_remote_stat()` instead.", help_text="Use `ActionBase._execute_remote_stat()` instead.",
) )
path = self._escape(self._unquote(path)) path = self._escape(path)
script = """ script = """
If (Test-Path -PathType Leaf '%(path)s') If (Test-Path -PathType Leaf '%(path)s')
{ {
@ -364,7 +359,7 @@ class ShellModule(ShellBase):
if arg_path: if arg_path:
# Running a module without the exec_wrapper and with an argument # Running a module without the exec_wrapper and with an argument
# file. # file.
script_path = self._unquote(cmd_parts[0]) script_path = cmd_parts[0]
if not script_path.lower().endswith('.ps1'): if not script_path.lower().endswith('.ps1'):
script_path += '.ps1' script_path += '.ps1'
@ -387,7 +382,6 @@ class ShellModule(ShellBase):
cmd_parts.insert(0, shebang[2:]) cmd_parts.insert(0, shebang[2:])
elif not shebang: elif not shebang:
# The module is assumed to be a binary # The module is assumed to be a binary
cmd_parts[0] = self._unquote(cmd_parts[0])
cmd_parts.append(arg_path) cmd_parts.append(arg_path)
script = """ script = """
Try Try
@ -431,17 +425,6 @@ class ShellModule(ShellBase):
super().wrap_for_exec(cmd) super().wrap_for_exec(cmd)
return '& %s; exit $LASTEXITCODE' % cmd return '& %s; exit $LASTEXITCODE' % cmd
def _unquote(self, value):
"""Remove any matching quotes that wrap the given value."""
value = to_text(value or '')
m = re.match(r'^\s*?\'(.*?)\'\s*?$', value)
if m:
return m.group(1)
m = re.match(r'^\s*?"(.*?)"\s*?$', value)
if m:
return m.group(1)
return value
def _escape(self, value): def _escape(self, value):
"""Return value escaped for use in PowerShell single quotes.""" """Return value escaped for use in PowerShell single quotes."""
# There are 5 chars that need to be escaped in a single quote. # There are 5 chars that need to be escaped in a single quote.

Loading…
Cancel
Save