error if repo_icon is a path not just a filename

https://gitlab.com/fdroid/fdroidserver/-/merge_requests/1425#note_1730855272

fdroidserver/common.py

@@ -710,6 +710,14 @@ def read_config():
         if not config['archive_url'].endswith('/archive'):
             raise FDroidException(_('archive_url needs to end with /archive'))
 
+    repo_icon = config.get('repo_icon', '')
+    if '/' in repo_icon:
+        raise FDroidException(
+            _('repo_icon should be a filename ({name}), not a path!').format(
+                name=os.path.basename(repo_icon)
+            )
+        )
+
     confignames_to_delete = set()
     for configname, dictvalue in config.items():
         if configname == 'java_paths':

fdroidserver/index.py

@@ -102,11 +102,10 @@ def make(apps, apks, repodir, archive):
     if common.config['repo_maxage'] != 0:
         repodict['maxage'] = common.config['repo_maxage']
 
+    repo_icon = common.config.get('repo_icon', common.default_config['repo_icon'])
     if archive:
         repodict['name'] = common.config['archive_name']
-        repodict['icon'] = common.config.get(
-            'archive_icon', common.default_config['repo_icon']
-        )
+        repodict['icon'] = repo_icon
         repodict['description'] = common.config['archive_description']
         archive_url = common.config.get(
             'archive_url', common.config['repo_url'][:-4] + 'archive'
@@ -117,9 +116,7 @@ def make(apps, apks, repodir, archive):
         repo_section = os.path.basename(urllib.parse.urlparse(archive_url).path)
     else:
         repodict['name'] = common.config['repo_name']
-        repodict['icon'] = common.config.get(
-            'repo_icon', common.default_config['repo_icon']
-        )
+        repodict['icon'] = repo_icon
         repodict['address'] = common.config['repo_url']
         if 'repo_web_base_url' in common.config:
             repodict["webBaseUrl"] = common.config['repo_web_base_url']
@@ -1469,9 +1466,7 @@ def make_v0(apps, apks, repodir, repodict, requestsdict, signer_fingerprints):
 
 def copy_repo_icon(repodir):
     icon_dir = os.path.join(repodir, 'icons')
-    repo_icon = os.path.basename(
-        common.config.get('repo_icon', common.default_config['repo_icon'])
-    )
+    repo_icon = common.config.get('repo_icon', common.default_config['repo_icon'])
     icon_path = os.path.join(icon_dir, repo_icon)
     if not os.path.exists(icon_path):
         os.makedirs(icon_dir, exist_ok=True)

tests/test_common.py

@@ -2904,6 +2904,24 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase):
             fdroidserver.common.read_config()['serverwebroot'],
         )
 
+    def test_config_errors_ends_with_repo(self):
+        os.chdir(self.testdir)
+        fdroidserver.common.write_config_file("""repo_url: https://foo.com/fdroid""")
+        with self.assertRaises(FDroidException):
+            fdroidserver.common.read_config()
+
+    def test_config_errors_ends_with_archive(self):
+        os.chdir(self.testdir)
+        fdroidserver.common.write_config_file("""archive_url: https://foo.com/fdroid/repo""")
+        with self.assertRaises(FDroidException):
+            fdroidserver.common.read_config()
+
+    def test_config_errors_repo_icon(self):
+        os.chdir(self.testdir)
+        fdroidserver.common.write_config_file("""repo_icon: /path/to/icon.png""")
+        with self.assertRaises(FDroidException):
+            fdroidserver.common.read_config()
+
     @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
     def test_config_serverwebroot_list_of_dicts_env(self):
         os.chdir(self.testdir)

tests/test_index.py

@@ -453,6 +453,21 @@ class IndexTest(unittest.TestCase):
         self.assertTrue(os.path.isdir(repo_icons_dir))
         self.assertEqual(test_value, copied_path.read_text())
 
+    def test_copy_repo_icon_skipped_if_exists(self):
+        os.chdir(self.testdir)
+        test_value = 'test'
+        default_repo_icon = common.default_config['repo_icon']
+        repo_icon = Path('repo/icons') / default_repo_icon
+        repo_icon.parent.mkdir(parents=True)
+        repo_icon.write_text(test_value)
+        archive_icon = Path('archive/icons') / default_repo_icon
+        archive_icon.parent.mkdir(parents=True)
+        archive_icon.write_text(test_value)
+        Path(default_repo_icon).write_text('this should not be copied')
+        index.copy_repo_icon('repo')
+        self.assertEqual(test_value, repo_icon.read_text())
+        self.assertEqual(test_value, archive_icon.read_text())
+
     def test_make_v0(self):
         os.chdir(self.testdir)
         os.mkdir('metadata')