diff --git a/fdroidserver/update.py b/fdroidserver/update.py index be8adf3c..dc23d762 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -177,7 +177,7 @@ def get_old_icon_filename(appid, versionCode): def get_icon_dir(repodir, density): - if density in ('0', '65534'): + if density in ('0', '65534', '65535'): return os.path.join(repodir, "icons") else: return os.path.join(repodir, "icons-%s" % density) @@ -1745,45 +1745,53 @@ def scan_apk(apk_file): # fmt: off -def _get_apk_icons_src(apkfile, apkobject, arsc, packageName): +def _get_apk_icons_src(apkfile, apkobject, arsc): """Extract the paths to the app icon in all available densities. - The folder name is normally generated by the Android Tools, but - there is nothing that prevents people from using whatever DPI - names they make up. Android will just ignore them, so we should - too. + Parse the manifest and the resources to find all available app + icons, in all available densities and file types (e.g. .png, .webp, + .xml). "ic_launcher" was the semi-official default icon name back + in the day. """ - icon_name = 'ic_launcher' icon_id_str = apkobject.get_attribute_value("application", "icon") - if icon_id_str: - try: - icon_id = int(icon_id_str.replace("@", "0x"), 16) - resource_id = arsc.get_id(packageName, icon_id) - if resource_id: - icon_name = arsc.get_id(packageName, icon_id)[1] - else: - # don't use 'anydpi' aka 0xFFFE aka 65534 since it is XML - icon_name = os.path.splitext( - os.path.basename(apkobject.get_app_icon(max_dpi=65534 - 1)) - )[0] - except Exception as e: - logging.error("Cannot fetch icon from %s: %s" % (apkfile, str(e))) - + if not icon_id_str: + icon_id_str = apkobject.get_attribute_value("activity", "icon") icons_src = dict() - density_re = re.compile(r'^res/(.*)/{}\.png$'.format(icon_name)) - with zipfile.ZipFile(apkfile) as zf: - for filename in zf.namelist(): - m = density_re.match(filename) - if m: - folder = m.group(1).split('-') - try: - density = screen_resolutions[folder[1]] - except Exception: - density = '160' - icons_src[density] = m.group(0) - if icons_src.get('-1') is None and '160' in icons_src: - icons_src['-1'] = icons_src['160'] + if not icon_id_str: + return icons_src + try: + with zipfile.ZipFile(apkfile) as zf: + names_in_zip = zf.namelist() + + icon_id = int(icon_id_str.replace("@", "0x"), 16) + + candidates = arsc.get_resolved_res_configs(icon_id) + for candidate in candidates: + density = candidate[0].get_density() + path = candidate[1] + if path.endswith('.xml') or path not in names_in_zip: + # check it actually exists in the ZIP, some + # toolkits do strange things, like Godot Engine. + continue + icons_src[str(density)] = path + if not icons_src: + # no PNGs found, use the XML icon name + app_icon = apkobject.get_app_icon() + if app_icon: + png = os.path.basename(app_icon.replace('.xml', '.png')) + res_name_re = re.compile( + rf'res/(drawable|mipmap)-?(x*[hlm]dpi|anydpi|nodpi).*/{png}' + ) + for name in names_in_zip: + m = res_name_re.match(name) + if m: + density = screen_resolutions.get(m.group(2)) + if density is not None: + icons_src[density] = m.group() + + except Exception as e: + logging.error("Cannot fetch icon from %s: %s" % (apkfile, str(e))) return icons_src @@ -1974,7 +1982,7 @@ def scan_apk_androguard(apk, apkfile): # mistakenly put in 'manifest' in index-v2, TODO move to useSdk for index-v3 manifest['maxSdkVersion'] = maxSdkVersion - icons_src = _get_apk_icons_src(apkfile, apkobject, arsc, apk['packageName']) + icons_src = _get_apk_icons_src(apkfile, apkobject, arsc) if icons_src: apk['icons_src'] = icons_src @@ -2290,6 +2298,13 @@ def extract_apk_icons(icon_filename, apk, apkzip, repo_dir): metadata dictionary. If the icon is an XML icon, then this tries to find PNG icon that can replace it. + There are some odd special cases for DPI values, including: + * 0 means the old default when no DPI specified. + * 65535 means special case 'nodpi', which is the final fallback case. + + For more, see the docstring on Androguard's get_app_icon(): + https://github.com/androguard/androguard/blob/dd458bead6165975c3ef0b1b78eaf2450e4889d9/androguard/core/apk/__init__.py#L681 + Parameters ---------- icon_filename @@ -2307,32 +2322,19 @@ def extract_apk_icons(icon_filename, apk, apkzip, repo_dir): A list of icon densities that are missing """ - res_name_re = re.compile(r'res/(drawable|mipmap)-(x*[hlm]dpi|anydpi).*/(.*)_[0-9]+dp.(png|xml)') - pngs = dict() - for f in apkzip.namelist(): - m = res_name_re.match(f) - if m and m.group(4) == 'png': - density = screen_resolutions[m.group(2)] - pngs[m.group(3) + '/' + density] = m.group(0) empty_densities = [] for density in screen_densities: if density not in apk['icons_src']: empty_densities.append(density) continue icon_src = apk['icons_src'][density] + if icon_src.endswith('.xml'): + empty_densities.append(density) + continue + icon_dir = get_icon_dir(repo_dir, density) icon_dest = os.path.join(icon_dir, icon_filename) - # Extract the icon files per density - if icon_src.endswith('.xml'): - m = res_name_re.match(icon_src) - if m: - name = pngs.get(m.group(3) + '/' + str(density)) - if name: - icon_src = name - if icon_src.endswith('.xml'): - empty_densities.append(density) - continue try: with open(icon_dest, 'wb') as f: f.write(get_icon_bytes(apkzip, icon_src)) @@ -2342,10 +2344,16 @@ def extract_apk_icons(icon_filename, apk, apkzip, repo_dir): del apk['icons_src'][density] empty_densities.append(density) - # '-1' here is a remnant of the parsing of aapt output, meaning "no DPI specified" - if '-1' in apk['icons_src'] and not apk['icons_src']['-1'].endswith('.xml'): - icon_src = apk['icons_src']['-1'] - icon_path = os.path.join(get_icon_dir(repo_dir, '0'), icon_filename) + non_dpi_case = None + if '0' in apk['icons_src']: + non_dpi_case = '0' + elif '65535' in apk['icons_src']: + non_dpi_case = '65535' + + # move image based on DPI from measuring the image size + if non_dpi_case: + icon_src = apk['icons_src'][non_dpi_case] + icon_path = os.path.join(get_icon_dir(repo_dir, non_dpi_case), icon_filename) with open(icon_path, 'wb') as f: f.write(get_icon_bytes(apkzip, icon_src)) im = None @@ -2432,6 +2440,7 @@ def fill_missing_icon_densities(empty_densities, icon_filename, apk, repo_dir): ) empty_densities.remove(density) + # If any of the icons are too big, then size them down. for density in screen_densities: icon_dir = get_icon_dir(repo_dir, density) icon_dest = os.path.join(icon_dir, icon_filename) diff --git a/tests/metadata/apk/info.guardianproject.urzip.yaml b/tests/metadata/apk/info.guardianproject.urzip.yaml index fd1be492..b63c774a 100644 --- a/tests/metadata/apk/info.guardianproject.urzip.yaml +++ b/tests/metadata/apk/info.guardianproject.urzip.yaml @@ -7,10 +7,9 @@ file: icon: info.guardianproject.urzip.100.png icons: '0': info.guardianproject.urzip.100.png - '160': info.guardianproject.urzip.100.png + '120': info.guardianproject.urzip.100.png icons_src: - '-1': res/drawable/ic_launcher.png - '160': res/drawable/ic_launcher.png + '0': res/drawable/ic_launcher.png manifest: signer: sha256: diff --git a/tests/metadata/apk/org.dyndns.fules.ck.yaml b/tests/metadata/apk/org.dyndns.fules.ck.yaml index 7de58928..bac98f72 100644 --- a/tests/metadata/apk/org.dyndns.fules.ck.yaml +++ b/tests/metadata/apk/org.dyndns.fules.ck.yaml @@ -11,7 +11,6 @@ icons: '160': org.dyndns.fules.ck.20.png '240': org.dyndns.fules.ck.20.png icons_src: - '-1': res/drawable-mdpi-v4/icon_launcher.png '120': res/drawable-ldpi-v4/icon_launcher.png '160': res/drawable-mdpi-v4/icon_launcher.png '240': res/drawable-hdpi-v4/icon_launcher.png diff --git a/tests/metadata/apk/org.maxsdkversion.yaml b/tests/metadata/apk/org.maxsdkversion.yaml index d31a77c6..9295d0ee 100644 --- a/tests/metadata/apk/org.maxsdkversion.yaml +++ b/tests/metadata/apk/org.maxsdkversion.yaml @@ -9,7 +9,6 @@ icons: '0': org.maxsdkversion.4.png '160': org.maxsdkversion.4.png icons_src: - '-1': res/drawable-mdpi-v4/mirror.png '160': res/drawable-mdpi-v4/mirror.png manifest: features: diff --git a/tests/test_update.py b/tests/test_update.py index 8b41ff28..828f3645 100755 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -909,8 +909,8 @@ class UpdateTest(unittest.TestCase): def test_scan_apk_features(self): apk_info = fdroidserver.update.scan_apk('repo/duplicate.permisssions_9999999.apk') self.assertEqual(apk_info['manifest']['versionName'], '') - self.assertEqual(apk_info['icons_src'], {'160': 'res/drawable/ic_launcher.png', - '-1': 'res/drawable/ic_launcher.png'}) + self.assertEqual(apk_info['icons_src'], {'0': 'res/drawable/ic_launcher.png'}) + self.assertEqual( apk_info['manifest']['features'], [{'name': 'android.hardware.telephony'}], @@ -920,8 +920,7 @@ class UpdateTest(unittest.TestCase): apk_info = fdroidserver.update.scan_apk('org.dyndns.fules.ck_20.apk') self.assertEqual(apk_info['icons_src'], {'240': 'res/drawable-hdpi-v4/icon_launcher.png', '120': 'res/drawable-ldpi-v4/icon_launcher.png', - '160': 'res/drawable-mdpi-v4/icon_launcher.png', - '-1': 'res/drawable-mdpi-v4/icon_launcher.png'}) + '160': 'res/drawable-mdpi-v4/icon_launcher.png'}) self.assertEqual(apk_info['icons'], {}) self.assertEqual(apk_info['antiFeatures'], dict()) self.assertEqual(apk_info['manifest']['versionName'], 'v1.6pre2') @@ -946,8 +945,7 @@ class UpdateTest(unittest.TestCase): def test_scan_apk_two_icons(self): apk_info = fdroidserver.update.scan_apk('org.bitbucket.tickytacky.mirrormirror_4.apk') self.assertEqual(apk_info['manifest']['versionName'], '1.0.3') - self.assertEqual(apk_info['icons_src'], {'160': 'res/drawable-mdpi/mirror.png', - '-1': 'res/drawable-mdpi/mirror.png'}) + self.assertEqual(apk_info['icons_src'], {'160': 'res/drawable-mdpi/mirror.png'}) def test_scan_apk_xml_icon(self): apk_info = fdroidserver.update.scan_apk('repo/info.zwanenburg.caffeinetile_4.apk') @@ -960,8 +958,7 @@ class UpdateTest(unittest.TestCase): self.assertEqual(apk_info['icons_src'], {'120': 'res/drawable-ldpi-v4/icon.png', '160': 'res/drawable-mdpi-v4/icon.png', '240': 'res/drawable-hdpi-v4/icon.png', - '320': 'res/drawable-xhdpi-v4/icon.png', - '-1': 'res/drawable-mdpi-v4/icon.png'}) + '320': 'res/drawable-xhdpi-v4/icon.png'}) def test_scan_apk_no_icons(self): apk_info = fdroidserver.update.scan_apk('SpeedoMeterApp.main_1.apk') @@ -994,10 +991,7 @@ class UpdateTest(unittest.TestCase): self.maxDiff = None expected = { 'icons': {}, - 'icons_src': { - '-1': 'res/drawable/ic_launcher.png', - '160': 'res/drawable/ic_launcher.png', - }, + 'icons_src': {'0': 'res/drawable/ic_launcher.png'}, 'file': { 'name': 'no.min.target.sdk_987.apk', 'sha256': 'e2e1dc1d550df2b5bc383860139207258645b5540abeccd305ed8b2cb6459d2c', @@ -1967,7 +1961,6 @@ class UpdateTest(unittest.TestCase): '240': 'res/drawable-hdpi-v4/icon_launcher.png', '120': 'res/drawable-ldpi-v4/icon_launcher.png', '160': 'res/drawable-mdpi-v4/icon_launcher.png', - '-1': 'res/drawable-mdpi-v4/icon_launcher.png', }, 'manifest': { 'nativecode': [ @@ -2257,110 +2250,126 @@ class UpdateTest(unittest.TestCase): fdroidserver.update.get_icon_dir(repodir, density), ) + def test_get_icon_dir_nodpi(self): + repodir = 'repo' + density = fdroidserver.update.screen_resolutions['nodpi'] + self.assertEqual( + f'{repodir}/icons', + fdroidserver.update.get_icon_dir(repodir, density), + ) + class TestGetApkIconsSrc(unittest.TestCase): - def get_apk_icons_src(self, apkfile, appid): + def get_apk_icons_src(self, apkfile): apkobject = fdroidserver.common.get_androguard_APK(apkfile) arsc = apkobject.get_android_resources() - return fdroidserver.update._get_apk_icons_src(apkfile, apkobject, arsc, appid) + return fdroidserver.update._get_apk_icons_src(apkfile, apkobject, arsc) def test_get_apk_icons_src_urzip(self): self.assertEqual( - { - '-1': 'res/drawable/ic_launcher.png', - '160': 'res/drawable/ic_launcher.png', - }, - self.get_apk_icons_src(basedir / 'urzip.apk', 'info.guardianproject.urzip'), + {'0': 'res/drawable/ic_launcher.png'}, + self.get_apk_icons_src(basedir / 'urzip.apk'), ) def test_get_apk_icons_src_mirrormirror(self): appid = 'org.bitbucket.tickytacky.mirrormirror' self.assertEqual( - { - '-1': 'res/drawable-mdpi/mirror.png', - '160': 'res/drawable-mdpi/mirror.png', - }, - self.get_apk_icons_src(basedir / f'{appid}_4.apk', appid), + {'160': 'res/drawable-mdpi/mirror.png'}, + self.get_apk_icons_src(basedir / f'{appid}_4.apk'), ) def test_get_apk_icons_src_fules_ck(self): appid = 'org.dyndns.fules.ck' self.assertEqual( { - '-1': 'res/drawable-mdpi-v4/icon_launcher.png', '120': 'res/drawable-ldpi-v4/icon_launcher.png', '160': 'res/drawable-mdpi-v4/icon_launcher.png', '240': 'res/drawable-hdpi-v4/icon_launcher.png', }, - self.get_apk_icons_src(basedir / f'{appid}_20.apk', appid), + self.get_apk_icons_src(basedir / f'{appid}_20.apk'), + ) + + def test_get_apk_icons_src_SpeedoMeterApp_main(self): + """Test handling APK with no icon set.""" + appid = 'SpeedoMeterApp.main' + self.assertEqual( + {}, + self.get_apk_icons_src(basedir / f'{appid}_1.apk'), + ) + + def test_get_apk_icons_src_fallingblocks(self): + """Test example made with Godot Engine.""" + appid = 'org.sajeg.fallingblocks' + self.assertEqual( + {'0': 'res/mipmap/icon.png'}, + self.get_apk_icons_src(basedir / f'{appid}_3.apk'), + ) + + def test_get_apk_icons_src_com_example_test_helloworld(self): + """Test APK with no apparent icon, but some similarly named files.""" + appid = 'com.example.test.helloworld' + self.assertEqual( + {}, + self.get_apk_icons_src(basedir / f'repo/{appid}_1.apk'), ) def test_get_apk_icons_src_com_politedroid_3(self): appid = 'com.politedroid' self.assertEqual( { - '-1': 'res/drawable-mdpi/icon.png', '120': 'res/drawable-ldpi/icon.png', '160': 'res/drawable-mdpi/icon.png', '240': 'res/drawable-hdpi/icon.png', '320': 'res/drawable-xhdpi/icon.png', }, - self.get_apk_icons_src(basedir / f'repo/{appid}_3.apk', appid), + self.get_apk_icons_src(basedir / f'repo/{appid}_3.apk'), ) def test_get_apk_icons_src_com_politedroid_6(self): appid = 'com.politedroid' self.assertEqual( { - '-1': 'res/drawable-mdpi-v4/icon.png', '120': 'res/drawable-ldpi-v4/icon.png', '160': 'res/drawable-mdpi-v4/icon.png', '240': 'res/drawable-hdpi-v4/icon.png', '320': 'res/drawable-xhdpi-v4/icon.png', }, - self.get_apk_icons_src(basedir / f'repo/{appid}_6.apk', appid), + self.get_apk_icons_src(basedir / f'repo/{appid}_6.apk'), ) def test_get_apk_icons_src_duplicate_permisssions(self): appid = 'duplicate.permisssions' self.assertEqual( - { - '-1': 'res/drawable/ic_launcher.png', - '160': 'res/drawable/ic_launcher.png', - }, - self.get_apk_icons_src(basedir / f'repo/{appid}_9999999.apk', appid), + {'0': 'res/drawable/ic_launcher.png'}, + self.get_apk_icons_src(basedir / f'repo/{appid}_9999999.apk'), ) def test_get_apk_icons_src_info_zwanenburg_caffeinetile(self): """Test an APK with no PNG or WebP, only XML.""" appid = 'info.zwanenburg.caffeinetile' self.assertEqual( - {}, - self.get_apk_icons_src(basedir / f'repo/{appid}_4.apk', appid), + dict(), + self.get_apk_icons_src(basedir / f'repo/{appid}_4.apk'), ) def test_get_apk_icons_src_org_maxsdkversion(self): appid = 'org.maxsdkversion' self.assertEqual( - { - '-1': 'res/drawable-mdpi-v4/mirror.png', - '160': 'res/drawable-mdpi-v4/mirror.png', - }, - self.get_apk_icons_src(basedir / f'repo/{appid}_4.apk', appid), + {'160': 'res/drawable-mdpi-v4/mirror.png'}, + self.get_apk_icons_src(basedir / f'repo/{appid}_4.apk'), ) def test_get_apk_icons_src_souch_smsbypass(self): appid = 'souch.smsbypass' self.assertEqual( { - '-1': 'res/drawable-mdpi-v4/ic_launcher.png', '160': 'res/drawable-mdpi-v4/ic_launcher.png', '213': 'res/drawable-tvdpi-v4/ic_launcher.png', '240': 'res/drawable-hdpi-v4/ic_launcher.png', '320': 'res/drawable-xhdpi-v4/ic_launcher.png', '480': 'res/drawable-xxhdpi-v4/ic_launcher.png', }, - self.get_apk_icons_src(basedir / f'repo/{appid}_9.apk', appid), + self.get_apk_icons_src(basedir / f'repo/{appid}_9.apk'), )