ssh and psrp - Support more complex characters in fetch_file (#83753)

* ssh and psrp - Support more complex chars in fetch_file

Fixes the psrp and ssh (with piped) fetch function to work with paths
that contains glob like characters in the path. For Windows this was
needed when using paths that contain `[]` in the path. For ssh this was
a problem with FreeBSD when using the piped transfer method with similar
characters.

Also tidies up the psrp logic to not inject the paths and buffer size
in the script but pass it as an object through an argument/parameter.

* Fix sanity check

changelogs/fragments/fetch-filename.yml

@@ -0,0 +1,3 @@
+bugfixes:
+  - psrp - Fix bug when attempting to fetch a file path that contains special glob characters like ``[]``
+  - ssh - Fix bug when attempting to fetch a file path with characters that should be quoted when using the ``piped`` transfer method

lib/ansible/plugins/connection/psrp.py

@@ -632,39 +632,41 @@ end {
         buffer_size = max_b64_size - (max_b64_size % 1024)
 
         # setup the file stream with read only mode
-        setup_script = '''$ErrorActionPreference = "Stop"
+        setup_script = '''param([string]$Path)
-$path = '%s'
+$ErrorActionPreference = "Stop"
 
-if (Test-Path -Path $path -PathType Leaf) {
+if (Test-Path -LiteralPath $path -PathType Leaf) {
     $fs = New-Object -TypeName System.IO.FileStream -ArgumentList @(
         $path,
         [System.IO.FileMode]::Open,
         [System.IO.FileAccess]::Read,
         [System.IO.FileShare]::Read
     )
-    $buffer_size = %d
 } elseif (Test-Path -Path $path -PathType Container) {
     Write-Output -InputObject "[DIR]"
 } else {
     Write-Error -Message "$path does not exist"
     $host.SetShouldExit(1)
-}''' % (self._shell._escape(in_path), buffer_size)
+}'''
 
         # read the file stream at the offset and return the b64 string
-        read_script = '''$ErrorActionPreference = "Stop"
+        read_script = '''param([int64]$Offset, [int]$BufferSize)
-$fs.Seek(%d, [System.IO.SeekOrigin]::Begin) > $null
+$ErrorActionPreference = "Stop"
-$buffer = New-Object -TypeName byte[] -ArgumentList $buffer_size
+$fs.Seek($Offset, [System.IO.SeekOrigin]::Begin) > $null
-$bytes_read = $fs.Read($buffer, 0, $buffer_size)
+$buffer = New-Object -TypeName byte[] -ArgumentList $BufferSize
-
+$read = $fs.Read($buffer, 0, $buffer.Length)
-if ($bytes_read -gt 0) {
+
-    $bytes = $buffer[0..($bytes_read - 1)]
+if ($read -gt 0) {
-    Write-Output -InputObject ([System.Convert]::ToBase64String($bytes))
+    [System.Convert]::ToBase64String($buffer, 0, $read)
 }'''
 
         # need to run the setup script outside of the local scope so the
         # file stream stays active between fetch operations
-        rc, stdout, stderr = self._exec_psrp_script(setup_script,
+        rc, stdout, stderr = self._exec_psrp_script(
-                                                    use_local_scope=False)
+            setup_script,
+            use_local_scope=False,
+            arguments=[in_path],
+        )
         if rc != 0:
             raise AnsibleError("failed to setup file stream for fetch '%s': %s"
                                % (out_path, to_native(stderr)))
@@ -679,7 +681,10 @@ if ($bytes_read -gt 0) {
             while True:
                 display.vvvvv("PSRP FETCH %s to %s (offset=%d" %
                               (in_path, out_path, offset), host=self._psrp_host)
-                rc, stdout, stderr = self._exec_psrp_script(read_script % offset)
+                rc, stdout, stderr = self._exec_psrp_script(
+                    read_script,
+                    arguments=[offset, buffer_size],
+                )
                 if rc != 0:
                     raise AnsibleError("failed to transfer file to '%s': %s"
                                        % (out_path, to_native(stderr)))
@@ -813,7 +818,7 @@ if ($bytes_read -gt 0) {
         script: str,
         input_data: bytes | str | t.Iterable | None = None,
         use_local_scope: bool = True,
-        arguments: t.Iterable[str] | None = None,
+        arguments: t.Iterable[t.Any] | None = None,
     ) -> tuple[int, bytes, bytes]:
         # Check if there's a command on the current pipeline that still needs to be closed.
         if self._last_pipeline:

lib/ansible/plugins/connection/ssh.py

@@ -1250,7 +1250,7 @@ class Connection(ConnectionBase):
                 if sftp_action == 'get':
                     # we pass sudoable=False to disable pty allocation, which
                     # would end up mixing stdout/stderr and screwing with newlines
-                    (returncode, stdout, stderr) = self.exec_command('dd if=%s bs=%s' % (in_path, BUFSIZE), sudoable=False)
+                    (returncode, stdout, stderr) = self.exec_command('dd if=%s bs=%s' % (self._shell.quote(in_path), BUFSIZE), sudoable=False)
                     with open(to_bytes(out_path, errors='surrogate_or_strict'), 'wb+') as out_file:
                         out_file.write(stdout)
                 else:

test/integration/targets/connection/test_connection.yml

@@ -3,6 +3,25 @@
   serial: 1
   tasks:
 
+  # SSH with scp has troubles with using complex filenames that require quoting
+  # or escaping. The more complex filename scenario is skipped in this mode.
+  # The default of sftp has no problems with these filenames.
+  - name: check if ssh with the scp file transfer is being tested
+    set_fact:
+      skip_complex_filename: >-
+        {{
+            ansible_connection == "ssh" and
+            lookup("ansible.builtin.config",
+                "ssh_transfer_method",
+                plugin_name=ansible_connection,
+                plugin_type="connection",
+            ) == "scp"
+        }}
+
+  - name: set test filename
+    set_fact:
+      test_filename: 汉语-{{ skip_complex_filename | ternary("file", "['foo bar']") }}.txt
+
   ### raw with unicode arg and output
 
   - name: raw with unicode arg and output
@@ -17,20 +36,20 @@
   ### copy local file with unicode filename and content
 
   - name: create local file with unicode filename and content
-    local_action: lineinfile dest={{ local_tmp }}-汉语/汉语.txt create=true line=汉语
+    local_action: lineinfile dest={{ local_tmp }}-汉语/{{ test_filename }} create=true line=汉语
   - name: remove remote file with unicode filename and content
-    action: "{{ action_prefix }}file path={{ remote_tmp }}-汉语/汉语.txt state=absent"
+    action: "{{ action_prefix }}file path={{ remote_tmp }}-汉语/{{ test_filename }} state=absent"
   - name: create remote directory with unicode name
     action: "{{ action_prefix }}file path={{ remote_tmp }}-汉语 state=directory"
   - name: copy local file with unicode filename and content
-    action: "{{ action_prefix }}copy src={{ local_tmp }}-汉语/汉语.txt dest={{ remote_tmp }}-汉语/汉语.txt"
+    action: "{{ action_prefix }}copy src={{ local_tmp }}-汉语/{{ test_filename }} dest={{ remote_tmp }}-汉语/{{ test_filename }}"
 
   ### fetch remote file with unicode filename and content
 
   - name: remove local file with unicode filename and content
-    local_action: file path={{ local_tmp }}-汉语/汉语.txt state=absent
+    local_action: file path={{ local_tmp }}-汉语/{{ test_filename }} state=absent
   - name: fetch remote file with unicode filename and content
-    fetch: src={{ remote_tmp }}-汉语/汉语.txt dest={{ local_tmp }}-汉语/汉语.txt fail_on_missing=true validate_checksum=true flat=true
+    fetch: src={{ remote_tmp }}-汉语/{{ test_filename }} dest={{ local_tmp }}-汉语/{{ test_filename }} fail_on_missing=true validate_checksum=true flat=true
 
   ### remove local and remote temp files
 

test/integration/targets/win_fetch/aliases

@@ -1 +1,2 @@
 shippable/windows/group1
+shippable/windows/smoketest