From 3f9370371b5fa8c2c8354185d39e4c83360497b0 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 16 Jun 2016 16:24:22 +1000 Subject: [PATCH] Make sure to query package manager for `PackageInfo` when required. A previous commit accidentally pushed the code which queries the `PackageManager` to a different method, but then still used the `packageInfo` which was supposed to be populated by that code. This change rectifies this, and in the process also clarifies/documents under what circumstances the `PackageManager` needs to be queried, rather than relying on the incoming intent. Fixes #686. --- .../data/InstalledAppProviderService.java | 50 +++++++++++++------ .../fdroid/data/InstalledAppTestUtils.java | 2 +- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java index 3c54f5519..da9db1bb2 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java +++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java @@ -9,6 +9,7 @@ import android.content.pm.PackageManager; import android.content.pm.Signature; import android.net.Uri; import android.os.Process; +import android.support.annotation.Nullable; import org.fdroid.fdroid.Hasher; import org.fdroid.fdroid.Utils; @@ -32,7 +33,7 @@ import rx.subjects.PublishSubject; * Since {@link android.content.ContentProvider#insert(Uri, ContentValues)} does not check * for duplicate records, it is entirely the job of this service to ensure that it is not * inserting duplicate versions of the same installed APK. On that note, - * {@link #insertAppIntoDb(Context, String, PackageInfo)} and + * {@link #insertAppIntoDb(Context, PackageInfo, String, String)} and * {@link #deleteAppFromDb(Context, String)} are both static methods to enable easy testing * of this stuff. */ @@ -156,10 +157,12 @@ public class InstalledAppProviderService extends IntentService { String packageName = intent.getData().getSchemeSpecificPart(); final String action = intent.getAction(); if (ACTION_INSERT.equals(action)) { - PackageInfo packageInfo = intent.getParcelableExtra(EXTRA_PACKAGE_INFO); - String hashType = "sha256"; - String hash = Utils.getBinaryHash(new File(packageInfo.applicationInfo.publicSourceDir), hashType); - insertAppIntoDb(this, packageName, packageInfo, hashType, hash); + PackageInfo packageInfo = getPackageInfo(intent, packageName); + if (packageInfo != null) { + String hashType = "sha256"; + String hash = Utils.getBinaryHash(new File(packageInfo.applicationInfo.publicSourceDir), hashType); + insertAppIntoDb(this, packageInfo, hashType, hash); + } } else if (ACTION_DELETE.equals(action)) { deleteAppFromDb(this, packageName); } @@ -167,23 +170,38 @@ public class InstalledAppProviderService extends IntentService { } } + /** + * This class will either have received an intent from the {@link InstalledAppProviderService} + * itself, while iterating over installed apps, or from a {@link Intent#ACTION_PACKAGE_ADDED} + * broadcast. In the first case, it will already have a {@link PackageInfo} for us. However if + * it is from the later case, we'll need to query the {@link PackageManager} ourselves to get + * this info. + * + * Can still return null, as there is potentially race conditions to do with uninstalling apps + * such that querying the {@link PackageManager} for a given package may throw an exception. + */ + @Nullable + private PackageInfo getPackageInfo(Intent intent, String packageName) { + PackageInfo packageInfo = intent.getParcelableExtra(EXTRA_PACKAGE_INFO); + if (packageInfo != null) { + return packageInfo; + } + + try { + return getPackageManager().getPackageInfo(packageName, PackageManager.GET_SIGNATURES); + } catch (PackageManager.NameNotFoundException e) { + e.printStackTrace(); + return null; + } + } + /** * @param hash Although the has could be calculated within this function, it is helpful to inject * the hash so as to be able to use this method during testing. Otherwise, the * hashing method will try to hash a non-existent .apk file and try to insert NULL * into the database when under test. */ - static void insertAppIntoDb(Context context, String packageName, PackageInfo packageInfo, String hashType, String hash) { - if (packageInfo == null) { - try { - packageInfo = context.getPackageManager().getPackageInfo(packageName, - PackageManager.GET_SIGNATURES); - } catch (PackageManager.NameNotFoundException e) { - e.printStackTrace(); - return; - } - } - + static void insertAppIntoDb(Context context, PackageInfo packageInfo, String hashType, String hash) { Uri uri = InstalledAppProvider.getContentUri(); ContentValues contentValues = new ContentValues(); contentValues.put(InstalledAppProvider.DataColumns.PACKAGE_NAME, packageInfo.packageName); diff --git a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java index 51fd6e905..332adc687 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java +++ b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java @@ -21,7 +21,7 @@ public class InstalledAppTestUtils { info.applicationInfo.publicSourceDir = "/tmp/mock-location"; String hashType = "sha256"; String hash = "00112233445566778899aabbccddeeff"; - InstalledAppProviderService.insertAppIntoDb(context, packageName, info, hashType, hash); + InstalledAppProviderService.insertAppIntoDb(context, info, hashType, hash); } }