From 36d2a8f899b85cea051cfd18de1cc29177f5e39b Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Mon, 27 Mar 2023 15:54:43 +0200
Subject: [PATCH] all ndk paths in config must be strings

The paths in the config must be strings because they are used in things
like env vars where they must be strings.  Plus lots of other places in the
code assumes they are strings.  This is the first step to defining the
border of where paths can be pathlib.Path() and where they must be strings.
---
 fdroidserver/common.py   |  1 +
 fdroidserver/metadata.py |  6 ++++--
 tests/common.TestCase    | 31 +++++++++++++++++++++++++++++++
 tests/metadata.TestCase  |  9 +++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/fdroidserver/common.py b/fdroidserver/common.py
index 59570b52..a6039dcb 100644
--- a/fdroidserver/common.py
+++ b/fdroidserver/common.py
@@ -116,6 +116,7 @@ env = None
 orig_path = None
 
 
+# All paths in the config must be strings, never pathlib.Path instances
 default_config = {
     'sdk_path': "$ANDROID_HOME",
     'ndk_paths': {},
diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py
index e5f82a69..fa895482 100644
--- a/fdroidserver/metadata.py
+++ b/fdroidserver/metadata.py
@@ -325,12 +325,14 @@ class Build(dict):
                 return f
         return 'ant'
 
-    def ndk_path(self):
-        """Return the path to the first configured NDK or an empty string."""
+    def ndk_path(self) -> str:
+        """Return the path string of the first configured NDK or an empty string."""
         ndk = self.ndk
         if isinstance(ndk, list):
             ndk = self.ndk[0]
         path = common.config['ndk_paths'].get(ndk)
+        if path and not isinstance(path, str):
+            raise TypeError('NDK path is not string')
         if path:
             return path
         for vsn, path in common.config['ndk_paths'].items():
diff --git a/tests/common.TestCase b/tests/common.TestCase
index 01f8a0e7..758b4881 100755
--- a/tests/common.TestCase
+++ b/tests/common.TestCase
@@ -2371,6 +2371,37 @@ class CommonTest(unittest.TestCase):
         )
         for f in config['java_paths'].values():
             self.assertTrue(f in java_paths)
+            self.assertTrue(isinstance(f, str))  # paths in config must be str
+
+    @mock.patch.dict(os.environ, clear=True)
+    def test_sdk_path_in_config_must_be_strings(self):
+        """All paths in config must be strings, and never pathlib.Path instances"""
+        os.environ['PATH'] = '/usr/bin:/usr/sbin'
+        config = {'sdk_path': Path('/opt/android-sdk')}
+        fdroidserver.common.fill_config_defaults(config)
+        build = fdroidserver.metadata.Build()
+        with self.assertRaises(TypeError):
+            fdroidserver.common.set_FDroidPopen_env(build)
+
+    @mock.patch.dict(os.environ, clear=True)
+    def test_ndk_paths_in_config_must_be_strings(self):
+        """All paths in config must be strings, and never pathlib.Path instances"""
+        fdroidserver.common.config = {
+            'ndk_paths': {'r21d': Path('/opt/android-sdk/ndk/r21d')}
+        }
+        build = fdroidserver.metadata.Build()
+        build.ndk = 'r21d'
+        os.environ['PATH'] = '/usr/bin:/usr/sbin'
+        with self.assertRaises(TypeError):
+            fdroidserver.common.set_FDroidPopen_env(build)
+
+    @mock.patch.dict(os.environ, clear=True)
+    def test_FDroidPopen_envs_paths_can_be_pathlib(self):
+        os.environ['PATH'] = '/usr/bin:/usr/sbin'
+        envs = {'PATHLIB': Path('/pathlib/path'), 'STRING': '/string/path'}
+        p = fdroidserver.common.FDroidPopen(['/bin/sh', '-c', 'export'], envs=envs)
+        self.assertIn('/string/path', p.output)
+        self.assertIn('/pathlib/path', p.output)
 
     def test_vcs_git_latesttags(self):
         tags = [
diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase
index 73650834..6b2a94b4 100755
--- a/tests/metadata.TestCase
+++ b/tests/metadata.TestCase
@@ -1141,6 +1141,15 @@ class MetadataTest(unittest.TestCase):
             build.ndk = ['r22b', 'r10e']
             self.assertEqual(r22b, build.ndk_path())
 
+    def test_build_ndk_path_only_accepts_str(self):
+        """Paths in the config must be strings, never pathlib.Path instances"""
+        config = {'ndk_paths': {'r24': Path('r24')}}
+        fdroidserver.common.config = config
+        build = fdroidserver.metadata.Build()
+        build.ndk = 'r24'
+        with self.assertRaises(TypeError):
+            build.ndk_path()
+
 
 if __name__ == "__main__":
     parser = optparse.OptionParser()