From fe3d929f672f9c8c99bdf28d18325c1ec6cf618d Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 12 Jun 2024 09:38:15 +0200
Subject: [PATCH] deploy: lists for command lines to handle escaping

fdroidserver uses lists of strings to handle the escaping command line
arguments, this converts the rclone code to that pattern.
---
 fdroidserver/deploy.py | 25 +++++--------------------
 tests/deploy.TestCase  | 25 +++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/fdroidserver/deploy.py b/fdroidserver/deploy.py
index 0d466245..da0e8d8d 100644
--- a/fdroidserver/deploy.py
+++ b/fdroidserver/deploy.py
@@ -30,7 +30,6 @@ from git import Repo
 import yaml
 from argparse import ArgumentParser
 import logging
-from shlex import split
 import pathlib
 import shutil
 import git
@@ -272,7 +271,7 @@ def update_remote_storage_with_rclone(
             logging.info('Custom configuration not found.')
             logging.info(
                 'Using default configuration at {}'.format(
-                    subprocess.check_output(split("rclone config file")).decode("utf-8")
+                    subprocess.check_output(['rclone', 'config', 'file'], text=True)
                 )
             )
             configfilename = None
@@ -281,7 +280,7 @@ def update_remote_storage_with_rclone(
         logging.info('Custom configuration not found.')
         logging.info(
             'Using default configuration at {}'.format(
-                subprocess.check_output(split("rclone config file")).decode("utf-8")
+                subprocess.check_output(['rclone', 'config', 'file'], text=True)
             )
         )
         configfilename = None
@@ -306,18 +305,8 @@ def update_remote_storage_with_rclone(
 
     for source in sources:
         for remote_config in rclone_config:
-            rclone_sync_command = (
-                'rclone sync '
-                + source
-                + ' '
-                + config['rclone_config']
-                + ':'
-                + config['awsbucket']
-                + '/'
-                + upload_dir
-            )
-
-            rclone_sync_command = split(rclone_sync_command)
+            complete_remote_path = f'{remote_config}:{config["awsbucket"]}/{upload_dir}'
+            rclone_sync_command = ['rclone', 'sync', source, complete_remote_path]
 
             if verbose:
                 rclone_sync_command += ['--verbose']
@@ -325,11 +314,7 @@ def update_remote_storage_with_rclone(
                 rclone_sync_command += ['--quiet']
 
             if configfilename:
-                rclone_sync_command += split('--config=' + configfilename)
-
-            complete_remote_path = (
-                config['rclone_config'] + ':' + config['awsbucket'] + '/' + upload_dir
-            )
+                rclone_sync_command += ['--config=' + configfilename]
 
             logging.debug(
                 "rclone sync all files in " + source + ' to ' + complete_remote_path
diff --git a/tests/deploy.TestCase b/tests/deploy.TestCase
index 3d45625d..649f8d58 100755
--- a/tests/deploy.TestCase
+++ b/tests/deploy.TestCase
@@ -194,6 +194,31 @@ class DeployTest(unittest.TestCase):
         self.assertFalse(dest_apk.is_file())
         self.assertTrue(dest_index.is_file())
 
+    @mock.patch('subprocess.call')
+    @mock.patch('subprocess.check_output', lambda cmd, text: '/path/to/rclone.conf')
+    def test_update_remote_storage_with_rclone_mock(self, mock_call):
+        def _mock_subprocess_call(cmd):
+            self.assertEqual(
+                cmd,
+                [
+                    'rclone',
+                    'sync',
+                    'repo',
+                    'test_local_config:test_bucket_folder/fdroid/repo',
+                ],
+            )
+            return 0
+
+        mock_call.side_effect = _mock_subprocess_call
+
+        fdroidserver.deploy.config = {
+            'awsbucket': 'test_bucket_folder',
+            'rclone': True,
+            'rclone_config': 'test_local_config',
+        }
+        fdroidserver.deploy.update_remote_storage_with_rclone('repo')
+        mock_call.assert_called_once()
+
     def test_update_serverwebroot(self):
         """rsync works with file paths, so this test uses paths for the URLs"""
         os.chdir(self.testdir)