diff --git a/F-Droid/src/org/fdroid/fdroid/AppDetails.java b/F-Droid/src/org/fdroid/fdroid/AppDetails.java index 3e6925f06..e127a6fb7 100644 --- a/F-Droid/src/org/fdroid/fdroid/AppDetails.java +++ b/F-Droid/src/org/fdroid/fdroid/AppDetails.java @@ -872,7 +872,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A } private void startDownload(Apk apk, String repoAddress) { - downloadHandler = new ApkDownloader(apk, repoAddress, Utils.getApkCacheDir(getBaseContext())); + downloadHandler = new ApkDownloader(getBaseContext(), apk, repoAddress); downloadHandler.setProgressListener(this); if (downloadHandler.download()) { updateProgressDialog(); diff --git a/F-Droid/src/org/fdroid/fdroid/FDroidApp.java b/F-Droid/src/org/fdroid/fdroid/FDroidApp.java index 1a67b5a1e..edbd7ba77 100644 --- a/F-Droid/src/org/fdroid/fdroid/FDroidApp.java +++ b/F-Droid/src/org/fdroid/fdroid/FDroidApp.java @@ -184,7 +184,8 @@ public class FDroidApp extends Application { SharedPreferences prefs = PreferenceManager .getDefaultSharedPreferences(getBaseContext()); curTheme = Theme.valueOf(prefs.getString(Preferences.PREF_THEME, Preferences.DEFAULT_THEME)); - if (!prefs.getBoolean(Preferences.PREF_CACHE_APK, false)) { + Utils.deleteFiles(Utils.getApkDownloadDir(this), null, ".apk"); + if (!Preferences.get().shouldCacheApks()) { Utils.deleteFiles(Utils.getApkCacheDir(this), null, ".apk"); } diff --git a/F-Droid/src/org/fdroid/fdroid/Preferences.java b/F-Droid/src/org/fdroid/fdroid/Preferences.java index 21d125dbd..9c5f8253d 100644 --- a/F-Droid/src/org/fdroid/fdroid/Preferences.java +++ b/F-Droid/src/org/fdroid/fdroid/Preferences.java @@ -67,6 +67,7 @@ public class Preferences implements SharedPreferences.OnSharedPreferenceChangeLi private static final boolean DEFAULT_ROOT_INSTALLER = false; private static final boolean DEFAULT_SYSTEM_INSTALLER = false; private static final boolean DEFAULT_LOCAL_REPO_BONJOUR = true; + private static final boolean DEFAULT_CACHE_APK = false; private static final boolean DEFAULT_LOCAL_REPO_HTTPS = false; private static final boolean DEFAULT_INCOMP_VER = false; private static final boolean DEFAULT_EXPERT = false; @@ -113,6 +114,10 @@ public class Preferences implements SharedPreferences.OnSharedPreferenceChangeLi return preferences.getBoolean(PREF_LOCAL_REPO_BONJOUR, DEFAULT_LOCAL_REPO_BONJOUR); } + public boolean shouldCacheApks() { + return preferences.getBoolean(PREF_CACHE_APK, DEFAULT_CACHE_APK); + } + public boolean showIncompatibleVersions() { return preferences.getBoolean(PREF_INCOMP_VER, DEFAULT_INCOMP_VER); } diff --git a/F-Droid/src/org/fdroid/fdroid/Utils.java b/F-Droid/src/org/fdroid/fdroid/Utils.java index 299140a5d..a85c9ce0c 100644 --- a/F-Droid/src/org/fdroid/fdroid/Utils.java +++ b/F-Droid/src/org/fdroid/fdroid/Utils.java @@ -310,14 +310,25 @@ public final class Utils { return b.build(); } + /** + * See {@link Utils#getApkDownloadDir(android.content.Context)} for why this is "unsafe". + */ + public static SanitizedFile getApkCacheDir(Context context) { + final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, true), "apks"); + if (!apkCacheDir.exists()) { + apkCacheDir.mkdir(); + } + return apkCacheDir; + } + /** * The directory where .apk files are downloaded (and stored - if the relevant property is enabled). * This must be on internal storage, to prevent other apps with "write external storage" from being * able to change the .apk file between F-Droid requesting the Package Manger to install, and the * Package Manager receiving that request. */ - public static File getApkCacheDir(Context context) { - SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "apks"); + public static File getApkDownloadDir(Context context) { + final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "temp"); if (!apkCacheDir.exists()) { apkCacheDir.mkdir(); } diff --git a/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java b/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java index bc285b380..0ae7a5ade 100644 --- a/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java +++ b/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java @@ -20,11 +20,15 @@ package org.fdroid.fdroid.net; +import android.content.Context; import android.os.Bundle; +import android.support.annotation.NonNull; import android.util.Log; import org.fdroid.fdroid.Hasher; +import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.ProgressListener; +import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.compat.FileCompat; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.SanitizedFile; @@ -58,9 +62,10 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { */ public static final String EVENT_DATA_ERROR_TYPE = "apkDownloadErrorType"; - private final Apk curApk; - private final String repoAddress; - private final SanitizedFile localFile; + @NonNull private final Apk curApk; + @NonNull private final String repoAddress; + @NonNull private final SanitizedFile localFile; + @NonNull private final SanitizedFile potentiallyCachedFile; private ProgressListener listener; private AsyncDownloadWrapper dlWrapper = null; @@ -68,7 +73,7 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { private int totalSize = 0; private boolean isComplete = false; - private final long id =++downloadIdCounter; + private final long id = ++downloadIdCounter; public void setProgressListener(ProgressListener listener) { this.listener = listener; @@ -78,10 +83,11 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { setProgressListener(null); } - public ApkDownloader(Apk apk, String repoAddress, File destDir) { + public ApkDownloader(@NonNull final Context context, @NonNull final Apk apk, @NonNull final String repoAddress) { curApk = apk; this.repoAddress = repoAddress; - localFile = new SanitizedFile(destDir, curApk.apkName); + localFile = new SanitizedFile(Utils.getApkDownloadDir(context), apk.apkName); + potentiallyCachedFile = new SanitizedFile(Utils.getApkCacheDir(context), apk.apkName); } /** @@ -104,23 +110,23 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { return repoAddress + "/" + curApk.apkName.replace(" ", "%20"); } - private Hasher createHasher() { + private Hasher createHasher(File apkFile) { Hasher hasher; try { - hasher = new Hasher(curApk.hashType, localFile); + hasher = new Hasher(curApk.hashType, apkFile); } catch (NoSuchAlgorithmException e) { - Log.e(TAG, "Error verifying hash of cached apk at " + localFile + ". " + + Log.e(TAG, "Error verifying hash of cached apk at " + apkFile + ". " + "I don't understand what the " + curApk.hashType + " hash algorithm is :("); hasher = null; } return hasher; } - private boolean hashMatches() { - if (!localFile.exists()) { + private boolean hashMatches(@NonNull final File apkFile) { + if (!apkFile.exists()) { return false; } - Hasher hasher = createHasher(); + Hasher hasher = createHasher(apkFile); return hasher != null && hasher.match(curApk.hash); } @@ -129,25 +135,35 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { * want to download, then we will return true. Otherwise, we return false * (and remove the cached file - if it exists and didn't match the correct hash). */ - private boolean verifyOrDeleteCachedVersion() { - if (localFile.exists()) { - if (hashMatches()) { - Log.d(TAG, "Using cached apk at " + localFile); + private boolean verifyOrDelete(@NonNull final File apkFile) { + if (apkFile.exists()) { + if (hashMatches(apkFile)) { + Log.d(TAG, "Using cached apk at " + apkFile); return true; } - Log.d(TAG, "Not using cached apk at " + localFile); - deleteLocalFile(); + Log.d(TAG, "Not using cached apk at " + apkFile + "(hash doesn't match, will delete file)"); + delete(apkFile); } return false; } - private void deleteLocalFile() { - if (localFile != null && localFile.exists()) { - localFile.delete(); + private void delete(@NonNull final File file) { + if (file.exists()) { + if (!file.delete()) { + Log.w(TAG, "Could not delete file " + file); + } } } - private void sendCompleteMessage() { + private void prepareApkFileAndSendCompleteMessage() { + + // Need the apk to be world readable, so that the installer is able to read it. + // Note that saving it into external storage for the purpose of letting the 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(localFile, true, false); + isComplete = true; sendMessage(EVENT_APK_DOWNLOAD_COMPLETE); } @@ -164,13 +180,15 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { public boolean download() { // Can we use the cached version? - if (verifyOrDeleteCachedVersion()) { - sendCompleteMessage(); + if (verifyOrDelete(potentiallyCachedFile)) { + delete(localFile); + Utils.copy(potentiallyCachedFile, localFile); + prepareApkFileAndSendCompleteMessage(); return false; } String remoteAddress = getRemoteAddress(); - Log.d(TAG, "Downloading apk from " + remoteAddress); + Log.d(TAG, "Downloading apk from " + remoteAddress + " to " + localFile); try { Downloader downloader = DownloaderFactory.create(remoteAddress, localFile); @@ -228,26 +246,28 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { public void onErrorDownloading(String localisedExceptionDetails) { Log.e(TAG, "Download failed: " + localisedExceptionDetails); sendError(ERROR_DOWNLOAD_FAILED); - deleteLocalFile(); + delete(localFile); + } + + private void cacheIfRequired() { + if (Preferences.get().shouldCacheApks()) { + Log.i(TAG, "Copying .apk file to cache at " + potentiallyCachedFile.getAbsolutePath()); + Utils.copy(localFile, potentiallyCachedFile); + } } @Override public void onDownloadComplete() { - if (!verifyOrDeleteCachedVersion()) { + if (!verifyOrDelete(localFile)) { sendError(ERROR_HASH_MISMATCH); return; } - // Need the apk to be world readable, so that the installer is able to read it. - // Note that saving it into external storage for the purpose of letting the 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(localFile, true, false); + cacheIfRequired(); Log.d(TAG, "Download finished: " + localFile); - sendCompleteMessage(); + prepareApkFileAndSendCompleteMessage(); } @Override