From 972ef3b0786a93c0a3d5734bde0c6116885f6d0a Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 13 Apr 2016 12:20:06 -0400 Subject: [PATCH 1/6] remove unused arg from FileCompat.setReadable() Just trying to keep the code as close to what is actually used as possible. --- .../org/fdroid/fdroid/compat/FileCompat.java | 17 +++++++++-------- .../org/fdroid/fdroid/installer/Installer.java | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java b/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java index 2dc047785..44c6ae8cb 100644 --- a/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java +++ b/app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java @@ -85,20 +85,21 @@ public class FileCompat { } } + /** + * Set a {@link SanitizedFile} readable by all if {@code readable} is {@code true}. + * + * @return {@code true} if the operation succeeded + */ @TargetApi(9) - public static boolean setReadable(SanitizedFile file, boolean readable, boolean ownerOnly) { - + public static boolean setReadable(SanitizedFile file, boolean readable) { if (Build.VERSION.SDK_INT >= 9) { - return file.setReadable(readable, ownerOnly); + return file.setReadable(readable, false); } - String mode; if (readable) { - mode = ownerOnly ? "0600" : "0644"; + return setMode(file, "0644"); } else { - mode = "0000"; + return setMode(file, "0000"); } - return setMode(file, mode); - } private static boolean setMode(SanitizedFile file, String mode) { 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 cdbb39bc0..8a2e21ed6 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -222,7 +222,7 @@ public abstract class Installer { // have access is insecure, because apps with permission to write to the external // storage can overwrite the app between F-Droid asking for it to be installed and // the installer actually installing it. - FileCompat.setReadable(apkToInstall, true, false); + FileCompat.setReadable(apkToInstall, true); installPackageInternal(apkToInstall); NotificationManager nm = (NotificationManager) From 6c47ade3794f819308a3b2c741b7036d4102dc93 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 14 Apr 2016 17:02:44 -0400 Subject: [PATCH 2/6] move init sequence comments to javadoc comments By putting these comments into javadoc, they are directly describing the code where it is, and there are many tools in IDEs for searching, viewing, sorting, etc. javadoc comments. Plain comments do not have those tools. --- .../java/org/fdroid/fdroid/FDroidApp.java | 26 +++++-------------- .../java/org/fdroid/fdroid/Preferences.java | 3 +++ .../java/org/fdroid/fdroid/UpdateService.java | 8 +++--- .../fdroid/data/InstalledAppCacheUpdater.java | 9 +++++-- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index d086d8e3f..6ec2355b8 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -136,6 +136,9 @@ public class FDroidApp extends Application { } } + /** + * Initialize the settings needed to run a local swap repo. + */ public static void initWifiSettings() { port = 8888; ipAddressString = null; @@ -182,26 +185,14 @@ public class FDroidApp extends Application { updateLanguage(); ACRA.init(this); - // Needs to be setup before anything else tries to access it. - // Perhaps the constructor is a better place, but then again, - // it is more deterministic as to when this gets called... - Preferences.setup(this); - curTheme = Preferences.get().getTheme(); - - // Apply the Google PRNG fixes to properly seed SecureRandom PRNGFixes.apply(); - // Check that the installed app cache hasn't gotten out of sync somehow. - // e.g. if we crashed/ran out of battery half way through responding - // to a package installed intent. It doesn't really matter where - // we put this in the bootstrap process, because it runs on a different - // thread, which will be delayed by some seconds to avoid an error where - // the database is locked due to the database updater. - InstalledAppCacheUpdater.updateInBackground(getApplicationContext()); - - // make sure the current proxy stuff is configured + Preferences.setup(this); + curTheme = Preferences.get().getTheme(); Preferences.get().configureProxy(); + InstalledAppCacheUpdater.updateInBackground(getApplicationContext()); + // If the user changes the preference to do with filtering rooted apps, // it is easier to just notify a change in the app provider, // so that the newly updated list will correctly filter relevant apps. @@ -278,9 +269,6 @@ public class FDroidApp extends Application { .build(); ImageLoader.getInstance().init(config); - // TODO reintroduce PinningTrustManager and MemorizingTrustManager - - // initialized the local repo information FDroidApp.initWifiSettings(); startService(new Intent(this, WifiStateChangeService.class)); // if the HTTPS pref changes, then update all affected things diff --git a/app/src/main/java/org/fdroid/fdroid/Preferences.java b/app/src/main/java/org/fdroid/fdroid/Preferences.java index aed547b90..326ac2107 100644 --- a/app/src/main/java/org/fdroid/fdroid/Preferences.java +++ b/app/src/main/java/org/fdroid/fdroid/Preferences.java @@ -347,6 +347,9 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh private static Preferences instance; + /** + * Needs to be setup before anything else tries to access it. + */ public static void setup(Context context) { if (instance != null) { final String error = "Attempted to reinitialize preferences after it " + diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index 860b6b198..c3cadf50b 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -101,9 +101,11 @@ public class UpdateService extends IntentService implements ProgressListener { context.startService(intent); } - // Schedule (or cancel schedule for) this service, according to the - // current preferences. Should be called a) at boot, b) if the preference - // is changed, or c) on startup, in case we get upgraded. + /** + * Schedule or cancel this service to update the app index, according to the + * current preferences. Should be called a) at boot, b) if the preference + * is changed, or c) on startup, in case we get upgraded. + */ public static void schedule(Context ctx) { SharedPreferences prefs = PreferenceManager diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppCacheUpdater.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppCacheUpdater.java index 137921cf7..1e86bc8c9 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppCacheUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppCacheUpdater.java @@ -51,8 +51,13 @@ public final class InstalledAppCacheUpdater { /** * Ensure our database of installed apps is in sync with what the PackageManager tells us is installed. - * Once completed, the relevant ContentProviders will be notified of any changes to installed statuses. - * This method returns immediately, and will continue to work in an AsyncTask. + * The installed app cache hasn't gotten out of sync somehow, e.g. if we crashed/ran out of battery + * half way through responding to a package installed {@link android.content.Intent}. Once completed, + * the relevant {@link android.content.ContentProvider}s will be notified of any changes to installed + * statuses. This method returns immediately, and will continue to work in an AsyncTask. It doesn't + * really matter where we put this in the bootstrap process, because it runs on a different thread, + * which will be delayed by some seconds to avoid an error where the database is locked due to the + * database updater. */ public static void updateInBackground(Context context) { InstalledAppCacheUpdater updater = new InstalledAppCacheUpdater(context); From 6fa84776506f137dbfac9eed1d11db203e640c9f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 14 Apr 2016 17:24:22 -0400 Subject: [PATCH 3/6] make Utils.getApkCacheDir() more likely to succeed * if there is a file there, remove it The paths are all from the system, so are safe. No SanitizedFile is needed. Plus, this method was not checking if the original and sanitized versions where different, and instead just creating the sanitized version. I worry that could cause odd bugs. --- app/src/main/java/org/fdroid/fdroid/Utils.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index 88a994bd1..c538274e4 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -325,8 +325,11 @@ public final class Utils { * Using {@link org.fdroid.fdroid.installer.Installer#installPackage(File, String, String)} * is fine since that does the right thing. */ - public static SanitizedFile getApkCacheDir(Context context) { - final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, true), "apks"); + public static File getApkCacheDir(Context context) { + File apkCacheDir = new File(StorageUtils.getCacheDirectory(context, true), "apks"); + if (apkCacheDir.isFile()) { + apkCacheDir.delete(); + } if (!apkCacheDir.exists()) { apkCacheDir.mkdir(); } From 83ee0c8f0b0aa9a3e4a81e1e2ced35940cc6f736 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 14 Apr 2016 18:59:24 -0400 Subject: [PATCH 4/6] clean up cached files in a low priority IntentService This moves the cache file deletion to a dedicated IntentService that runs at the lowest possible priority. The cache cleanup does not need to happen with any kind of priority, so it shouldn't delay the app start or take any resources away from foreground processes. This also changes the logic around the "Cache packages" preference. The downloader always saves APKs, then if "Cache packages" is disabled, those APKs are deleted when they are older than an hour. This also simplifies Utils.deleteFiles() since the endswith arg is no longer needed. --- .../java/org/fdroid/fdroid/UtilsTest.java | 35 ++++++++++ app/src/main/AndroidManifest.xml | 3 + .../org/fdroid/fdroid/CleanCacheService.java | 70 +++++++++++++++++++ .../java/org/fdroid/fdroid/FDroidApp.java | 24 +------ .../main/java/org/fdroid/fdroid/Utils.java | 55 ++++++--------- 5 files changed, 130 insertions(+), 57 deletions(-) create mode 100644 app/src/main/java/org/fdroid/fdroid/CleanCacheService.java diff --git a/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java b/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java index da038dd10..96f701328 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java @@ -1,13 +1,18 @@ package org.fdroid.fdroid; +import android.app.Instrumentation; import android.content.Context; import android.support.test.InstrumentationRegistry; import android.support.test.runner.AndroidJUnit4; +import org.apache.commons.io.FileUtils; import org.junit.Test; import org.junit.runner.RunWith; +import java.io.File; +import java.io.IOException; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -137,4 +142,34 @@ public class UtilsTest { } // TODO write tests that work with a Certificate + + @Test + public void testClearOldFiles() throws IOException, InterruptedException { + Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation(); + File dir = new File(TestUtils.getWriteableDir(instrumentation), "clearOldFiles"); + FileUtils.deleteQuietly(dir); + dir.mkdirs(); + assertTrue(dir.isDirectory()); + + File first = new File(dir, "first"); + File second = new File(dir, "second"); + assertFalse(first.exists()); + assertFalse(second.exists()); + + first.createNewFile(); + assertTrue(first.exists()); + + Thread.sleep(7000); + second.createNewFile(); + assertTrue(second.exists()); + + Utils.clearOldFiles(dir, 3); + assertFalse(first.exists()); + assertTrue(second.exists()); + + Thread.sleep(7000); + Utils.clearOldFiles(dir, 3); + assertFalse(first.exists()); + assertFalse(second.exists()); + } } diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index c499b50d7..b9b3419c2 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -445,6 +445,9 @@ + diff --git a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java new file mode 100644 index 000000000..48d5910c3 --- /dev/null +++ b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java @@ -0,0 +1,70 @@ +package org.fdroid.fdroid; + +import android.app.IntentService; +import android.content.Context; +import android.content.Intent; +import android.os.Process; + +import org.apache.commons.io.FileUtils; + +import java.io.File; + +/** + * Handles cleaning up caches files that are not going to be used, and do not + * block the operation of the app itself. For things that must happen before + * F-Droid starts normal operation, that should go into + * {@link FDroidApp#onCreate()} + */ +public class CleanCacheService extends IntentService { + public static final String TAG = "CleanCacheService"; + + public static void start(Context context) { + Intent intent = new Intent(context, CleanCacheService.class); + context.startService(intent); + } + + public CleanCacheService() { + super("CleanCacheService"); + } + + @Override + protected void onHandleIntent(Intent intent) { + Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST); + + int cachetime; + if (Preferences.get().shouldCacheApks()) { + cachetime = Integer.MAX_VALUE; + } else { + cachetime = 3600; // keep for 1 hour to allow resumable downloads + } + Utils.clearOldFiles(Utils.getApkCacheDir(this), cachetime); + deleteStrayIndexFiles(); + } + + /** + * Delete index files which were downloaded, but not removed (e.g. due to F-Droid being + * force closed during processing of the file, before getting a chance to delete). This + * may include both "index-*-downloaded" and "index-*-extracted.xml" files. + *

+ * Note that if the SD card is not ready, then the cache directory will probably not be + * available. In this situation no files will be deleted (and thus they may still exist + * after the SD card becomes available). + */ + private void deleteStrayIndexFiles() { + File cacheDir = getCacheDir(); + if (cacheDir == null) { + return; + } + + final File[] files = cacheDir.listFiles(); + if (files == null) { + return; + } + + for (File f : files) { + if (f.getName().startsWith("index-")) { + FileUtils.deleteQuietly(f); + } + } + } +} diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 6ec2355b8..4213dce38 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -221,29 +221,7 @@ public class FDroidApp extends Application { } }); - // Clear cached apk files. We used to just remove them after they'd - // been installed, but this causes problems for proprietary gapps - // users since the introduction of verification (on pre-4.2 Android), - // because the install intent says it's finished when it hasn't. - if (!Preferences.get().shouldCacheApks()) { - Utils.deleteFiles(Utils.getApkCacheDir(this), null, ".apk"); - } - - // Index files which downloaded, but were not removed (e.g. due to F-Droid being force - // closed during processing of the file, before getting a chance to delete). This may - // include both "index-*-downloaded" and "index-*-extracted.xml" files. The first is from - // either signed or unsigned repos, and the later is from signed repos. - Utils.deleteFiles(getCacheDir(), "index-", null); - - // As above, but for legacy F-Droid clients that downloaded under a different name, and - // extracted to the files directory rather than the cache directory. - // TODO: This can be removed in a a few months or a year (e.g. 2016) because people will - // have upgraded their clients, this code will have executed, and they will not have any - // left over files any more. Even if they do hold off upgrading until this code is removed, - // the only side effect is that they will have a few more MiB of storage taken up on their - // device until they uninstall and re-install F-Droid. - Utils.deleteFiles(getCacheDir(), "dl-", null); - Utils.deleteFiles(getFilesDir(), "index-", null); + CleanCacheService.start(this); UpdateService.schedule(getApplicationContext()); bluetoothAdapter = getBluetoothAdapter(); diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index c538274e4..fa6148ece 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -37,6 +37,7 @@ import com.nostra13.universalimageloader.core.assist.ImageScaleType; import com.nostra13.universalimageloader.core.display.FadeInBitmapDisplayer; import com.nostra13.universalimageloader.utils.StorageUtils; +import org.apache.commons.io.FileUtils; import org.fdroid.fdroid.compat.FileCompat; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.Repo; @@ -336,6 +337,26 @@ public final class Utils { return apkCacheDir; } + /** + * Recursively delete files in {@code dir} that were last modified + * {@code secondsAgo} seconds ago, e.g. when it was downloaded. + * + * @param dir The directory to recurse in + * @param secondsAgo The number of seconds old that marks a file for deletion. + */ + public static void clearOldFiles(File dir, long secondsAgo) { + long olderThan = System.currentTimeMillis() - (secondsAgo * 1000L); + for (File f : dir.listFiles()) { + if (f.isDirectory()) { + clearOldFiles(f, olderThan); + f.delete(); + } + if (FileUtils.isFileOlder(f, olderThan)) { + f.delete(); + } + } + } + public static String calcFingerprint(String keyHexString) { if (TextUtils.isEmpty(keyHexString) || keyHexString.matches(".*[^a-fA-F0-9].*")) { @@ -650,40 +671,6 @@ public final class Utils { } } - /** - * Remove all files from the {@param directory} either beginning with {@param startsWith} - * or ending with {@param endsWith}. Note that if the SD card is not ready, then the - * cache directory will probably not be available. In this situation no files will be - * deleted (and thus they may still exist after the SD card becomes available). - */ - public static void deleteFiles(@Nullable File directory, @Nullable String startsWith, @Nullable String endsWith) { - - if (directory == null) { - return; - } - - final File[] files = directory.listFiles(); - if (files == null) { - return; - } - - if (startsWith != null) { - debugLog(TAG, "Cleaning up files in " + directory + " that start with \"" + startsWith + "\""); - } - - if (endsWith != null) { - debugLog(TAG, "Cleaning up files in " + directory + " that end with \"" + endsWith + "\""); - } - - for (File f : files) { - if (((startsWith != null && f.getName().startsWith(startsWith)) - || (endsWith != null && f.getName().endsWith(endsWith))) - && !f.delete()) { - Log.w(TAG, "Couldn't delete cache file " + f); - } - } - } - public static void debugLog(String tag, String msg) { if (BuildConfig.DEBUG) { Log.d(tag, msg); From cbf1bda433a7b7a0de2690002202d6c77fae2c32 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 14 Apr 2016 19:28:52 -0400 Subject: [PATCH 5/6] add preference to set the time to keep cached APKs This schedules CleanCacheService to run regularly, and delete files older than the value set in the new "Keep cached apps" preference. It auto- migrates the old "Cache packages" pref to the new one. The default cache time for people who did not have "Cache packages" enabled is one day. --- .../org/fdroid/fdroid/CleanCacheService.java | 34 +++++++++++++------ .../java/org/fdroid/fdroid/FDroidApp.java | 2 +- .../java/org/fdroid/fdroid/Preferences.java | 34 ++++++++++++++++--- .../views/fragments/PreferencesFragment.java | 6 ++-- app/src/main/res/values/array.xml | 9 +++++ app/src/main/res/values/donottranslate.xml | 9 +++++ app/src/main/res/values/strings.xml | 11 ++++-- app/src/main/res/xml/preferences.xml | 7 ++-- 8 files changed, 89 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java index 48d5910c3..0838b6770 100644 --- a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java +++ b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java @@ -1,9 +1,13 @@ package org.fdroid.fdroid; +import android.app.AlarmManager; import android.app.IntentService; +import android.app.PendingIntent; import android.content.Context; import android.content.Intent; import android.os.Process; +import android.os.SystemClock; +import android.util.Log; import org.apache.commons.io.FileUtils; @@ -18,9 +22,26 @@ import java.io.File; public class CleanCacheService extends IntentService { public static final String TAG = "CleanCacheService"; - public static void start(Context context) { + /** + * Schedule or cancel this service to update the app index, according to the + * current preferences. Should be called a) at boot, b) if the preference + * is changed, or c) on startup, in case we get upgraded. + */ + public static void schedule(Context context) { + long keepTime = Preferences.get().getKeepCacheTime(); + long interval = 604800000; // 1 day + if (keepTime < interval) { + interval = keepTime * 1000; + } + Log.i(TAG, "schedule " + keepTime + " " + interval); + Intent intent = new Intent(context, CleanCacheService.class); - context.startService(intent); + PendingIntent pending = PendingIntent.getService(context, 0, intent, 0); + + AlarmManager alarm = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); + alarm.cancel(pending); + alarm.setInexactRepeating(AlarmManager.ELAPSED_REALTIME, + SystemClock.elapsedRealtime() + 5000, interval, pending); } public CleanCacheService() { @@ -30,14 +51,7 @@ public class CleanCacheService extends IntentService { @Override protected void onHandleIntent(Intent intent) { Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST); - - int cachetime; - if (Preferences.get().shouldCacheApks()) { - cachetime = Integer.MAX_VALUE; - } else { - cachetime = 3600; // keep for 1 hour to allow resumable downloads - } - Utils.clearOldFiles(Utils.getApkCacheDir(this), cachetime); + Utils.clearOldFiles(Utils.getApkCacheDir(this), Preferences.get().getKeepCacheTime()); deleteStrayIndexFiles(); } diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 4213dce38..c774fccd9 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -221,7 +221,7 @@ public class FDroidApp extends Application { } }); - CleanCacheService.start(this); + CleanCacheService.schedule(this); UpdateService.schedule(getApplicationContext()); bluetoothAdapter = getBluetoothAdapter(); diff --git a/app/src/main/java/org/fdroid/fdroid/Preferences.java b/app/src/main/java/org/fdroid/fdroid/Preferences.java index 326ac2107..160e655d3 100644 --- a/app/src/main/java/org/fdroid/fdroid/Preferences.java +++ b/app/src/main/java/org/fdroid/fdroid/Preferences.java @@ -32,9 +32,11 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh private static final String TAG = "Preferences"; + private final Context context; private final SharedPreferences preferences; private Preferences(Context context) { + this.context = context; preferences = PreferenceManager.getDefaultSharedPreferences(context); preferences.registerOnSharedPreferenceChangeListener(this); if (preferences.getString(PREF_LOCAL_REPO_NAME, null) == null) { @@ -52,7 +54,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh public static final String PREF_INCOMP_VER = "incompatibleVersions"; public static final String PREF_THEME = "theme"; public static final String PREF_IGN_TOUCH = "ignoreTouchscreen"; - public static final String PREF_CACHE_APK = "cacheDownloaded"; + public static final String PREF_KEEP_CACHE_TIME = "keepCacheFor"; public static final String PREF_UNSTABLE_UPDATES = "unstableUpdates"; public static final String PREF_EXPERT = "expert"; public static final String PREF_PRIVILEGED_INSTALLER = "privilegedInstaller"; @@ -72,7 +74,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh private static final int DEFAULT_UPD_HISTORY = 14; private static final boolean DEFAULT_PRIVILEGED_INSTALLER = false; //private static final boolean DEFAULT_LOCAL_REPO_BONJOUR = true; - private static final boolean DEFAULT_CACHE_APK = false; + private static final long DEFAULT_KEEP_CACHE_SECONDS = 86400; // one day private static final boolean DEFAULT_UNSTABLE_UPDATES = false; //private static final boolean DEFAULT_LOCAL_REPO_HTTPS = false; private static final boolean DEFAULT_INCOMP_VER = false; @@ -139,8 +141,32 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh PreferencesCompat.apply(preferences.edit().putBoolean(PREF_POST_PRIVILEGED_INSTALL, postInstall)); } - public boolean shouldCacheApks() { - return preferences.getBoolean(PREF_CACHE_APK, DEFAULT_CACHE_APK); + /** + * Old preference replaced by {@link #PREF_KEEP_CACHE_TIME} + */ + private static final String PREF_CACHE_APK = "cacheDownloaded"; + + /** + * Time in seconds to keep cached files. Anything that has been around longer will be deleted + */ + public long getKeepCacheTime() { + String value = preferences.getString(PREF_KEEP_CACHE_TIME, String.valueOf(DEFAULT_KEEP_CACHE_SECONDS)); + + if (preferences.contains(PREF_CACHE_APK)) { + if (preferences.getBoolean(PREF_CACHE_APK, false)) { + value = context.getString(R.string.keep_forever); + } + SharedPreferences.Editor editor = preferences.edit(); + editor.remove(PREF_CACHE_APK); + editor.putString(PREF_KEEP_CACHE_TIME, value); + PreferencesCompat.apply(editor); + } + + try { + return Long.parseLong(value); + } catch (NumberFormatException e) { + return DEFAULT_KEEP_CACHE_SECONDS; + } } public boolean getUnstableUpdates() { diff --git a/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java b/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java index a150db286..5043fc914 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java +++ b/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java @@ -39,7 +39,7 @@ public class PreferencesFragment extends PreferenceFragment Preferences.PREF_IGN_TOUCH, Preferences.PREF_LOCAL_REPO_NAME, Preferences.PREF_LANGUAGE, - Preferences.PREF_CACHE_APK, + Preferences.PREF_KEEP_CACHE_TIME, Preferences.PREF_EXPERT, Preferences.PREF_PRIVILEGED_INSTALLER, Preferences.PREF_ENABLE_PROXY, @@ -143,8 +143,8 @@ public class PreferencesFragment extends PreferenceFragment } break; - case Preferences.PREF_CACHE_APK: - checkSummary(key, R.string.cache_downloaded_on); + case Preferences.PREF_KEEP_CACHE_TIME: + entrySummary(key); break; case Preferences.PREF_EXPERT: diff --git a/app/src/main/res/values/array.xml b/app/src/main/res/values/array.xml index de5f1f86b..c575ea5a4 100644 --- a/app/src/main/res/values/array.xml +++ b/app/src/main/res/values/array.xml @@ -10,6 +10,15 @@ @string/interval_2w + + @string/keep_hour + @string/keep_day + @string/keep_week + @string/keep_month + @string/keep_year + @string/keep_forever + + @string/theme_light @string/theme_dark diff --git a/app/src/main/res/values/donottranslate.xml b/app/src/main/res/values/donottranslate.xml index d6a202fab..c7d387726 100644 --- a/app/src/main/res/values/donottranslate.xml +++ b/app/src/main/res/values/donottranslate.xml @@ -23,6 +23,15 @@ 336 + + 3600 + 86400 + 604800 + 2592000 + 31449600 + 2147483647 + + light dark diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d2203a659..2b2cb2c8b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -10,8 +10,8 @@ by Delete Enable NFC Send… - Cache packages - Keep downloaded package files on device + Keep cached apps + Keep downloaded APK files on device Updates Unstable updates Suggest updates to unstable versions @@ -383,6 +383,13 @@ Weekly Every 2 weeks + 1 Hour + 1 Day + 1 Week + 1 Month + 1 Year + Forever + Light Dark Night diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index 4b2c7d14f..31fb9aee9 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -73,9 +73,10 @@ android:dependency="enableProxy" /> - + From 77052c2b4514b7d20f2e3c4d074f5f24b3235156 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 2 May 2016 20:35:38 +0200 Subject: [PATCH 6/6] remove cosmetic changes to security sensitive code: RepoUpdater Security-sensitive code should not be changed unless there is a good reason to do so. It is too easy to introduce bugs. This change does not address an issue, so I'm reverting it. See comment in javadoc header for the class. This reverts commit 2074718391c2c17a974218bc6565cce2dc05407e for just the RepoUpdater.java file. --- .../java/org/fdroid/fdroid/RepoUpdater.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index 5fd62034b..4dc986e05 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -92,15 +92,6 @@ public class RepoUpdater { return hasChanged; } - private static void cleanupDownloader(Downloader d) { - if (d == null || d.outputFile == null) { - return; - } - if (!d.outputFile.delete()) { - Log.w(TAG, "Couldn't delete file: " + d.outputFile.getAbsolutePath()); - } - } - private Downloader downloadIndex() throws UpdateException { Downloader downloader = null; try { @@ -115,7 +106,11 @@ public class RepoUpdater { } } catch (IOException e) { - cleanupDownloader(downloader); + if (downloader != null && downloader.outputFile != null) { + if (!downloader.outputFile.delete()) { + Log.w(TAG, "Couldn't delete file: " + downloader.outputFile.getAbsolutePath()); + } + } throw new UpdateException(repo, "Error getting index file", e); } catch (InterruptedException e) { @@ -202,8 +197,10 @@ public class RepoUpdater { } finally { FDroidApp.enableSpongyCastleOnLollipop(); Utils.closeQuietly(indexInputStream); - if (downloadedFile != null && !downloadedFile.delete()) { - Log.w(TAG, "Couldn't delete file: " + downloadedFile.getAbsolutePath()); + if (downloadedFile != null) { + if (!downloadedFile.delete()) { + Log.w(TAG, "Couldn't delete file: " + downloadedFile.getAbsolutePath()); + } } } }