From 63807a688d484254b2e381367493a961296818c6 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 12 May 2016 12:25:25 +1000 Subject: [PATCH 1/3] Return app name correctly from `getAppName()`. Due to the earlier refactoring of `getNotificationTitle()` (or something like that) to `getAppName()`, it was still returning `getString(downloading_apk, appName)` instead of just the app name. --- .../org/fdroid/fdroid/installer/InstallManagerService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java index 12a20b692..f8ed6a7c3 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -245,13 +245,13 @@ public class InstallManagerService extends Service { App app = ACTIVE_APPS.get(apk.packageName); if (app == null || TextUtils.isEmpty(app.name)) { if (TEMP_HACK_APP_NAMES.containsKey(urlString)) { - return getString(R.string.downloading_apk, TEMP_HACK_APP_NAMES.get(urlString)); + return TEMP_HACK_APP_NAMES.get(urlString); } else { // this is ugly, but its better than nothing as a failsafe - return getString(R.string.downloading_apk, urlString); + return urlString; } } else { - return getString(R.string.downloading_apk, app.name); + return app.name; } } From 5c4d23d2d6d2f1b6f500fcc1d20fbfd750bfb223 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 12 May 2016 14:25:34 +1000 Subject: [PATCH 2/3] Do full verification of apk before talking to installer. If we are capable of bailing earlier rather than later, then we should. This way, if a hash doesn't match, the file will be removed and a new download will begin, as expected. The alternative is to let the installer catch the unmatching hashes. By then though, it is too late to really do anything meaningfull and it becomes more difficult to recover in a way that the user would expect. --- .../installer/InstallManagerService.java | 18 +++++++++++++++++- .../org/fdroid/fdroid/installer/Installer.java | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java index f8ed6a7c3..f9fb4e4a1 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -25,6 +25,7 @@ import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.DownloaderService; import java.io.File; +import java.security.NoSuchAlgorithmException; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -144,7 +145,7 @@ public class InstallManagerService extends Service { if (!apkFilePath.exists() || apkFileSize < apk.size) { Utils.debugLog(TAG, "download " + urlString + " " + apkFilePath); DownloaderService.queue(this, urlString); - } else if (apkFileSize == apk.size) { + } else if (apkIsCached(apkFilePath, apk)) { Utils.debugLog(TAG, "skip download, we have it, straight to install " + urlString + " " + apkFilePath); sendBroadcast(intent.getData(), Downloader.ACTION_STARTED, apkFilePath); sendBroadcast(intent.getData(), Downloader.ACTION_COMPLETE, apkFilePath); @@ -156,6 +157,21 @@ public class InstallManagerService extends Service { return START_REDELIVER_INTENT; // if killed before completion, retry Intent } + /** + * Verifies the size of the file on disk matches, and then hashes the file to compare with what + * we received from the signed repo (i.e. {@link Apk#hash} and {@link Apk#hashType}). + * Bails out if the file sizes don't match to prevent having to do the work of hashing the file. + */ + private static boolean apkIsCached(File apkFile, Apk apkToCheck) { + try { + return apkFile.length() == apkToCheck.size && + Installer.verifyApkFile(apkFile, apkToCheck.hash, apkToCheck.hashType); + } catch (NoSuchAlgorithmException e) { + e.printStackTrace(); + return false; + } + } + private void sendBroadcast(Uri uri, String action, File file) { Intent intent = new Intent(action); intent.setData(uri); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index 41266f021..daf27221d 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -156,7 +156,7 @@ public abstract class Installer { /** * Checks the APK file against the provided hash, returning whether it is a match. */ - private static boolean verifyApkFile(File apkFile, String hash, String hashType) + public static boolean verifyApkFile(File apkFile, String hash, String hashType) throws NoSuchAlgorithmException { if (!apkFile.exists()) { return false; From 0967b797631df270b2e9412162b201daeecc508e Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 12 May 2016 15:04:41 +1000 Subject: [PATCH 3/3] Remove notification correctly upon cancellation of download. The check was set up to only cancel when the `AppDetails` for that app was shown. This is the correct behaviour for the 'complete' event, but not the cancel. The cancel event should always result in the relevant notification being removed. --- .../org/fdroid/fdroid/installer/InstallManagerService.java | 4 +--- .../main/java/org/fdroid/fdroid/net/DownloaderService.java | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java index f9fb4e4a1..c6a5e2272 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -226,9 +226,7 @@ public class InstallManagerService extends Service { Apk apk = ACTIVE_APKS.remove(urlString); ACTIVE_APPS.remove(apk.packageName); unregisterDownloaderReceivers(urlString); - if (AppDetails.isAppVisible(apk.packageName)) { - cancelNotification(urlString); - } + cancelNotification(urlString); } }; localBroadcastManager.registerReceiver(startedReceiver, diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 7e5961d95..a85d311ab 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -89,6 +89,7 @@ public class DownloaderService extends Service { @Override public void handleMessage(Message msg) { + Utils.debugLog(TAG, "Handling download message with ID of " + msg.what); handleIntent((Intent) msg.obj); stopSelf(msg.arg1); } @@ -120,12 +121,13 @@ public class DownloaderService extends Service { Utils.debugLog(TAG, "Cancelling download of " + uriString); Integer whatToRemove = uriString.hashCode(); if (serviceHandler.hasMessages(whatToRemove)) { + Utils.debugLog(TAG, "Removing download with ID of " + whatToRemove + " from service handler, then sending interrupted event."); serviceHandler.removeMessages(whatToRemove); sendBroadcast(intent.getData(), Downloader.ACTION_INTERRUPTED); } else if (isActive(uriString)) { downloader.cancelDownload(); } else { - Log.e(TAG, "ACTION_CANCEL called on something not queued or running"); + Log.e(TAG, "ACTION_CANCEL called on something not queued or running (expected to find message with ID of " + whatToRemove + " in queue)."); } } else if (ACTION_QUEUE.equals(intent.getAction())) { Message msg = serviceHandler.obtainMessage();