From 532d1dfc72cfc0f027c888c47f05af4d2c166879 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 8 Feb 2018 23:52:23 +0100 Subject: [PATCH] make sure cached file exists before trying to scan it Files in the cache can be deleted at any time, without warning. F-Droid's CleanCacheService can do it, the user can do it in Settings --> Apps, etc. So when working with files from the cache, the methods need to be extra defensive, checking that the file that they were given still exists. closes #1305 --- .../fdroid/fdroid/AppUpdateStatusService.java | 14 +++++--- .../main/java/org/fdroid/fdroid/data/App.java | 34 +++++++++++++------ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java index 03cfb6841..dc5740d0e 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java +++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java @@ -9,7 +9,6 @@ import android.net.Uri; import android.support.annotation.Nullable; import android.text.TextUtils; import android.util.Log; - import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.InstalledAppProviderService; @@ -109,7 +108,7 @@ public class AppUpdateStatusService extends IntentService { Utils.debugLog(TAG, "Found package for " + downloadedInfo.packageName + ", checking its hash to see if it downloaded correctly."); Apk downloadedApk = findApkMatchingHash(apkPath); - if (downloadedApk == null) { + if (downloadedApk == null) { Log.i(TAG, "Either the apk wasn't downloaded fully, or the repo it came from has been disabled. Either way, not notifying the user about it."); return null; } @@ -129,7 +128,8 @@ public class AppUpdateStatusService extends IntentService { AppUpdateStatusManager.getInstance(this).markAsNoLongerPendingInstall(downloadedApk.getUrl()); return null; } - } catch (PackageManager.NameNotFoundException ignored) { } + } catch (PackageManager.NameNotFoundException ignored) { + } Utils.debugLog(TAG, downloadedApk.packageName + " is pending install, so we need to notify the user about installing it."); return downloadedApk; @@ -140,12 +140,16 @@ public class AppUpdateStatusService extends IntentService { * This method looks for all matching records in the database. It then asks each of these * {@link Apk} instances where they expect to be downloaded. If they expect to be downloaded * to {@param apkPath} then that instance is returned. - * + *

* If no files have a matching hash, or only those which don't belong to the correct repo, then - * this will return null. + * this will return null. This method needs to do its own check whether the file exists, + * since files can be deleted from the cache at any time without warning. */ @Nullable private Apk findApkMatchingHash(File apkPath) { + if (!apkPath.canRead()) { + return null; + } // NOTE: This presumes SHA256 is the only supported hash. It seems like that is an assumption // in more than one place in the F-Droid client. If this becomes a problem in the future, we diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index a6d2430b0..9bfd051c4 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -706,11 +706,19 @@ public class App extends ValueObject implements Comparable, Parcelable { this.compatible = true; } + /** + * Initializes an {@link App} instances from an APK file. Since the file + * could in the cache, and files can disappear from the cache at any time, + * this needs to be quite defensive ensuring that {@code apkFile} still + * exists. + */ private void initApkFromApkFile(Context context, Apk apk, PackageInfo packageInfo, SanitizedFile apkFile) throws IOException, CertificateEncodingException { // TODO include signature hash calculation here - apk.hashType = "sha256"; - apk.hash = Utils.getBinaryHash(apkFile, apk.hashType); + if (apkFile.canRead()) { + apk.hashType = "sha256"; + apk.hash = Utils.getBinaryHash(apkFile, apk.hashType); + } initInstalledApk(context, apk, packageInfo, apkFile); } @@ -750,10 +758,22 @@ public class App extends ValueObject implements Comparable, Parcelable { apk.packageName = this.packageName; apk.requestedPermissions = packageInfo.requestedPermissions; apk.apkName = apk.packageName + "_" + apk.versionCode + ".apk"; - apk.installedFile = apkFile; initInstalledObbFiles(apk); + final FeatureInfo[] features = packageInfo.reqFeatures; + if (features != null && features.length > 0) { + apk.features = new String[features.length]; + for (int i = 0; i < features.length; i++) { + apk.features[i] = features[i].name; + } + } + + if (!apkFile.canRead()) { + return; + } + + apk.installedFile = apkFile; JarFile apkJar = new JarFile(apkFile); HashSet abis = new HashSet<>(3); Pattern pattern = Pattern.compile("^lib/([a-z0-9-]+)/.*"); @@ -766,14 +786,6 @@ public class App extends ValueObject implements Comparable, Parcelable { } apk.nativecode = abis.toArray(new String[abis.size()]); - final FeatureInfo[] features = packageInfo.reqFeatures; - if (features != null && features.length > 0) { - apk.features = new String[features.length]; - for (int i = 0; i < features.length; i++) { - apk.features[i] = features[i].name; - } - } - final JarEntry aSignedEntry = (JarEntry) apkJar.getEntry("AndroidManifest.xml"); if (aSignedEntry == null) {