From 70e7e720b9c2f3cbcf3ae44105abf613fd96767d Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 28 Aug 2019 13:42:40 +0200 Subject: [PATCH] update: use graphics filename with hash in index to support caching Using a filename based on the hash of the contents means that the caching algorithms for fdroidclient and browsers can safely cache the file forever using the filename, since this guarantees that the contents will never change for a given filename. This does not cover screenshots, only icon.png, featureGraphic.png, tvBanner.png, and promoGraphic.png. fdroidserver#689 fdroid-website!453 --- .gitignore | 3 +++ CHANGELOG.md | 2 ++ fdroidserver/update.py | 42 ++++++++++++++++++++++++++++++++++++---- tests/repo/index-v1.json | 8 ++++---- tests/update.TestCase | 25 ++++++++++++++++-------- 5 files changed, 64 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index 73a13d4c..25aab3b3 100644 --- a/.gitignore +++ b/.gitignore @@ -53,6 +53,9 @@ makebuildserver.config.py /tests/repo/info.guardianproject.urzip/ /tests/repo/info.guardianproject.checkey/en-US/phoneScreenshots/checkey-phone.png /tests/repo/info.guardianproject.checkey/en-US/phoneScreenshots/checkey.png +/tests/repo/obb.mainpatch.current/en-US/featureGraphic_ffhLaojxbGAfu9ROe1MJgK5ux8d0OVc6b65nmvOBaTk=.png +/tests/repo/obb.mainpatch.current/en-US/icon_WI0pkO3LsklrsTAnRr-OQSxkkoMY41lYe2-fAvXLiLg=.png +/tests/repo/org.videolan.vlc/en-US/icon_yAfSvPRJukZzMMfUzvbYqwaD1XmHXNtiPBtuPVHW-6s=.png /tests/urzip-πÇÇπÇÇ现代汉语通用字-български-عربي1234.apk /unsigned/ diff --git a/CHANGELOG.md b/CHANGELOG.md index a486c5f8..20f0e850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ([!663](https://gitlab.com/fdroid/fdroidserver/merge_requests/663)) * added support for gradle 5.5.1 ([!656](https://gitlab.com/fdroid/fdroidserver/merge_requests/656)) +* add SHA256 to filename of repo graphics + ([!669](https://gitlab.com/fdroid/fdroidserver/merge_requests/669)) ### Fixed * checkupdates: UpdateCheckIngore gets properly observed now diff --git a/fdroidserver/update.py b/fdroidserver/update.py index ce8ab0f4..d1dd11bf 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -34,6 +34,7 @@ import time import copy from datetime import datetime from argparse import ArgumentParser +from base64 import urlsafe_b64encode import collections from binascii import hexlify @@ -522,6 +523,18 @@ def sha256sum(filename): return sha.hexdigest() +def sha256base64(filename): + '''Calculate the sha256 of the given file as URL-safe base64''' + hasher = hashlib.sha256() + with open(filename, 'rb') as f: + while True: + t = f.read(16384) + if len(t) == 0: + break + hasher.update(t) + return urlsafe_b64encode(hasher.digest()).decode() + + def has_known_vulnerability(filename): """checks for known vulnerabilities in the APK @@ -755,6 +768,16 @@ def _strip_and_copy_image(in_file, outpath): os.utime(out_file, times=(stat_result.st_atime, stat_result.st_mtime)) +def _get_base_hash_extension(f): + '''split a graphic/screenshot filename into base, sha256, and extension + ''' + base, extension = common.get_extension(f) + sha256_index = base.find('_') + if sha256_index > 0: + return base[:sha256_index], base[sha256_index + 1:], extension + return base, None, extension + + def copy_triple_t_store_metadata(apps): """Include store metadata from the app's source repo @@ -965,8 +988,8 @@ def insert_localized_app_metadata(apps): os.makedirs(screenshotdestdir, mode=0o755, exist_ok=True) _strip_and_copy_image(f, screenshotdestdir) - repofiles = sorted(glob.glob(os.path.join('repo', '[A-Za-z]*', '[a-z][a-z]*'))) - for d in repofiles: + repodirs = sorted(glob.glob(os.path.join('repo', '[A-Za-z]*', '[a-z][a-z]*'))) + for d in repodirs: if not os.path.isdir(d): continue for f in sorted(glob.glob(os.path.join(d, '*.*')) + glob.glob(os.path.join(d, '*Screenshots', '*.*'))): @@ -977,7 +1000,7 @@ def insert_localized_app_metadata(apps): locale = segments[2] screenshotdir = segments[3] filename = os.path.basename(f) - base, extension = common.get_extension(filename) + base, sha256, extension = _get_base_hash_extension(filename) if packageName not in apps: logging.warning(_('Found "{path}" graphic without metadata for app "{name}"!') @@ -989,7 +1012,18 @@ def insert_localized_app_metadata(apps): logging.warning(_('Only PNG and JPEG are supported for graphics, found: {path}').format(path=f)) elif base in GRAPHIC_NAMES: # there can only be zero or one of these per locale - graphics[base] = filename + basename = base + '.' + extension + basepath = os.path.join(os.path.dirname(f), basename) + if sha256: + if not os.path.samefile(f, basepath): + os.unlink(f) + else: + sha256 = sha256base64(f) + filename = base + '_' + sha256 + '.' + extension + index_file = os.path.join(os.path.dirname(f), filename) + if not os.path.exists(index_file): + os.link(f, index_file, follow_symlinks=False) + graphics[base] = filename elif screenshotdir in SCREENSHOT_DIRS: # there can any number of these per locale logging.debug(_('adding to {name}: {path}').format(name=screenshotdir, path=f)) diff --git a/tests/repo/index-v1.json b/tests/repo/index-v1.json index a5a1d1e9..492b6051 100644 --- a/tests/repo/index-v1.json +++ b/tests/repo/index-v1.json @@ -141,8 +141,8 @@ "lastUpdated": 1496275200000, "localized": { "en-US": { - "featureGraphic": "featureGraphic.png", - "icon": "icon.png", + "featureGraphic": "featureGraphic_ffhLaojxbGAfu9ROe1MJgK5ux8d0OVc6b65nmvOBaTk=.png", + "icon": "icon_WI0pkO3LsklrsTAnRr-OQSxkkoMY41lYe2-fAvXLiLg=.png", "phoneScreenshots": [ "screenshot-main.png" ], @@ -196,8 +196,8 @@ "localized": { "en-US": { "description": "full description\n", - "featureGraphic": "featureGraphic.png", - "icon": "icon.png", + "featureGraphic": "featureGraphic_GFRT5BovZsENGpJq1HqPODGWBRPWQsx25B95Ol5w_wU=.png", + "icon": "icon_NJXNzMcyf-v9i5a1ElJi0j9X1LvllibCa48xXYPlOqQ=.png", "name": "title\n", "summary": "short description\n", "video": "video\n" diff --git a/tests/update.TestCase b/tests/update.TestCase index 454780c8..fc5f69bb 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -66,9 +66,10 @@ class UpdateTest(unittest.TestCase): shutil.copytree(os.path.join('source-files', 'eu.siacs.conversations'), os.path.join('build', 'eu.siacs.conversations')) + testfilename = 'icon_yAfSvPRJukZzMMfUzvbYqwaD1XmHXNtiPBtuPVHW-6s=.png' testfile = os.path.join('repo', 'org.videolan.vlc', 'en-US', 'icon.png') cpdir = os.path.join('metadata', 'org.videolan.vlc', 'en-US') - cpfile = os.path.join(cpdir, 'icon.png') + cpfile = os.path.join(cpdir, testfilename) os.makedirs(cpdir, exist_ok=True) shutil.copy(testfile, cpfile) shutil.copystat(testfile, cpfile) @@ -98,8 +99,12 @@ class UpdateTest(unittest.TestCase): fdroidserver.update.insert_localized_app_metadata(apps) appdir = os.path.join('repo', 'info.guardianproject.urzip', 'en-US') - self.assertTrue(os.path.isfile(os.path.join(appdir, 'icon.png'))) - self.assertTrue(os.path.isfile(os.path.join(appdir, 'featureGraphic.png'))) + self.assertTrue(os.path.isfile(os.path.join( + appdir, + 'icon_NJXNzMcyf-v9i5a1ElJi0j9X1LvllibCa48xXYPlOqQ=.png'))) + self.assertTrue(os.path.isfile(os.path.join( + appdir, + 'featureGraphic_GFRT5BovZsENGpJq1HqPODGWBRPWQsx25B95Ol5w_wU=.png'))) self.assertEqual(6, len(apps)) for packageName, app in apps.items(): @@ -112,16 +117,20 @@ class UpdateTest(unittest.TestCase): self.assertEqual('title\n', app['localized']['en-US']['name']) self.assertEqual('short description\n', app['localized']['en-US']['summary']) self.assertEqual('video\n', app['localized']['en-US']['video']) - self.assertEqual('icon.png', app['localized']['en-US']['icon']) - self.assertEqual('featureGraphic.png', app['localized']['en-US']['featureGraphic']) + self.assertEqual('icon_NJXNzMcyf-v9i5a1ElJi0j9X1LvllibCa48xXYPlOqQ=.png', + app['localized']['en-US']['icon']) + self.assertEqual('featureGraphic_GFRT5BovZsENGpJq1HqPODGWBRPWQsx25B95Ol5w_wU=.png', + app['localized']['en-US']['featureGraphic']) self.assertEqual('100\n', app['localized']['en-US']['whatsNew']) elif packageName == 'org.videolan.vlc': - self.assertEqual('icon.png', app['localized']['en-US']['icon']) + self.assertEqual(testfilename, app['localized']['en-US']['icon']) self.assertEqual(9, len(app['localized']['en-US']['phoneScreenshots'])) self.assertEqual(15, len(app['localized']['en-US']['sevenInchScreenshots'])) elif packageName == 'obb.mainpatch.current': - self.assertEqual('icon.png', app['localized']['en-US']['icon']) - self.assertEqual('featureGraphic.png', app['localized']['en-US']['featureGraphic']) + self.assertEqual('icon_WI0pkO3LsklrsTAnRr-OQSxkkoMY41lYe2-fAvXLiLg=.png', + app['localized']['en-US']['icon']) + self.assertEqual('featureGraphic_ffhLaojxbGAfu9ROe1MJgK5ux8d0OVc6b65nmvOBaTk=.png', + app['localized']['en-US']['featureGraphic']) self.assertEqual(1, len(app['localized']['en-US']['phoneScreenshots'])) self.assertEqual(1, len(app['localized']['en-US']['sevenInchScreenshots'])) elif packageName == 'com.nextcloud.client':