From 9d2fe4000d370125a164e181b91acf87877b7538 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 1 Jun 2016 20:18:23 +0200 Subject: [PATCH 1/6] use download URL as unique ID through the whole install process InstallManagerService and DownloaderService both use the download URL as the unique ID to represent a given APK install through the whole lifecycle of the install and download process. This converts the installer stuff to use the same semantics. A Uri instance is mostly used there because its the most useful format, but ultimately, the String, Uri, and int all derive from the exact same URL. This then removes the local APK URI from use in the installer broadcasts. While I normally think reusing terms from Android is the best thing to do, "originating URI" drives me nuts because it is almost nonsense English. "Originating" is a verb in the continuous form, meaning that it is an action that is ongoing. A URI is a static thing, and in this case, a URI that points to a file that is completely downloaded. I left the term in place for DefaultInstaller because it wraps PackageManager, which is where that term originates. This handles "Use strings instead of Uris in InstallManagerService for urlString" as listed in #680 --- .../java/org/fdroid/fdroid/AppDetails.java | 8 +--- .../fdroid/installer/DefaultInstaller.java | 18 ++++---- .../installer/DefaultInstallerActivity.java | 31 ++++++------- .../fdroid/installer/ExtensionInstaller.java | 15 +++---- .../installer/InstallManagerService.java | 43 ++++++++++--------- .../fdroid/fdroid/installer/Installer.java | 39 +++++++++++------ .../fdroid/installer/InstallerService.java | 23 +++++----- .../fdroid/installer/PrivilegedInstaller.java | 16 +++---- .../views/swap/SwapWorkflowActivity.java | 2 +- 9 files changed, 100 insertions(+), 95 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 5eb446118..2f633bde2 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -85,14 +85,13 @@ import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.InstalledAppProvider; import org.fdroid.fdroid.data.RepoProvider; -import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.installer.InstallManagerService; +import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.installer.InstallerFactory; import org.fdroid.fdroid.installer.InstallerService; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.DownloaderService; -import java.io.File; import java.util.Iterator; import java.util.List; @@ -530,11 +529,8 @@ public class AppDetails extends AppCompatActivity { @Override public void onReceive(Context context, Intent intent) { cleanUpFinishedDownload(); - - Uri localUri = - Uri.fromFile(new File(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH))); localBroadcastManager.registerReceiver(installReceiver, - Installer.getInstallIntentFilter(localUri)); + Installer.getInstallIntentFilter(intent.getData())); } }; diff --git a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java index c6d73654f..b3bcf6218 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java @@ -45,34 +45,34 @@ public class DefaultInstaller extends Installer { } @Override - protected void installPackage(Uri uri, Uri originatingUri, String packageName) { - sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_STARTED); + protected void installPackage(Uri localApkUri, Uri downloadUri, String packageName) { + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_STARTED); - Utils.debugLog(TAG, "DefaultInstaller uri: " + uri + " file: " + new File(uri.getPath())); + Utils.debugLog(TAG, "DefaultInstaller uri: " + localApkUri + " file: " + new File(localApkUri.getPath())); Uri sanitizedUri; try { - sanitizedUri = Installer.prepareApkFile(context, uri, packageName); + sanitizedUri = Installer.prepareApkFile(context, localApkUri, packageName); } catch (Installer.InstallFailedException e) { Log.e(TAG, "prepareApkFile failed", e); - sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_INTERRUPTED, + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, e.getMessage()); return; } Intent installIntent = new Intent(context, DefaultInstallerActivity.class); installIntent.setAction(DefaultInstallerActivity.ACTION_INSTALL_PACKAGE); - installIntent.putExtra(DefaultInstallerActivity.EXTRA_ORIGINATING_URI, originatingUri); + installIntent.putExtra(Installer.EXTRA_DOWNLOAD_URI, downloadUri); installIntent.setData(sanitizedUri); PendingIntent installPendingIntent = PendingIntent.getActivity( context.getApplicationContext(), - uri.hashCode(), + localApkUri.hashCode(), installIntent, PendingIntent.FLAG_UPDATE_CURRENT); - sendBroadcastInstall(uri, originatingUri, - Installer.ACTION_INSTALL_USER_INTERACTION, installPendingIntent); + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_USER_INTERACTION, + installPendingIntent); } @Override diff --git a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java index 6e1e64ff4..e4727ee8b 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java @@ -42,14 +42,11 @@ public class DefaultInstallerActivity extends FragmentActivity { public static final String ACTION_UNINSTALL_PACKAGE = "org.fdroid.fdroid.UNINSTALL_PACKAGE"; public static final String EXTRA_UNINSTALL_PACKAGE_NAME = "uninstallPackageName"; - public static final String EXTRA_ORIGINATING_URI = "originatingUri"; private static final int REQUEST_CODE_INSTALL = 0; private static final int REQUEST_CODE_UNINSTALL = 1; - private Uri installOriginatingUri; - private Uri installUri; - + private Uri downloadUri; private String uninstallPackageName; // for the broadcasts @@ -64,10 +61,9 @@ public class DefaultInstallerActivity extends FragmentActivity { Intent intent = getIntent(); String action = intent.getAction(); if (ACTION_INSTALL_PACKAGE.equals(action)) { - installUri = intent.getData(); - installOriginatingUri = intent.getParcelableExtra(EXTRA_ORIGINATING_URI); - - installPackage(installUri, installOriginatingUri); + Uri localApkUri = intent.getData(); + downloadUri = intent.getParcelableExtra(Installer.EXTRA_DOWNLOAD_URI); + installPackage(localApkUri); } else if (ACTION_UNINSTALL_PACKAGE.equals(action)) { uninstallPackageName = intent.getStringExtra(EXTRA_UNINSTALL_PACKAGE_NAME); @@ -78,7 +74,7 @@ public class DefaultInstallerActivity extends FragmentActivity { } @SuppressLint("InlinedApi") - private void installPackage(Uri uri, Uri originatingUri) { + private void installPackage(Uri uri) { if (uri == null) { throw new RuntimeException("Set the data uri to point to an apk location!"); } @@ -121,12 +117,11 @@ public class DefaultInstallerActivity extends FragmentActivity { startActivityForResult(intent, REQUEST_CODE_INSTALL); } catch (ActivityNotFoundException e) { Log.e(TAG, "ActivityNotFoundException", e); - installer.sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_INTERRUPTED, + installer.sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, "This Android rom does not support ACTION_INSTALL_PACKAGE!"); finish(); } - installer.sendBroadcastInstall(installUri, installOriginatingUri, - Installer.ACTION_INSTALL_STARTED); + installer.sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_STARTED); } protected void uninstallPackage(String packageName) { @@ -172,31 +167,29 @@ public class DefaultInstallerActivity extends FragmentActivity { * never executed on Androids < 4.0 */ if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) { - installer.sendBroadcastInstall(installUri, installOriginatingUri, - Installer.ACTION_INSTALL_COMPLETE); + installer.sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_COMPLETE); break; } // Fallback on N for https://gitlab.com/fdroid/fdroidclient/issues/631 if ("N".equals(Build.VERSION.CODENAME)) { - installer.sendBroadcastInstall(installUri, installOriginatingUri, - Installer.ACTION_INSTALL_COMPLETE); + installer.sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_COMPLETE); break; } switch (resultCode) { case Activity.RESULT_OK: - installer.sendBroadcastInstall(installUri, installOriginatingUri, + installer.sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_COMPLETE); break; case Activity.RESULT_CANCELED: - installer.sendBroadcastInstall(installUri, installOriginatingUri, + installer.sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED); break; case Activity.RESULT_FIRST_USER: default: // AOSP returns Activity.RESULT_FIRST_USER on error - installer.sendBroadcastInstall(installUri, installOriginatingUri, + installer.sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, getString(R.string.install_error_unknown)); break; diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java index 970b8f99f..adb61d16d 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java @@ -46,14 +46,13 @@ public class ExtensionInstaller extends Installer { } @Override - protected void installPackage(Uri uri, Uri originatingUri, String packageName) { + protected void installPackage(Uri localApkUri, Uri downloadUri, String packageName) { Uri sanitizedUri; try { - sanitizedUri = Installer.prepareApkFile(context, uri, packageName); + sanitizedUri = Installer.prepareApkFile(context, localApkUri, packageName); } catch (InstallFailedException e) { Log.e(TAG, "prepareApkFile failed", e); - sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_INTERRUPTED, - e.getMessage()); + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, e.getMessage()); return; } @@ -61,7 +60,7 @@ public class ExtensionInstaller extends Installer { // NOTE: Disabled for debug builds to be able to use official extension from repo ApkSignatureVerifier signatureVerifier = new ApkSignatureVerifier(context); if (!BuildConfig.DEBUG && !signatureVerifier.hasFDroidSignature(new File(sanitizedUri.getPath()))) { - sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_INTERRUPTED, + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, "APK signature of extension not correct!"); } Intent installIntent = new Intent(context, InstallExtensionDialogActivity.class); @@ -70,15 +69,15 @@ public class ExtensionInstaller extends Installer { PendingIntent installPendingIntent = PendingIntent.getActivity( context.getApplicationContext(), - uri.hashCode(), + localApkUri.hashCode(), installIntent, PendingIntent.FLAG_UPDATE_CURRENT); - sendBroadcastInstall(uri, originatingUri, + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_USER_INTERACTION, installPendingIntent); // don't use broadcasts for the rest of this special installer - sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_COMPLETE); + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_COMPLETE); } @Override 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 9903ad188..4936ccbad 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -55,8 +55,10 @@ import java.util.Set; *
  • for a {@link Uri} ID, use {@code Uri}, {@link Intent#getData()} *
  • for a {@code String} ID, use {@code urlString}, {@link Uri#toString()}, or * {@link Intent#getDataString()} - *
  • for an {@code int} ID, use {@link String#hashCode()} + *
  • for an {@code int} ID, use {@link String#hashCode()} or {@link Uri#hashCode()} *

    + * The implementations of {@link Uri#toString()} and {@link Intent#getDataString()} both + * include caching of the generated {@code String}, so it should be plenty fast. */ public class InstallManagerService extends Service { public static final String TAG = "InstallManagerService"; @@ -154,7 +156,7 @@ public class InstallManagerService extends Service { Apk apk = new Apk(intent.getParcelableExtra(EXTRA_APK)); addToActive(urlString, app, apk); - NotificationCompat.Builder builder = createNotificationBuilder(intent.getDataString(), apk); + NotificationCompat.Builder builder = createNotificationBuilder(urlString, apk); notificationManager.notify(urlString.hashCode(), builder.build()); registerDownloaderReceivers(urlString, builder); @@ -224,18 +226,18 @@ public class InstallManagerService extends Service { @Override public void onReceive(Context context, Intent intent) { // elsewhere called urlString - Uri originatingUri = intent.getData(); + Uri downloadUri = intent.getData(); + String urlString = downloadUri.toString(); File localFile = new File(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH)); - Uri localUri = Uri.fromFile(localFile); + Uri localApkUri = Uri.fromFile(localFile); - Utils.debugLog(TAG, "download completed of " + originatingUri - + " to " + localUri); + Utils.debugLog(TAG, "download completed of " + urlString + " to " + localApkUri); - unregisterDownloaderReceivers(intent.getDataString()); + unregisterDownloaderReceivers(urlString); + registerInstallerReceivers(downloadUri); - registerInstallerReceivers(localUri); - Apk apk = ACTIVE_APKS.get(originatingUri.toString()); - InstallerService.install(context, localUri, originatingUri, apk.packageName); + Apk apk = ACTIVE_APKS.get(urlString); + InstallerService.install(context, localApkUri, downloadUri, apk.packageName); } }; BroadcastReceiver interruptedReceiver = new BroadcastReceiver() { @@ -262,19 +264,18 @@ public class InstallManagerService extends Service { } - private void registerInstallerReceivers(Uri uri) { + private void registerInstallerReceivers(Uri downloadUri) { BroadcastReceiver installReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - Uri originatingUri = intent.getParcelableExtra(Installer.EXTRA_ORIGINATING_URI); - + String downloadUrl = intent.getDataString(); switch (intent.getAction()) { case Installer.ACTION_INSTALL_STARTED: // nothing to do break; case Installer.ACTION_INSTALL_COMPLETE: - Apk apkComplete = removeFromActive(originatingUri.toString()); + Apk apkComplete = removeFromActive(downloadUrl); PackageManagerCompat.setInstaller(getPackageManager(), apkComplete.packageName); @@ -286,16 +287,16 @@ public class InstallManagerService extends Service { // show notification if app details is not visible if (!TextUtils.isEmpty(errorMessage)) { - App app = getAppFromActive(originatingUri.toString()); + App app = getAppFromActive(downloadUrl); String title = String.format( getString(R.string.install_error_notify_title), app.name); // show notification if app details is not visible if (AppDetails.isAppVisible(app.packageName)) { - cancelNotification(originatingUri.toString()); + cancelNotification(downloadUrl); } else { - notifyError(originatingUri.toString(), title, errorMessage); + notifyError(downloadUrl, title, errorMessage); } } @@ -305,12 +306,12 @@ public class InstallManagerService extends Service { PendingIntent installPendingIntent = intent.getParcelableExtra(Installer.EXTRA_USER_INTERACTION_PI); - Apk apkUserInteraction = getApkFromActive(originatingUri.toString()); + Apk apkUserInteraction = getApkFromActive(downloadUrl); // show notification if app details is not visible if (AppDetails.isAppVisible(apkUserInteraction.packageName)) { - cancelNotification(originatingUri.toString()); + cancelNotification(downloadUrl); } else { - notifyDownloadComplete(apkUserInteraction, originatingUri.toString(), installPendingIntent); + notifyDownloadComplete(apkUserInteraction, downloadUrl, installPendingIntent); } break; @@ -321,7 +322,7 @@ public class InstallManagerService extends Service { }; localBroadcastManager.registerReceiver(installReceiver, - Installer.getInstallIntentFilter(uri)); + Installer.getInstallIntentFilter(downloadUri)); } private NotificationCompat.Builder createNotificationBuilder(String urlString, Apk apk) { 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 b1ed0ff13..1d4ae6e68 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -64,10 +64,14 @@ public abstract class Installer { public static final String ACTION_UNINSTALL_USER_INTERACTION = "org.fdroid.fdroid.installer.Installer.action.UNINSTALL_USER_INTERACTION"; /** - * Same as http://developer.android.com/reference/android/content/Intent.html#EXTRA_ORIGINATING_URI - * In InstallManagerService often called urlString + * The URI where the APK was originally downloaded from. This is also used + * as the unique ID representing this in the whole install process in + * {@link InstallManagerService}, there is is generally known as the + * "download URL" since it is the URL used to download the APK. + * + * @see Intent#EXTRA_ORIGINATING_URI */ - public static final String EXTRA_ORIGINATING_URI = "org.fdroid.fdroid.installer.Installer.extra.ORIGINATING_URI"; + static final String EXTRA_DOWNLOAD_URI = "org.fdroid.fdroid.installer.Installer.extra.DOWNLOAD_URI"; public static final String EXTRA_PACKAGE_NAME = "org.fdroid.fdroid.installer.Installer.extra.PACKAGE_NAME"; public static final String EXTRA_USER_INTERACTION_PI = "org.fdroid.fdroid.installer.Installer.extra.USER_INTERACTION_PI"; public static final String EXTRA_ERROR_MESSAGE = "org.fdroid.fdroid.net.installer.Installer.extra.ERROR_MESSAGE"; @@ -238,24 +242,23 @@ public abstract class Installer { return hasher.match(hash); } - public void sendBroadcastInstall(Uri uri, Uri originatingUri, String action, + public void sendBroadcastInstall(Uri downloadUri, String action, PendingIntent pendingIntent) { - sendBroadcastInstall(uri, originatingUri, action, pendingIntent, null); + sendBroadcastInstall(downloadUri, action, pendingIntent, null); } - public void sendBroadcastInstall(Uri uri, Uri originatingUri, String action) { - sendBroadcastInstall(uri, originatingUri, action, null, null); + public void sendBroadcastInstall(Uri downloadUri, String action) { + sendBroadcastInstall(downloadUri, action, null, null); } - public void sendBroadcastInstall(Uri uri, Uri originatingUri, String action, String errorMessage) { - sendBroadcastInstall(uri, originatingUri, action, null, errorMessage); + public void sendBroadcastInstall(Uri downloadUri, String action, String errorMessage) { + sendBroadcastInstall(downloadUri, action, null, errorMessage); } - public void sendBroadcastInstall(Uri uri, Uri originatingUri, String action, + public void sendBroadcastInstall(Uri downloadUri, String action, PendingIntent pendingIntent, String errorMessage) { Intent intent = new Intent(action); - intent.setData(uri); - intent.putExtra(Installer.EXTRA_ORIGINATING_URI, originatingUri); + intent.setData(downloadUri); intent.putExtra(Installer.EXTRA_USER_INTERACTION_PI, pendingIntent); if (!TextUtils.isEmpty(errorMessage)) { intent.putExtra(Installer.EXTRA_ERROR_MESSAGE, errorMessage); @@ -312,10 +315,20 @@ public abstract class Installer { return intentFilter; } - protected abstract void installPackage(Uri uri, Uri originatingUri, String packageName); + /** + * @param localApkUri points to the local copy of the APK to be installed + * @param downloadUri serves as the unique ID for all actions related to the + * installation of that specific APK + * @param packageName package name of the app that should be installed + */ + protected abstract void installPackage(Uri localApkUri, Uri downloadUri, String packageName); protected abstract void uninstallPackage(String packageName); + /** + * This {@link Installer} instance is capable of "unattended" install and + * uninstall activities, without the system enforcing a user prompt. + */ protected abstract boolean isUnattended(); } diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java index 1af566fa3..7e08806c2 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java @@ -33,6 +33,10 @@ import android.net.Uri; * i.e., runs sequentially * - no cancel operation is needed. Cancelling an installation * would be the same as starting uninstall afterwards + *

    + * The download URL is only used as the unique ID that represents this + * particular apk throughout the whole install process in + * {@link InstallManagerService}. */ public class InstallerService extends IntentService { @@ -50,9 +54,8 @@ public class InstallerService extends IntentService { if (ACTION_INSTALL.equals(intent.getAction())) { Uri uri = intent.getData(); - Uri originatingUri = intent.getParcelableExtra(Installer.EXTRA_ORIGINATING_URI); - - installer.installPackage(uri, originatingUri, packageName); + Uri downloadUri = intent.getParcelableExtra(Installer.EXTRA_DOWNLOAD_URI); + installer.installPackage(uri, downloadUri, packageName); } else if (ACTION_UNINSTALL.equals(intent.getAction())) { installer.uninstallPackage(packageName); } @@ -61,16 +64,16 @@ public class InstallerService extends IntentService { /** * Install an apk from {@link Uri} * - * @param context this app's {@link Context} - * @param uri {@link Uri} pointing to (downloaded) local apk file - * @param originatingUri {@link Uri} where the apk has been downloaded from - * @param packageName package name of the app that should be installed + * @param context this app's {@link Context} + * @param localApkUri {@link Uri} pointing to (downloaded) local apk file + * @param downloadUri {@link Uri} where the apk has been downloaded from + * @param packageName package name of the app that should be installed */ - public static void install(Context context, Uri uri, Uri originatingUri, String packageName) { + public static void install(Context context, Uri localApkUri, Uri downloadUri, String packageName) { Intent intent = new Intent(context, InstallerService.class); intent.setAction(ACTION_INSTALL); - intent.setData(uri); - intent.putExtra(Installer.EXTRA_ORIGINATING_URI, originatingUri); + intent.setData(localApkUri); + intent.putExtra(Installer.EXTRA_DOWNLOAD_URI, downloadUri); intent.putExtra(Installer.EXTRA_PACKAGE_NAME, packageName); context.startService(intent); } diff --git a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java index 24a4001ff..66bf38e88 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java @@ -297,15 +297,15 @@ public class PrivilegedInstaller extends Installer { } @Override - protected void installPackage(final Uri uri, final Uri originatingUri, String packageName) { - sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_STARTED); + protected void installPackage(final Uri localApkUri, final Uri downloadUri, String packageName) { + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_STARTED); final Uri sanitizedUri; try { - sanitizedUri = Installer.prepareApkFile(context, uri, packageName); + sanitizedUri = Installer.prepareApkFile(context, localApkUri, packageName); } catch (Installer.InstallFailedException e) { Log.e(TAG, "prepareApkFile failed", e); - sendBroadcastInstall(uri, originatingUri, Installer.ACTION_INSTALL_INTERRUPTED, + sendBroadcastInstall(downloadUri, Installer.ACTION_INSTALL_INTERRUPTED, e.getMessage()); return; } @@ -318,9 +318,9 @@ public class PrivilegedInstaller extends Installer { @Override public void handleResult(String packageName, int returnCode) throws RemoteException { if (returnCode == INSTALL_SUCCEEDED) { - sendBroadcastInstall(uri, originatingUri, ACTION_INSTALL_COMPLETE); + sendBroadcastInstall(downloadUri, ACTION_INSTALL_COMPLETE); } else { - sendBroadcastInstall(uri, originatingUri, ACTION_INSTALL_INTERRUPTED, + sendBroadcastInstall(downloadUri, ACTION_INSTALL_INTERRUPTED, "Error " + returnCode + ": " + INSTALL_RETURN_CODES.get(returnCode)); } @@ -330,7 +330,7 @@ public class PrivilegedInstaller extends Installer { try { boolean hasPermissions = privService.hasPrivilegedPermissions(); if (!hasPermissions) { - sendBroadcastInstall(uri, originatingUri, ACTION_INSTALL_INTERRUPTED, + sendBroadcastInstall(downloadUri, ACTION_INSTALL_INTERRUPTED, context.getString(R.string.system_install_denied_permissions)); return; } @@ -339,7 +339,7 @@ public class PrivilegedInstaller extends Installer { null, callback); } catch (RemoteException e) { Log.e(TAG, "RemoteException", e); - sendBroadcastInstall(uri, originatingUri, ACTION_INSTALL_INTERRUPTED, + sendBroadcastInstall(downloadUri, ACTION_INSTALL_INTERRUPTED, "connecting to privileged service failed"); } } diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java index f6a81b9c8..371d53350 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java @@ -791,7 +791,7 @@ public class SwapWorkflowActivity extends AppCompatActivity { Uri localUri = Uri.fromFile(apkFile); localBroadcastManager.registerReceiver(installReceiver, - Installer.getInstallIntentFilter(Uri.fromFile(apkFile))); + Installer.getInstallIntentFilter(originatingUri)); InstallerService.install(this, localUri, originatingUri, packageName); } From f949c5807f5475c9701f1cc6bea470473b1eeeb0 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 1 Jun 2016 20:57:00 +0200 Subject: [PATCH 2/6] make swap install via InstallManagerService Somehow, it ended up that there was a partial reimplementation of the install in SwapWorkflowActivity. --- .../views/swap/SwapWorkflowActivity.java | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java index 371d53350..72a8eca54 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java @@ -44,14 +44,10 @@ import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.NewRepoConfig; import org.fdroid.fdroid.installer.InstallManagerService; import org.fdroid.fdroid.installer.Installer; -import org.fdroid.fdroid.installer.InstallerService; import org.fdroid.fdroid.localrepo.LocalRepoManager; import org.fdroid.fdroid.localrepo.SwapService; import org.fdroid.fdroid.localrepo.peers.Peer; -import org.fdroid.fdroid.net.Downloader; -import org.fdroid.fdroid.net.DownloaderService; -import java.io.File; import java.util.Arrays; import java.util.Date; import java.util.HashMap; @@ -773,26 +769,10 @@ public class SwapWorkflowActivity extends AppCompatActivity { public void install(@NonNull final App app) { final Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode); - String urlString = apk.getUrl(); - BroadcastReceiver downloadCompleteReceiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - String path = intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH); - handleDownloadComplete(new File(path), app.packageName, intent.getDataString()); - } - }; - localBroadcastManager.registerReceiver(downloadCompleteReceiver, - DownloaderService.getIntentFilter(urlString, Downloader.ACTION_COMPLETE)); - InstallManagerService.queue(this, app, apk); - } - - private void handleDownloadComplete(File apkFile, String packageName, String urlString) { - Uri originatingUri = Uri.parse(urlString); - Uri localUri = Uri.fromFile(apkFile); - + Uri downloadUri = Uri.parse(apk.getUrl()); localBroadcastManager.registerReceiver(installReceiver, - Installer.getInstallIntentFilter(originatingUri)); - InstallerService.install(this, localUri, originatingUri, packageName); + Installer.getInstallIntentFilter(downloadUri)); + InstallManagerService.queue(this, app, apk); } private final BroadcastReceiver installReceiver = new BroadcastReceiver() { From 738a92f5d251b066cb8b7ecb80314f54bd68a8ec Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 1 Jun 2016 21:14:57 +0200 Subject: [PATCH 3/6] reduce visibility of installer extra constants This should encourage devs to stick with the nice static methods that are the "public" API. --- .../installer/DefaultInstallerActivity.java | 6 +++--- .../fdroid/fdroid/installer/Installer.java | 20 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java index e4727ee8b..06e7442d2 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstallerActivity.java @@ -38,10 +38,10 @@ import org.fdroid.fdroid.R; public class DefaultInstallerActivity extends FragmentActivity { public static final String TAG = "AndroidInstallerAct"; - public static final String ACTION_INSTALL_PACKAGE = "org.fdroid.fdroid.INSTALL_PACKAGE"; - public static final String ACTION_UNINSTALL_PACKAGE = "org.fdroid.fdroid.UNINSTALL_PACKAGE"; + static final String ACTION_INSTALL_PACKAGE = "org.fdroid.fdroid.installer.DefaultInstaller.action.INSTALL_PACKAGE"; + static final String ACTION_UNINSTALL_PACKAGE = "org.fdroid.fdroid.installer.DefaultInstaller.action.UNINSTALL_PACKAGE"; - public static final String EXTRA_UNINSTALL_PACKAGE_NAME = "uninstallPackageName"; + static final String EXTRA_UNINSTALL_PACKAGE_NAME = "org.fdroid.fdroid.installer.DefaultInstaller.extra.UNINSTALL_PACKAGE_NAME"; private static final int REQUEST_CODE_INSTALL = 0; private static final int REQUEST_CODE_UNINSTALL = 1; 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 1d4ae6e68..1d0671c2a 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -95,7 +95,7 @@ public abstract class Installer { localBroadcastManager = LocalBroadcastManager.getInstance(context); } - public static Uri prepareApkFile(Context context, Uri uri, String packageName) + static Uri prepareApkFile(Context context, Uri uri, String packageName) throws InstallFailedException { File apkFile = new File(uri.getPath()); @@ -233,7 +233,7 @@ public abstract class Installer { /** * Checks the APK file against the provided hash, returning whether it is a match. */ - public static boolean verifyApkFile(File apkFile, String hash, String hashType) + static boolean verifyApkFile(File apkFile, String hash, String hashType) throws NoSuchAlgorithmException { if (!apkFile.exists()) { return false; @@ -242,20 +242,20 @@ public abstract class Installer { return hasher.match(hash); } - public void sendBroadcastInstall(Uri downloadUri, String action, + void sendBroadcastInstall(Uri downloadUri, String action, PendingIntent pendingIntent) { sendBroadcastInstall(downloadUri, action, pendingIntent, null); } - public void sendBroadcastInstall(Uri downloadUri, String action) { + void sendBroadcastInstall(Uri downloadUri, String action) { sendBroadcastInstall(downloadUri, action, null, null); } - public void sendBroadcastInstall(Uri downloadUri, String action, String errorMessage) { + void sendBroadcastInstall(Uri downloadUri, String action, String errorMessage) { sendBroadcastInstall(downloadUri, action, null, errorMessage); } - public void sendBroadcastInstall(Uri downloadUri, String action, + void sendBroadcastInstall(Uri downloadUri, String action, PendingIntent pendingIntent, String errorMessage) { Intent intent = new Intent(action); intent.setData(downloadUri); @@ -266,20 +266,20 @@ public abstract class Installer { localBroadcastManager.sendBroadcast(intent); } - public void sendBroadcastUninstall(String packageName, String action, String errorMessage) { + void sendBroadcastUninstall(String packageName, String action, String errorMessage) { sendBroadcastUninstall(packageName, action, null, errorMessage); } - public void sendBroadcastUninstall(String packageName, String action) { + void sendBroadcastUninstall(String packageName, String action) { sendBroadcastUninstall(packageName, action, null, null); } - public void sendBroadcastUninstall(String packageName, String action, + void sendBroadcastUninstall(String packageName, String action, PendingIntent pendingIntent) { sendBroadcastUninstall(packageName, action, pendingIntent, null); } - public void sendBroadcastUninstall(String packageName, String action, + void sendBroadcastUninstall(String packageName, String action, PendingIntent pendingIntent, String errorMessage) { Uri uri = Uri.fromParts("package", packageName, null); From 07cadd862a7cea8edc85828a24a0500e0e5a7d91 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 1 Jun 2016 22:00:48 +0200 Subject: [PATCH 4/6] Installer IntentFilters must also match on host and port Without this rule, two https:// URLs with the same path and APK name would both match. With multiple repo and swap support, this could easily happen. --- app/src/main/java/org/fdroid/fdroid/installer/Installer.java | 1 + 1 file changed, 1 insertion(+) 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 1d0671c2a..ed7e49864 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -300,6 +300,7 @@ public abstract class Installer { intentFilter.addAction(Installer.ACTION_INSTALL_INTERRUPTED); intentFilter.addAction(Installer.ACTION_INSTALL_USER_INTERACTION); intentFilter.addDataScheme(uri.getScheme()); + intentFilter.addDataAuthority(uri.getHost(), String.valueOf(uri.getPort())); intentFilter.addDataPath(uri.getPath(), PatternMatcher.PATTERN_LITERAL); return intentFilter; } From e95dec30b5e7e2fa0e63f4cf2aac07846d317388 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 1 Jun 2016 22:04:57 +0200 Subject: [PATCH 5/6] remove APK from active list if install process is interrupted If the install process is interrupted, then InstallManagerService is no longer managing it. It will make the announcements and set the notification, then forget about that APK. --- .../fdroid/fdroid/installer/InstallManagerService.java | 8 ++++---- 1 file 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 4936ccbad..9494fdacf 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -288,18 +288,18 @@ public class InstallManagerService extends Service { // show notification if app details is not visible if (!TextUtils.isEmpty(errorMessage)) { App app = getAppFromActive(downloadUrl); - String title = String.format( - getString(R.string.install_error_notify_title), - app.name); // show notification if app details is not visible if (AppDetails.isAppVisible(app.packageName)) { cancelNotification(downloadUrl); } else { + String title = String.format( + getString(R.string.install_error_notify_title), + app.name); notifyError(downloadUrl, title, errorMessage); } } - + removeFromActive(downloadUrl); localBroadcastManager.unregisterReceiver(this); break; case Installer.ACTION_INSTALL_USER_INTERACTION: From 43e902407f27c8d4b9b03ab7e8a5d1a573eb6f81 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 1 Jun 2016 22:14:09 +0200 Subject: [PATCH 6/6] add some javadoc notes about the new installer stuff --- .../main/java/org/fdroid/fdroid/installer/Installer.java | 6 +++++- .../fdroid/privileged/views/UninstallDialogActivity.java | 7 +++++++ 2 files changed, 12 insertions(+), 1 deletion(-) 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 ed7e49864..9e67d9ef1 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -46,7 +46,7 @@ import java.security.NoSuchAlgorithmException; import java.util.Map; /** - * + * Handles the actual install process. Subclasses implement the details. */ public abstract class Installer { final Context context; @@ -293,6 +293,10 @@ public abstract class Installer { localBroadcastManager.sendBroadcast(intent); } + /** + * Gets an {@link IntentFilter} for matching events from the install + * process based on the original download URL as a {@link Uri}. + */ public static IntentFilter getInstallIntentFilter(Uri uri) { IntentFilter intentFilter = new IntentFilter(); intentFilter.addAction(Installer.ACTION_INSTALL_STARTED); diff --git a/app/src/main/java/org/fdroid/fdroid/privileged/views/UninstallDialogActivity.java b/app/src/main/java/org/fdroid/fdroid/privileged/views/UninstallDialogActivity.java index f0d2bd04c..fa2380719 100644 --- a/app/src/main/java/org/fdroid/fdroid/privileged/views/UninstallDialogActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/privileged/views/UninstallDialogActivity.java @@ -34,6 +34,13 @@ import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.R; import org.fdroid.fdroid.installer.Installer; +/** + * This class provides the confirmation prompt for when the user chooses to + * uninstall an app. This has to be implemented here for the privileged + * extension, it is only shown for {@link Installer} instances that can do + * installs and uninstalls without user prompts, which is detected via + * {@link Installer#isUnattended()}. + */ public class UninstallDialogActivity extends FragmentActivity { @Override