Merge branch 'graphic-hash-filename-for-caching' into 'master'

Graphic hash filename for caching

See merge request fdroid/fdroidserver!669

.gitignore

@@ -46,12 +46,16 @@ makebuildserver.config.py
 /tests/archive/index.xml
 /tests/archive/index-v1.jar
 /tests/archive/index-v1.json
+/tests/metadata/org.videolan.vlc/en-US/icon*.png
 /tests/repo/index.jar
 /tests/repo/index_unsigned.jar
 /tests/repo/index-v1.jar
 /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/
 

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

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
 
@@ -707,33 +720,62 @@ def _set_author_entry(app, key, f):
             app[key] = text
 
 
-def _strip_and_copy_image(inpath, outpath):
+def _strip_and_copy_image(in_file, outpath):
     """Remove any metadata from image and copy it to new path
 
     Sadly, image metadata like EXIF can be used to exploit devices.
     It is not used at all in the F-Droid ecosystem, so its much safer
     just to remove it entirely.
 
-    """
+    This uses size+mtime to check for a new file since this process
+    actually modifies the resulting file to strip out the EXIF.
+
+    outpath can be path to either a file or dir.  The dir that outpath
+    refers to must exist before calling this.
+
+    """
+    logging.debug('copying ' + in_file + ' ' + outpath)
 
-    extension = common.get_extension(inpath)[1]
     if os.path.isdir(outpath):
-        outpath = os.path.join(outpath, os.path.basename(inpath))
+        out_file = os.path.join(outpath, os.path.basename(in_file))
+    else:
+        out_file = outpath
+
+    if os.path.exists(out_file):
+        in_stat = os.stat(in_file)
+        out_stat = os.stat(out_file)
+        if in_stat.st_size == out_stat.st_size \
+           and in_stat.st_mtime == out_stat.st_mtime:
+            return
+
+    extension = common.get_extension(in_file)[1]
     if extension == 'png':
-        with open(inpath, 'rb') as fp:
+        with open(in_file, 'rb') as fp:
             in_image = Image.open(fp)
-            in_image.save(outpath, "PNG", optimize=True,
+            in_image.save(out_file, "PNG", optimize=True,
                           pnginfo=BLANK_PNG_INFO, icc_profile=None)
     elif extension == 'jpg' or extension == 'jpeg':
-        with open(inpath, 'rb') as fp:
+        with open(in_file, 'rb') as fp:
             in_image = Image.open(fp)
             data = list(in_image.getdata())
             out_image = Image.new(in_image.mode, in_image.size)
         out_image.putdata(data)
-        out_image.save(outpath, "JPEG", optimize=True)
+        out_image.save(out_file, "JPEG", optimize=True)
     else:
         raise FDroidException(_('Unsupported file type "{extension}" for repo graphic')
                               .format(extension=extension))
+    stat_result = os.stat(in_file)
+    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):
@@ -845,7 +887,6 @@ def copy_triple_t_store_metadata(apps):
                         os.makedirs(destdir, mode=0o755, exist_ok=True)
                         sourcefile = os.path.join(root, f)
                         destfile = os.path.join(destdir, repofilename)
-                        logging.debug('copying ' + sourcefile + ' ' + destfile)
                         _strip_and_copy_image(sourcefile, destfile)
 
 
@@ -934,7 +975,6 @@ def insert_localized_app_metadata(apps):
                     destdir = os.path.join('repo', packageName, locale)
                 if base in GRAPHIC_NAMES and extension in ALLOWED_EXTENSIONS:
                     os.makedirs(destdir, mode=0o755, exist_ok=True)
-                    logging.debug('copying ' + os.path.join(root, f) + ' ' + destdir)
                     _strip_and_copy_image(os.path.join(root, f), destdir)
             for d in dirs:
                 if d in SCREENSHOT_DIRS:
@@ -946,11 +986,10 @@ def insert_localized_app_metadata(apps):
                         if extension in ALLOWED_EXTENSIONS:
                             screenshotdestdir = os.path.join(destdir, d)
                             os.makedirs(screenshotdestdir, mode=0o755, exist_ok=True)
-                            logging.debug('copying ' + f + ' ' + screenshotdestdir)
                             _strip_and_copy_image(f, screenshotdestdir)
 
-    repofiles = sorted(glob.glob(os.path.join('repo', '[A-Za-z]*', '[a-z][a-z]*')))
+    repodirs = sorted(glob.glob(os.path.join('repo', '[A-Za-z]*', '[a-z][a-z]*')))
-    for d in repofiles:
+    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', '*.*'))):
@@ -961,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}"!')
@@ -973,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))

setup.py

@@ -49,10 +49,14 @@ def get_data_files():
     return data_files
 
 
+with open("README.md", "r") as fh:
+    long_description = fh.read()
+
+
 setup(name='fdroidserver',
       version='1.2a',
       description='F-Droid Server Tools',
-      long_description='README.md',
+      long_description=long_description,
       long_description_content_type='text/markdown',
       author='The F-Droid Project',
       author_email='team@f-droid.org',
@@ -80,7 +84,7 @@ setup(name='fdroidserver',
           'python-vagrant',
           'PyYAML',
           'qrcode',
-          'ruamel.yaml >= 0.13',
+          'ruamel.yaml >= 0.15',
           'requests >= 2.5.2, != 2.11.0, != 2.12.2, != 2.18.0',
           'docker-py >= 1.9, < 2.0',
       ],

tests/repo/index-v1.json

@@ -141,8 +141,8 @@
       "lastUpdated": 1496275200000,
       "localized": {
         "en-US": {
-          "featureGraphic": "featureGraphic.png",
+          "featureGraphic": "featureGraphic_ffhLaojxbGAfu9ROe1MJgK5ux8d0OVc6b65nmvOBaTk=.png",
-          "icon": "icon.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",
+          "featureGraphic": "featureGraphic_GFRT5BovZsENGpJq1HqPODGWBRPWQsx25B95Ol5w_wU=.png",
-          "icon": "icon.png",
+          "icon": "icon_NJXNzMcyf-v9i5a1ElJi0j9X1LvllibCa48xXYPlOqQ=.png",
           "name": "title\n",
           "summary": "short description\n",
           "video": "video\n"

tests/update.TestCase

@@ -66,6 +66,14 @@ 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, testfilename)
+        os.makedirs(cpdir, exist_ok=True)
+        shutil.copy(testfile, cpfile)
+        shutil.copystat(testfile, cpfile)
+
         apps = dict()
         for packageName in ('info.guardianproject.urzip', 'org.videolan.vlc', 'obb.mainpatch.current',
                             'com.nextcloud.client', 'com.nextcloud.client.dev',
@@ -91,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(
-        self.assertTrue(os.path.isfile(os.path.join(appdir, 'featureGraphic.png')))
+            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():
@@ -105,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('icon_NJXNzMcyf-v9i5a1ElJi0j9X1LvllibCa48xXYPlOqQ=.png',
-                self.assertEqual('featureGraphic.png', app['localized']['en-US']['featureGraphic'])
+                                 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('icon_WI0pkO3LsklrsTAnRr-OQSxkkoMY41lYe2-fAvXLiLg=.png',
-                self.assertEqual('featureGraphic.png', app['localized']['en-US']['featureGraphic'])
+                                 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':