From 8600ce8d8a56398a4eb731f0cccb848c4e18d2eb Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 1 Feb 2018 14:37:29 +0100 Subject: [PATCH 1/6] prevent crashes from update notifications on < android-11 closes #1306 * https://stackoverflow.com/questions/3112008/android-java-lang-illegalargumentexception-contentintent-required-error-cause * https://stackoverflow.com/questions/20032249/is-setcontentintentpendingintent-required-in-notificationcompat-builder --- app/src/main/java/org/fdroid/fdroid/NotificationHelper.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java index 00ce60cbe..1f5fa61b5 100644 --- a/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/NotificationHelper.java @@ -433,6 +433,12 @@ class NotificationHelper { intentDeleted.setClass(context, NotificationBroadcastReceiver.class); PendingIntent piDeleted = PendingIntent.getBroadcast(context, 0, intentDeleted, PendingIntent.FLAG_UPDATE_CURRENT); builder.setDeleteIntent(piDeleted); + + if (Build.VERSION.SDK_INT < 11) { + Intent intent = new Intent(); + intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + builder.setContentIntent(PendingIntent.getActivity(context, 0, intent, 0)); + } return builder.build(); } From 532d1dfc72cfc0f027c888c47f05af4d2c166879 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 8 Feb 2018 23:52:23 +0100 Subject: [PATCH 2/6] 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) { From 8a0abdd841459aae8a082703f078ab608683ec86 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 9 Feb 2018 09:41:43 +0100 Subject: [PATCH 3/6] AppDetails2 run style formatter and fix line length issues --- .../java/org/fdroid/fdroid/AppDetails2.java | 79 +++++++++++-------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java index 92ad8d14c..352e9fb2f 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails2.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails2.java @@ -48,10 +48,8 @@ import android.view.Menu; import android.view.MenuItem; import android.view.ViewTreeObserver; import android.widget.Toast; - import com.nostra13.universalimageloader.core.DisplayImageOptions; import com.nostra13.universalimageloader.core.ImageLoader; - import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; @@ -69,8 +67,9 @@ import org.fdroid.fdroid.views.apps.FeatureImage; import java.util.Iterator; -@SuppressWarnings("LineLength") -public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog.ShareChooserDialogListener, AppDetailsRecyclerViewAdapter.AppDetailsRecyclerViewAdapterCallbacks { +public class AppDetails2 extends AppCompatActivity + implements ShareChooserDialog.ShareChooserDialogListener, + AppDetailsRecyclerViewAdapter.AppDetailsRecyclerViewAdapterCallbacks { public static final String EXTRA_APPID = "appid"; private static final String TAG = "AppDetails2"; @@ -121,7 +120,8 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog appBarLayout = (AppBarLayout) coordinatorLayout.findViewById(R.id.app_bar); recyclerView = (RecyclerView) findViewById(R.id.rvDetails); adapter = new AppDetailsRecyclerViewAdapter(this, app, this); - OverscrollLinearLayoutManager lm = new OverscrollLinearLayoutManager(this, LinearLayoutManager.VERTICAL, false); + OverscrollLinearLayoutManager lm = new OverscrollLinearLayoutManager(this, + LinearLayoutManager.VERTICAL, false); lm.setStackFromEnd(false); // Has to be invoked after AppDetailsRecyclerViewAdapter is created. @@ -146,10 +146,12 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog public int onOverscrollY(int overscroll) { int consumed = 0; if (overscroll < 0) { - CoordinatorLayout.LayoutParams lp = (CoordinatorLayout.LayoutParams) appBarLayout.getLayoutParams(); + CoordinatorLayout.LayoutParams lp = (CoordinatorLayout.LayoutParams) + appBarLayout.getLayoutParams(); CoordinatorLayout.Behavior behavior = lp.getBehavior(); if (behavior != null && behavior instanceof AppBarLayout.Behavior) { - ((AppBarLayout.Behavior) behavior).onNestedScroll(coordinatorLayout, appBarLayout, recyclerView, 0, 0, 0, overscroll); + ((AppBarLayout.Behavior) behavior).onNestedScroll( + coordinatorLayout, appBarLayout, recyclerView, 0, 0, 0, overscroll); consumed = overscroll; // Consume all of it! } } @@ -173,7 +175,8 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog final FeatureImage featureImage = (FeatureImage) findViewById(R.id.feature_graphic); DisplayImageOptions displayImageOptions = Utils.getImageLoadingOptions().build(); String featureGraphicUrl = app.getFeatureGraphicUrl(this); - featureImage.loadImageAndDisplay(ImageLoader.getInstance(), displayImageOptions, featureGraphicUrl, app.iconUrl); + featureImage.loadImageAndDisplay(ImageLoader.getInstance(), displayImageOptions, + featureGraphicUrl, app.iconUrl); } private String getPackageNameFromIntent(Intent intent) { @@ -197,26 +200,27 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog app = newApp; // Remove all "installed" statuses for this app, since we are now viewing it. - AppUpdateStatusManager appUpdateStatusManager = AppUpdateStatusManager.getInstance(this); - for (AppUpdateStatusManager.AppUpdateStatus status : appUpdateStatusManager.getByPackageName(app.packageName)) { + AppUpdateStatusManager ausm = AppUpdateStatusManager.getInstance(this); + for (AppUpdateStatusManager.AppUpdateStatus status : ausm.getByPackageName(app.packageName)) { if (status.status == AppUpdateStatusManager.Status.Installed) { - appUpdateStatusManager.removeApk(status.getUniqueKey()); + ausm.removeApk(status.getUniqueKey()); } } } /** - * Some notifications (like "downloading" and "installed") are not shown for this app if it is open in app details. - * When closing, we need to refresh the notifications, so they are displayed again. + * Some notifications (like "downloading" and "installed") are not shown + * for this app if it is open in app details. When closing, we need to + * refresh the notifications, so they are displayed again. */ private void updateNotificationsForApp() { if (app != null) { - AppUpdateStatusManager appUpdateStatusManager = AppUpdateStatusManager.getInstance(this); - for (AppUpdateStatusManager.AppUpdateStatus status : appUpdateStatusManager.getByPackageName(app.packageName)) { + AppUpdateStatusManager ausm = AppUpdateStatusManager.getInstance(this); + for (AppUpdateStatusManager.AppUpdateStatus status : ausm.getByPackageName(app.packageName)) { if (status.status == AppUpdateStatusManager.Status.Installed) { - appUpdateStatusManager.removeApk(status.getUniqueKey()); + ausm.removeApk(status.getUniqueKey()); } else { - appUpdateStatusManager.refreshApk(status.getUniqueKey()); + ausm.refreshApk(status.getUniqueKey()); } } } @@ -245,7 +249,8 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog * Then, asks the view to update itself to reflect this status. */ private void refreshStatus() { - Iterator statuses = AppUpdateStatusManager.getInstance(this).getByPackageName(app.packageName).iterator(); + AppUpdateStatusManager ausm = AppUpdateStatusManager.getInstance(this); + Iterator statuses = ausm.getByPackageName(app.packageName).iterator(); if (statuses.hasNext()) { AppUpdateStatusManager.AppUpdateStatus status = statuses.next(); updateAppStatus(status, false); @@ -304,10 +309,12 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog Intent shareIntent = new Intent(Intent.ACTION_SEND); shareIntent.setType("text/plain"); shareIntent.putExtra(Intent.EXTRA_SUBJECT, app.name); - shareIntent.putExtra(Intent.EXTRA_TEXT, app.name + " (" + app.summary + ") - https://f-droid.org/app/" + app.packageName); + shareIntent.putExtra(Intent.EXTRA_TEXT, app.name + " (" + app.summary + + ") - https://f-droid.org/app/" + app.packageName); boolean showNearbyItem = app.isInstalled(getApplicationContext()) && fdroidApp.bluetoothAdapter != null; - ShareChooserDialog.createChooser((CoordinatorLayout) findViewById(R.id.rootCoordinator), this, this, shareIntent, showNearbyItem); + CoordinatorLayout coordinatorLayout = (CoordinatorLayout) findViewById(R.id.rootCoordinator); + ShareChooserDialog.createChooser(coordinatorLayout, this, this, shareIntent, showNearbyItem); return true; } else if (item.getItemId() == R.id.action_ignore_all) { app.getPrefs(this).ignoreAllUpdates ^= true; @@ -420,7 +427,8 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog private void initiateInstall(Apk apk) { if (isAppDownloading()) { - Log.i(TAG, "Ignoring request to install " + apk.packageName + " version " + apk.versionName + ", as we are already downloading (either that or a different version)."); + Log.i(TAG, "Ignoring request to install " + apk.packageName + " version " + apk.versionName + + ", as we are already downloading (either that or a different version)."); return; } @@ -489,7 +497,8 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog case ReadyToInstall: if (justReceived) { adapter.clearProgress(); - localBroadcastManager.registerReceiver(installReceiver, Installer.getInstallIntentFilter(Uri.parse(newStatus.getUniqueKey()))); + localBroadcastManager.registerReceiver(installReceiver, + Installer.getInstallIntentFilter(Uri.parse(newStatus.getUniqueKey()))); } break; @@ -520,13 +529,19 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog private final BroadcastReceiver appStatusReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - AppUpdateStatusManager.AppUpdateStatus status = intent.getParcelableExtra(AppUpdateStatusManager.EXTRA_STATUS); + AppUpdateStatusManager.AppUpdateStatus status = intent.getParcelableExtra( + AppUpdateStatusManager.EXTRA_STATUS); - boolean isRemoving = TextUtils.equals(intent.getAction(), AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED); - if (currentStatus != null && isRemoving && !TextUtils.equals(status.getUniqueKey(), currentStatus.getUniqueKey())) { - Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + status.getUniqueKey() + " not " + currentStatus.getUniqueKey()); + boolean isRemoving = TextUtils.equals(intent.getAction(), + AppUpdateStatusManager.BROADCAST_APPSTATUS_REMOVED); + if (currentStatus != null + && isRemoving + && !TextUtils.equals(status.getUniqueKey(), currentStatus.getUniqueKey())) { + Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + + status.getUniqueKey() + " not " + currentStatus.getUniqueKey()); } else if (status != null && !TextUtils.equals(status.apk.packageName, app.packageName)) { - Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + status.apk.packageName + " not " + app.packageName); + Utils.debugLog(TAG, "Ignoring app status change because it belongs to " + + status.apk.packageName + " not " + app.packageName); } else { updateAppStatus(status, true); } @@ -583,15 +598,17 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog case Installer.ACTION_INSTALL_USER_INTERACTION: Apk apk = intent.getParcelableExtra(Installer.EXTRA_APK); if (!isAppVisible(apk.packageName)) { - Utils.debugLog(TAG, "Ignore request for user interaction from installer, because " + apk.packageName + " is no longer showing."); + Utils.debugLog(TAG, "Ignore request for user interaction from installer, because " + + apk.packageName + " is no longer showing."); break; } - Utils.debugLog(TAG, "Automatically showing package manager for " + apk.packageName + " as it is being viewed by the user."); - PendingIntent installPendingIntent = intent.getParcelableExtra(Installer.EXTRA_USER_INTERACTION_PI); + Utils.debugLog(TAG, "Automatically showing package manager for " + apk.packageName + + " as it is being viewed by the user."); + PendingIntent pendingIntent = intent.getParcelableExtra(Installer.EXTRA_USER_INTERACTION_PI); try { - installPendingIntent.send(); + pendingIntent.send(); } catch (PendingIntent.CanceledException e) { Log.e(TAG, "PI canceled", e); } From 833d3f40fdcd07f93c5d086703df39268b9483b3 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 9 Feb 2018 12:28:53 +0100 Subject: [PATCH 4/6] CleanCacheService: reduce logcat noise, check if file exists before rm --- app/src/main/java/org/fdroid/fdroid/CleanCacheService21.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/main/java/org/fdroid/fdroid/CleanCacheService21.java b/app/src/main/java/org/fdroid/fdroid/CleanCacheService21.java index 308d70c2a..66e3853bf 100644 --- a/app/src/main/java/org/fdroid/fdroid/CleanCacheService21.java +++ b/app/src/main/java/org/fdroid/fdroid/CleanCacheService21.java @@ -14,6 +14,9 @@ import java.io.File; @TargetApi(21) class CleanCacheService21 { static void deleteIfOld(File file, long olderThan) { + if (file == null || !file.exists()) { + return; + } try { StructStat stat = Os.lstat(file.getAbsolutePath()); if ((stat.st_atime * 1000L) < olderThan) { From 30b00156db7593cfaee19251b96bb742d0921510 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 13 Feb 2018 15:16:11 +0100 Subject: [PATCH 5/6] javadoc cleanup --- .../fdroid/fdroid/installer/PrivilegedInstaller.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) 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 d745dbe8a..389859587 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java @@ -55,12 +55,11 @@ import java.util.HashMap; *

* This installer makes unattended installs/uninstalls possible. * Thus no PendingIntents are returned. - *

- * Sources for Android 4.4 change: - * https://groups.google.com/forum/#!msg/android- - * security-discuss/r7uL_OEMU5c/LijNHvxeV80J - * https://android.googlesource.com/platform - * /frameworks/base/+/ccbf84f44c9e6a5ed3c08673614826bb237afc54 + * + * @see + * Sources for Android 4.4 change + * @see + * Commit that restricted "signatureOrSystem" permissions */ public class PrivilegedInstaller extends Installer { From 5c2e9305a8aad25e7621e1372546b352b971ee36 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 13 Feb 2018 16:58:17 +0100 Subject: [PATCH 6/6] only force index update when the locale actually changes This was forcing an index update on any config change, even just a simple screen rotation. Now it actually checks whether its needed. closes #1325 --- .../java/org/fdroid/fdroid/FDroidApp.java | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 2e899c75b..3306a32e9 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -285,7 +285,21 @@ public class FDroidApp extends Application { public void onConfigurationChanged(Configuration newConfig) { super.onConfigurationChanged(newConfig); Languages.setLanguage(this); - UpdateService.forceUpdateRepo(this); + + // update the descriptions based on the new language preferences + SharedPreferences atStartTime = getAtStartTimeSharedPreferences(); + final String lastLocaleKey = "lastLocale"; + String lastLocale = atStartTime.getString(lastLocaleKey, null); + String currentLocale; + if (Build.VERSION.SDK_INT < 24) { + currentLocale = newConfig.locale.toString(); + } else { + currentLocale = newConfig.getLocales().toString(); + } + if (!TextUtils.equals(lastLocale, currentLocale)) { + UpdateService.forceUpdateRepo(this); + } + atStartTime.edit().putString(lastLocaleKey, currentLocale).apply(); } @Override @@ -435,7 +449,7 @@ public class FDroidApp extends Application { Provisioner.scanAndProcess(getApplicationContext()); // if the underlying OS version has changed, then fully rebuild the database - SharedPreferences atStartTime = getSharedPreferences("at-start-time", Context.MODE_PRIVATE); + SharedPreferences atStartTime = getAtStartTimeSharedPreferences(); if (Build.VERSION.SDK_INT != atStartTime.getInt("build-version", Build.VERSION.SDK_INT)) { UpdateService.forceUpdateRepo(this); } @@ -483,6 +497,10 @@ public class FDroidApp extends Application { return ((BluetoothManager) getSystemService(BLUETOOTH_SERVICE)).getAdapter(); } + private SharedPreferences getAtStartTimeSharedPreferences() { + return getSharedPreferences("at-start-time", Context.MODE_PRIVATE); + } + public void sendViaBluetooth(Activity activity, int resultCode, String packageName) { if (resultCode == Activity.RESULT_CANCELED) { return;