From 4cd96d4a9fa18a5446219e50bcb2252556a2e9f9 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 02:37:05 +0200 Subject: [PATCH 1/9] use yaml safeloader in tests Try to use CSafeLoader when possible because its significantly faster. Use the normal Safeloader otherwise. (This mirrors the non-test code behaviour) --- tests/metadata.TestCase | 9 +++++++-- tests/update.TestCase | 15 ++++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 8ab502c4..418d3afe 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -18,6 +18,11 @@ import textwrap from collections import OrderedDict from unittest import mock +try: + from yaml import CSafeLoader as SafeLoader +except ImportError: + from yaml import SafeLoader + localmodule = os.path.realpath( os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..')) print('localmodule: ' + localmodule) @@ -103,7 +108,7 @@ class MetadataTest(unittest.TestCase): def test_valid_funding_yml_regex(self): """Check the regex can find all the cases""" with open(os.path.join(self.basedir, 'funding-usernames.yaml')) as fp: - data = yaml.safe_load(fp) + data = yaml.load(fp, Loader=SafeLoader) for k, entries in data.items(): for entry in entries: @@ -135,7 +140,7 @@ class MetadataTest(unittest.TestCase): frommeta = dict(apps[appid]) self.assertTrue(appid in apps) with open(savepath, 'r') as f: - frompickle = yaml.load(f) + frompickle = yaml.load(f, Loader=SafeLoader) self.assertEqual(frommeta, frompickle) # comment above assert and uncomment below to update test # files when new metadata fields are added diff --git a/tests/update.TestCase b/tests/update.TestCase index eee01734..be9d2799 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -21,6 +21,11 @@ from binascii import unhexlify from distutils.version import LooseVersion from testcommon import TmpCwd +try: + from yaml import CSafeLoader as SafeLoader +except ImportError: + from yaml import SafeLoader + localmodule = os.path.realpath( os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..')) print('localmodule: ' + localmodule) @@ -634,7 +639,7 @@ class UpdateTest(unittest.TestCase): # yaml.dump(apk, f, default_flow_style=False) with open(savepath, 'r') as f: - from_yaml = yaml.load(f) + from_yaml = yaml.load(f, Loader=SafeLoader) self.maxDiff = None self.assertEqual(apk, from_yaml) @@ -844,7 +849,7 @@ class UpdateTest(unittest.TestCase): self.assertEqual('Internet', app['Categories'][0]) break with open(testfile) as fp: - data = yaml.load(fp) + data = yaml.load(fp, Loader=SafeLoader) self.assertEqual('urzip', data['Name']) self.assertEqual('urzip', data['Summary']) @@ -943,7 +948,7 @@ class UpdateTest(unittest.TestCase): ''')) fdroidserver.update.create_metadata_from_template(apk) with open(os.path.join('metadata', 'rocks.janicerand.yml')) as f: - metadata_content = yaml.load(f) + metadata_content = yaml.load(f, Loader=SafeLoader) self.maxDiff = None self.assertDictEqual(metadata_content, {'ArchivePolicy': '', @@ -1050,7 +1055,7 @@ class UpdateTest(unittest.TestCase): apps = {app.id: app} with open(os.path.join('build', app.id, 'FUNDING.yml'), 'w') as fp: fp.write(line) - data = yaml.load(line) + data = yaml.load(line, Loader=SafeLoader) fdroidserver.update.insert_funding_yml_donation_links(apps) if 'liberapay' in data: self.assertEqual(data['liberapay'], app.get('Liberapay')) @@ -1080,7 +1085,7 @@ class UpdateTest(unittest.TestCase): def test_sanitize_funding_yml(self): with open(os.path.join(self.basedir, 'funding-usernames.yaml')) as fp: - data = yaml.safe_load(fp) + data = yaml.load(fp, Loader=SafeLoader) for k, entries in data.items(): for entry in entries: if k in 'custom': From 9bf0758f1939419cf1b801ecc633eef67660c004 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 02:37:30 +0200 Subject: [PATCH 2/9] make update.Testcase tests work standalone --- tests/update.TestCase | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/tests/update.TestCase b/tests/update.TestCase index be9d2799..8f9da35e 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -212,6 +212,10 @@ class UpdateTest(unittest.TestCase): shutil.copytree(os.path.join(self.basedir, 'triple-t-2'), tmptestsdir) os.chdir(tmptestsdir) + config = dict() + fdroidserver.common.fill_config_defaults(config) + fdroidserver.common.config = config + fdroidserver.update.config = config fdroidserver.update.options = fdroidserver.common.options apps = fdroidserver.metadata.read_metadata(xref=True) @@ -344,9 +348,10 @@ class UpdateTest(unittest.TestCase): def testScanApksAndObbs(self): os.chdir(os.path.join(localmodule, 'tests')) - if os.path.basename(os.getcwd()) != 'tests': - raise Exception('This test must be run in the "tests/" subdir') - + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + shutil.copytree(os.path.join(self.basedir, 'repo'), 'repo') + shutil.copytree(os.path.join(self.basedir, 'metadata'), 'metadata') config = dict() fdroidserver.common.fill_config_defaults(config) config['ndk_paths'] = dict() @@ -401,9 +406,9 @@ class UpdateTest(unittest.TestCase): def test_apkcache_json(self): """test the migration from pickle to json""" os.chdir(os.path.join(localmodule, 'tests')) - if os.path.basename(os.getcwd()) != 'tests': - raise Exception('This test must be run in the "tests/" subdir') - + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + shutil.copytree(os.path.join(self.basedir,'repo'), 'repo') config = dict() fdroidserver.common.fill_config_defaults(config) config['ndk_paths'] = dict() @@ -442,9 +447,6 @@ class UpdateTest(unittest.TestCase): fdroidserver.common.config = config fdroidserver.update.config = config os.chdir(os.path.join(localmodule, 'tests')) - if os.path.basename(os.getcwd()) != 'tests': - raise Exception('This test must be run in the "tests/" subdir') - try: config['aapt'] = fdroidserver.common.find_sdk_tools_cmd('aapt') except fdroidserver.exception.FDroidException: @@ -748,6 +750,7 @@ class UpdateTest(unittest.TestCase): fdroidserver.common.fill_config_defaults(config) fdroidserver.common.config = config fdroidserver.update.config = config + fdroidserver.update.options = fdroidserver.common.options fdroidserver.update.options.delete_unknown = False knownapks = fdroidserver.common.KnownApks() @@ -761,9 +764,10 @@ class UpdateTest(unittest.TestCase): def test_translate_per_build_anti_features(self): os.chdir(os.path.join(localmodule, 'tests')) - if os.path.basename(os.getcwd()) != 'tests': - raise Exception('This test must be run in the "tests/" subdir') - + testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) + os.chdir(testdir) + shutil.copytree(os.path.join(self.basedir, 'repo'), 'repo') + shutil.copytree(os.path.join(self.basedir, 'metadata'), 'metadata') config = dict() fdroidserver.common.fill_config_defaults(config) config['ndk_paths'] = dict() From e613b650984f1625d632828dff0d73bc1ea3230a Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 15:51:47 +0200 Subject: [PATCH 3/9] we need FullLoader for one test, we are dumping custom objects --- tests/update.TestCase | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/update.TestCase b/tests/update.TestCase index 8f9da35e..0f539296 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -26,6 +26,11 @@ try: except ImportError: from yaml import SafeLoader +try: + from yaml import CFullLoader as FullLoader +except ImportError: + from yaml import FullLoader + localmodule = os.path.realpath( os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..')) print('localmodule: ' + localmodule) @@ -408,7 +413,7 @@ class UpdateTest(unittest.TestCase): os.chdir(os.path.join(localmodule, 'tests')) testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir) os.chdir(testdir) - shutil.copytree(os.path.join(self.basedir,'repo'), 'repo') + shutil.copytree(os.path.join(self.basedir, 'repo'), 'repo') config = dict() fdroidserver.common.fill_config_defaults(config) config['ndk_paths'] = dict() @@ -594,13 +599,10 @@ class UpdateTest(unittest.TestCase): fdroidserver.common.fill_config_defaults(config) fdroidserver.update.config = config os.chdir(os.path.join(localmodule, 'tests')) - if os.path.basename(os.getcwd()) != 'tests': - raise Exception('This test must be run in the "tests/" subdir') config['ndk_paths'] = dict() fdroidserver.common.config = config fdroidserver.update.config = config - fdroidserver.update.options = type('', (), {})() fdroidserver.update.options.clean = True fdroidserver.update.options.rename_apks = False @@ -641,7 +643,7 @@ class UpdateTest(unittest.TestCase): # yaml.dump(apk, f, default_flow_style=False) with open(savepath, 'r') as f: - from_yaml = yaml.load(f, Loader=SafeLoader) + from_yaml = yaml.load(f, Loader=FullLoader) self.maxDiff = None self.assertEqual(apk, from_yaml) From 709f4c9b18a49f53b12f0250f120d42c9182fd1e Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 15:56:42 +0200 Subject: [PATCH 4/9] pickle -> yaml rename --- tests/metadata.TestCase | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 418d3afe..c48bdba2 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -140,8 +140,8 @@ class MetadataTest(unittest.TestCase): frommeta = dict(apps[appid]) self.assertTrue(appid in apps) with open(savepath, 'r') as f: - frompickle = yaml.load(f, Loader=SafeLoader) - self.assertEqual(frommeta, frompickle) + from_yaml = yaml.load(f, Loader=SafeLoader) + self.assertEqual(frommeta, from_yaml) # comment above assert and uncomment below to update test # files when new metadata fields are added # with open(savepath, 'w') as f: From 89f63b3e1c4d70afed5b5ec53390a9fe59873d92 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 17:10:43 +0200 Subject: [PATCH 5/9] tests: use yaml.Loader on older yaml versions --- tests/update.TestCase | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/update.TestCase b/tests/update.TestCase index 0f539296..25efe369 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -29,7 +29,13 @@ except ImportError: try: from yaml import CFullLoader as FullLoader except ImportError: - from yaml import FullLoader + try: + # FullLoader is available from PyYaml 5.1+, as we don't load user + # controlled data here, it's okay to fall back the unsafe older + # Loader + from yaml import FullLoader + except ImportError: + from yaml import Loader as FullLoader localmodule = os.path.realpath( os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..')) From 2367461465d40647d6ffb24abcb33d1a494446bd Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 17:12:16 +0200 Subject: [PATCH 6/9] tests: debian: apksigner is required for the tests to run now We need to use a shell wrapper for apksigner though because docker and binfmt don't play well together --- .gitlab-ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0de8ffaf..ca0dca28 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -66,12 +66,17 @@ debian_testing: - apt-get install aapt androguard + apksigner fdroidserver git gnupg python3-defusedxml python3-setuptools zipalign + # Debian has apksigner depend on binfmt support which isn't very docker friendly + # We create a shell wrapper instead + - echo -e '#!/bin/sh\njava -jar /usr/lib/android-sdk/build-tools/debian/apksigner.jar "$@"' > /usr/local/bin/apksigner + - chmod +x /usr/local/bin/apksigner - python3 -c 'import fdroidserver' - python3 -c 'import androguard' - cd tests From f6b7572b10c7aae43e048e0b6fac8765cb3ee4b1 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 17:14:15 +0200 Subject: [PATCH 7/9] fix fedora test minimum build tools version is determined by apksigner now. --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ca0dca28..14807970 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -202,7 +202,7 @@ fedora_latest: - wget --no-verbose -O tools.zip https://dl.google.com/android/repository/tools_r25.2.5-linux.zip - unzip -q tools.zip - rm tools.zip - - export AAPT_VERSION=`sed -n "s,^MINIMUM_AAPT_VERSION\s*=\s*['\"]\(.*\)[['\"],\1,p" fdroidserver/common.py` + - export BUILD_TOOLS_VERSION=`sed -n "s,^MINIMUM_APKSIGNER_BUILD_TOOLS_VERSION\s*=\s*['\"]\(.*\)[['\"],\1,p" fdroidserver/common.py` - export JAVA_HOME=/etc/alternatives/jre - export ANDROID_HOME=`pwd`/android-sdk - mkdir $ANDROID_HOME @@ -214,7 +214,7 @@ fedora_latest: - mkdir ~/.android - touch ~/.android/repositories.cfg - echo y | $ANDROID_HOME/tools/bin/sdkmanager "platform-tools" - - echo y | $ANDROID_HOME/tools/bin/sdkmanager "build-tools;$AAPT_VERSION" + - echo y | $ANDROID_HOME/tools/bin/sdkmanager "build-tools;$BUILD_TOOLS_VERSION" - chown -R testuser . - cd tests - su testuser --login --command From 7eb32feaa5876c9847ef3e87a8655e1cd387235a Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 17:26:50 +0200 Subject: [PATCH 8/9] skip new signing test when we can't find apksigner Also add some error handling to the find_apksigner() method. --- fdroidserver/common.py | 4 ++++ tests/common.TestCase | 2 ++ 2 files changed, 6 insertions(+) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index e6b10305..586867a6 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -429,7 +429,11 @@ def find_apksigner(): if set_command_in_config('apksigner'): return config['apksigner'] build_tools_path = os.path.join(config['sdk_path'], 'build-tools') + if not os.path.isdir(build_tools_path): + return None for f in sorted(os.listdir(build_tools_path), reverse=True): + if not os.path.isdir(os.path.join(build_tools_path, f)): + continue if LooseVersion(f) < LooseVersion(MINIMUM_AAPT_BUILD_TOOLS_VERSION): return None if os.path.exists(os.path.join(build_tools_path, f, 'apksigner')): diff --git a/tests/common.TestCase b/tests/common.TestCase index eb67836c..98815d5b 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -675,6 +675,8 @@ class CommonTest(unittest.TestCase): def test_sign_apk_targetsdk_30(self): fdroidserver.common.config = None config = fdroidserver.common.read_config(fdroidserver.common.options) + if not fdroidserver.common.find_apksigner(): + self.skipTest('SKIPPING as apksigner is not installed!') config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') config['keyalias'] = 'sova' config['keystorepass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI=' From b2f6483671cb4d115a90371d4a7129d2acbb2409 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Thu, 10 Sep 2020 18:38:06 +0200 Subject: [PATCH 9/9] use new find_apksigner in test_scan_apk --- tests/update.TestCase | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/update.TestCase b/tests/update.TestCase index 25efe369..4f9c3f3f 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -484,17 +484,16 @@ class UpdateTest(unittest.TestCase): print('USE_ANDROGUARD', use_androguard) - try: - apksigner = fdroidserver.common.find_sdk_tools_cmd('apksigner') - if use_androguard and apksigner: # v2 parsing needs both + apksigner = fdroidserver.common.find_apksigner() + if apksigner: + if use_androguard: # v2 parsing needs both config['apksigner'] = apksigner apk_info = fdroidserver.update.scan_apk('v2.only.sig_2.apk') self.assertIsNone(apk_info.get('maxSdkVersion')) self.assertEqual(apk_info.get('versionName'), 'v2-only') self.assertEqual(apk_info.get('versionCode'), 2) - except fdroidserver.exception.FDroidException: + else: print('WARNING: skipping v2-only test since apksigner cannot be found') - apk_info = fdroidserver.update.scan_apk('repo/v1.v2.sig_1020.apk') self.assertIsNone(apk_info.get('maxSdkVersion')) self.assertEqual(apk_info.get('versionName'), 'v1+2')