diff --git a/fdroidserver/common.py b/fdroidserver/common.py index df9ffe25..48434581 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -39,6 +39,7 @@ import socket import base64 import urllib.parse import urllib.request +import yaml import zipfile import tempfile import json @@ -284,14 +285,18 @@ def regsub_file(pattern, repl, path): f.write(text) -def read_config(opts, config_file='config.py'): +def read_config(opts): """Read the repository config The config is read from config_file, which is in the current directory when any of the repo management commands are used. If - there is a local metadata file in the git repo, then config.py is + there is a local metadata file in the git repo, then the config is not required, just use defaults. + config.yml is the preferred form because no code is executed when + reading it. config.py is deprecated and supported for backwards + compatibility. + """ global config, options @@ -301,12 +306,23 @@ def read_config(opts, config_file='config.py'): options = opts config = {} + config_file = 'config.yml' + old_config_file = 'config.py' - if os.path.isfile(config_file): + if os.path.exists(config_file) and os.path.exists(old_config_file): + logging.error(_("""Conflicting config files! Using {newfile}, ignoring {oldfile}!""") + .format(oldfile=old_config_file, newfile=config_file)) + + if os.path.exists(config_file): logging.debug(_("Reading '{config_file}'").format(config_file=config_file)) - with io.open(config_file, "rb") as f: - code = compile(f.read(), config_file, 'exec') - exec(code, None, config) # nosec TODO switch to YAML file + with open(config_file) as fp: + config = yaml.safe_load(fp) + elif os.path.exists(old_config_file): + logging.warning(_("""{oldfile} is deprecated, use {newfile}""") + .format(oldfile=old_config_file, newfile=config_file)) + with io.open(old_config_file, "rb") as fp: + code = compile(fp.read(), old_config_file, 'exec') + exec(code, None, config) # nosec TODO automatically migrate else: logging.warning(_("No 'config.py' found, using defaults.")) @@ -329,10 +345,14 @@ def read_config(opts, config_file='config.py'): '-providerArg', 'opensc-fdroid.cfg'] if any(k in config for k in ["keystore", "keystorepass", "keypass"]): - st = os.stat(config_file) + if os.path.exists(config_file): + f = config_file + elif os.path.exists(old_config_file): + f = old_config_file + st = os.stat(f) if st.st_mode & stat.S_IRWXG or st.st_mode & stat.S_IRWXO: logging.warning(_("unsafe permissions on '{config_file}' (should be 0600)!") - .format(config_file=config_file)) + .format(config_file=f)) fill_config_defaults(config) diff --git a/tests/common.TestCase b/tests/common.TestCase index ff7befb1..b49ba10e 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -47,6 +47,7 @@ class CommonTest(unittest.TestCase): if not os.path.exists(self.tmpdir): os.makedirs(self.tmpdir) os.chdir(self.basedir) + fdroidserver.common.config = None def test_parse_human_readable_size(self): for k, v in ((9827, 9827), (123.456, 123), ('123b', 123), ('1.2', 1), @@ -1409,6 +1410,70 @@ class CommonTest(unittest.TestCase): self.assertIsNotNone(result) self.assertNotEqual(result, '') + def test_with_no_config(self): + """It should set defaults if no config file is found""" + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + self.assertFalse(os.path.exists('config.yml')) + self.assertFalse(os.path.exists('config.py')) + config = fdroidserver.common.read_config(fdroidserver.common.options) + self.assertEqual(None, config.get('apksigner')) + self.assertIsNotNone(config.get('char_limits')) + + def test_with_config_yml(self): + """Make sure it is possible to use config.yml alone.""" + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + with open('config.yml', 'w') as fp: + fp.write('apksigner: yml') + self.assertTrue(os.path.exists('config.yml')) + self.assertFalse(os.path.exists('config.py')) + config = fdroidserver.common.read_config(fdroidserver.common.options) + self.assertEqual('yml', config.get('apksigner')) + + def test_with_config_py(self): + """Make sure it is still possible to use config.py alone.""" + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + with open('config.py', 'w') as fp: + fp.write('apksigner = "py"') + self.assertFalse(os.path.exists('config.yml')) + self.assertTrue(os.path.exists('config.py')) + config = fdroidserver.common.read_config(fdroidserver.common.options) + self.assertEqual("py", config.get('apksigner')) + + def test_config_perm_warning(self): + """Exercise the code path that issues a warning about unsafe permissions.""" + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + with open('config.yml', 'w') as fp: + fp.write('keystore: foo.jks') + self.assertTrue(os.path.exists(fp.name)) + os.chmod(fp.name, 0o666) + fdroidserver.common.read_config(fdroidserver.common.options) + os.remove(fp.name) + fdroidserver.common.config = None + + with open('config.py', 'w') as fp: + fp.write('keystore = "foo.jks"') + self.assertTrue(os.path.exists(fp.name)) + os.chmod(fp.name, 0o666) + fdroidserver.common.read_config(fdroidserver.common.options) + + def test_with_both_config_yml_py(self): + """If config.yml and config.py are present, config.py should be ignored.""" + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + with open('config.yml', 'w') as fp: + fp.write('apksigner: yml') + with open('config.py', 'w') as fp: + fp.write('apksigner = "py"') + self.assertTrue(os.path.exists('config.yml')) + self.assertTrue(os.path.exists('config.py')) + config = fdroidserver.common.read_config(fdroidserver.common.options) + self.assertEqual('yml', config.get('apksigner')) + + if __name__ == "__main__": os.chdir(os.path.dirname(__file__))