From 20c66a825d2cf1873818f3d37c40aca07290b695 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 31 Mar 2016 21:55:07 +0200 Subject: [PATCH 01/15] use IntentService style to download all APKs via a queue DownloaderService is based on IntentService to provide queued requests that run in a background thread via the Handler and the HandlerThread. It began as the IntentService code, but it could not be a subclass because the downloading needs to be cancelable. IntentServices cannot be canceled and they provide no visibility into their queue. DownloaderService then announces relevant events via LocalBroadcastManager and Intents with custom "action" Strings. https://gitlab.com/fdroid/fdroidclient/issues/601 #601 --- .../fdroid/net/DownloaderServiceTest.java | 41 +++ app/src/main/AndroidManifest.xml | 3 + .../java/org/fdroid/fdroid/AppDetails.java | 141 ++++----- .../org/fdroid/fdroid/net/ApkDownloader.java | 182 +++--------- .../fdroid/net/AsyncDownloadWrapper.java | 79 ----- .../fdroid/fdroid/net/AsyncDownloader.java | 17 -- .../org/fdroid/fdroid/net/Downloader.java | 8 +- .../fdroid/fdroid/net/DownloaderFactory.java | 17 +- .../fdroid/fdroid/net/DownloaderService.java | 276 ++++++++++++++++++ .../fdroid/views/swap/SwapAppsView.java | 41 +-- .../views/swap/SwapWorkflowActivity.java | 24 +- 11 files changed, 463 insertions(+), 366 deletions(-) create mode 100644 app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java delete mode 100644 app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java delete mode 100644 app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java create mode 100644 app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java diff --git a/app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java b/app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java new file mode 100644 index 000000000..7803ad070 --- /dev/null +++ b/app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java @@ -0,0 +1,41 @@ + +package org.fdroid.fdroid.net; + +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; +import android.support.v4.content.LocalBroadcastManager; +import android.test.ServiceTestCase; +import android.util.Log; + +@SuppressWarnings("PMD") // TODO port this to JUnit 4 semantics +public class DownloaderServiceTest extends ServiceTestCase { + public static final String TAG = "DownloaderServiceTest"; + + String[] urls = { + "https://en.wikipedia.org/wiki/Index.html", + "https://mirrors.kernel.org/debian/dists/stable/Release", + "https://f-droid.org/archive/de.we.acaldav_5.apk", + // sites that use SNI for HTTPS + "https://guardianproject.info/fdroid/repo/index.jar", + }; + + public DownloaderServiceTest() { + super(DownloaderService.class); + } + + public void testQueueingDownload() throws InterruptedException { + LocalBroadcastManager localBroadcastManager = LocalBroadcastManager.getInstance(getContext()); + localBroadcastManager.registerReceiver(new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + Log.i(TAG, "onReceive " + intent); + } + }, new IntentFilter(Downloader.ACTION_PROGRESS)); + for (String url : urls) { + DownloaderService.queue(getContext(), url); + } + Thread.sleep(30000); + } +} diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 9b21da52c..3610320ea 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -439,6 +439,9 @@ + diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 26668808c..690bd0618 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -29,7 +29,6 @@ import android.content.ContentValues; import android.content.Context; import android.content.DialogInterface; import android.content.Intent; -import android.content.IntentFilter; import android.content.pm.PackageManager; import android.database.ContentObserver; import android.graphics.Bitmap; @@ -92,6 +91,7 @@ import org.fdroid.fdroid.installer.Installer.AndroidNotCompatibleException; import org.fdroid.fdroid.installer.Installer.InstallerCallback; import org.fdroid.fdroid.net.ApkDownloader; import org.fdroid.fdroid.net.Downloader; +import org.fdroid.fdroid.net.DownloaderService; import java.io.File; import java.util.Iterator; @@ -106,7 +106,7 @@ interface AppDetailsData { /** * Interface which allows the apk list fragment to communicate with the activity when * a user requests to install/remove an apk by clicking on an item in the list. - * + *

* NOTE: This is not to do with with the sudo/packagemanager/other installer * stuff which allows multiple ways to install apps. It is only here to make fragment- * activity communication possible. @@ -117,7 +117,7 @@ interface AppInstallListener { void removeApk(String packageName); } -public class AppDetails extends AppCompatActivity implements ProgressListener, AppDetailsData, AppInstallListener { +public class AppDetails extends AppCompatActivity implements AppDetailsData, AppInstallListener { private static final String TAG = "AppDetails"; @@ -451,17 +451,7 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A refreshApkList(); refreshHeader(); supportInvalidateOptionsMenu(); - - if (downloadHandler != null) { - if (downloadHandler.isComplete()) { - downloadCompleteInstallApk(); - } else { - localBroadcastManager.registerReceiver(downloaderProgressReceiver, - new IntentFilter(Downloader.LOCAL_ACTION_PROGRESS)); - downloadHandler.setProgressListener(this); - headerFragment.startProgress(); - } - } + registerDownloaderReceivers(); } /** @@ -469,23 +459,11 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A */ private void cleanUpFinishedDownload() { if (downloadHandler != null) { - downloadHandler.removeProgressListener(); headerFragment.removeProgress(); downloadHandler = null; } } - /** - * Once the download completes successfully, call this method to start the install process - * with the file that was downloaded. - */ - private void downloadCompleteInstallApk() { - if (downloadHandler != null) { - installApk(downloadHandler.localFile()); - cleanUpFinishedDownload(); - } - } - protected void onStop() { super.onStop(); getContentResolver().unregisterContentObserver(myAppObserver); @@ -499,16 +477,41 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A Utils.debugLog(TAG, "Updating 'ignore updates', as it has changed since we started the activity..."); setIgnoreUpdates(app.packageName, app.ignoreAllUpdates, app.ignoreThisUpdate); } - - localBroadcastManager.unregisterReceiver(downloaderProgressReceiver); - if (downloadHandler != null) { - downloadHandler.removeProgressListener(); - } - + unregisterDownloaderReceivers(); headerFragment.removeProgress(); } - private final BroadcastReceiver downloaderProgressReceiver = new BroadcastReceiver() { + private void unregisterDownloaderReceivers() { + localBroadcastManager.unregisterReceiver(startedReceiver); + localBroadcastManager.unregisterReceiver(progressReceiver); + localBroadcastManager.unregisterReceiver(completeReceiver); + localBroadcastManager.unregisterReceiver(interruptedReceiver); + } + + private void registerDownloaderReceivers() { + if (downloadHandler != null) { // if a download is active + String url = downloadHandler.urlString; + localBroadcastManager.registerReceiver(startedReceiver, + DownloaderService.getIntentFilter(url, Downloader.ACTION_STARTED)); + localBroadcastManager.registerReceiver(progressReceiver, + DownloaderService.getIntentFilter(url, Downloader.LOCAL_ACTION_PROGRESS)); + localBroadcastManager.registerReceiver(completeReceiver, + DownloaderService.getIntentFilter(url, Downloader.ACTION_COMPLETE)); + localBroadcastManager.registerReceiver(interruptedReceiver, + DownloaderService.getIntentFilter(url, Downloader.ACTION_INTERRUPTED)); + } + } + + private final BroadcastReceiver startedReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + if (headerFragment != null) { + headerFragment.startProgress(); + } + } + }; + + private final BroadcastReceiver progressReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { if (headerFragment != null) { @@ -518,6 +521,23 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A } }; + private final BroadcastReceiver completeReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + File localFile = new File(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH)); + installApk(localFile); + cleanUpFinishedDownload(); + } + }; + + private final BroadcastReceiver interruptedReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + Toast.makeText(context, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); + cleanUpFinishedDownload(); + } + }; + private void onAppChanged() { if (!reset(app.packageName)) { this.finish(); @@ -550,7 +570,7 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A @Override protected void onDestroy() { if (downloadHandler != null && !inProcessOfChangingConfiguration) { - downloadHandler.cancel(); + DownloaderService.cancel(context, downloadHandler.urlString); cleanUpFinishedDownload(); } inProcessOfChangingConfiguration = false; @@ -793,11 +813,6 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A return; } - // Ignore call if another download is running. - if (downloadHandler != null && !downloadHandler.isComplete()) { - return; - } - final String repoaddress = getRepoAddress(apk); if (repoaddress == null) return; @@ -853,13 +868,9 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A private void startDownload(Apk apk, String repoAddress) { downloadHandler = new ApkDownloader(getBaseContext(), app, apk, repoAddress); - - localBroadcastManager.registerReceiver(downloaderProgressReceiver, - new IntentFilter(Downloader.LOCAL_ACTION_PROGRESS)); - downloadHandler.setProgressListener(this); - if (downloadHandler.download()) { - headerFragment.startProgress(); - } + registerDownloaderReceivers(); + downloadHandler.download(); + headerFragment.startProgress(); } private void installApk(File file) { @@ -951,42 +962,6 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A startActivity(Intent.createChooser(shareIntent, getString(R.string.menu_share))); } - @Override - public void onProgress(Event event) { - if (downloadHandler == null || !downloadHandler.isEventFromThis(event)) { - // Choose not to respond to events from previous downloaders. - // We don't even care if we receive "cancelled" events or the like, because - // we dealt with cancellations in the onCancel listener of the dialog, - // rather than waiting to receive the event here. We try and be careful in - // the download thread to make sure that we check for cancellations before - // sending events, but it is not possible to be perfect, because the interruption - // which triggers the download can happen after the check to see if - Utils.debugLog(TAG, "Discarding downloader event \"" + event.type + "\" as it is from an old (probably cancelled) downloader."); - return; - } - - boolean finished = false; - switch (event.type) { - case ApkDownloader.EVENT_ERROR: - // this must be on the main UI thread - Toast.makeText(this, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); - cleanUpFinishedDownload(); - finished = true; - break; - case ApkDownloader.EVENT_APK_DOWNLOAD_COMPLETE: - downloadCompleteInstallApk(); - finished = true; - break; - } - - if (finished) { - if (headerFragment != null) { - headerFragment.removeProgress(); - } - downloadHandler = null; - } - } - @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { // handle cases for install manager first @@ -1536,7 +1511,7 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A return; } - activity.downloadHandler.cancel(); + DownloaderService.cancel(getContext(), activity.downloadHandler.urlString); activity.cleanUpFinishedDownload(); setProgressVisible(false); updateViews(); diff --git a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java index ac3a5ff09..0673a7fe4 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -20,6 +20,7 @@ package org.fdroid.fdroid.net; +import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.net.Uri; @@ -30,16 +31,13 @@ import android.widget.Toast; import org.fdroid.fdroid.Hasher; import org.fdroid.fdroid.Preferences; -import org.fdroid.fdroid.ProgressListener; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; -import org.fdroid.fdroid.compat.FileCompat; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.SanitizedFile; import java.io.File; -import java.io.IOException; import java.security.NoSuchAlgorithmException; /** @@ -47,62 +45,24 @@ import java.security.NoSuchAlgorithmException; * If the file has previously been downloaded, it will make use of that * instead, without going to the network to download a new one. */ -public class ApkDownloader implements AsyncDownloader.Listener { +public class ApkDownloader { private static final String TAG = "ApkDownloader"; - public static final String EVENT_APK_DOWNLOAD_COMPLETE = "apkDownloadComplete"; - public static final String EVENT_ERROR = "apkDownloadError"; + public final String urlString; - public static final String ACTION_STATUS = "apkDownloadStatus"; - - private static final String EVENT_SOURCE_ID = "sourceId"; - private static long downloadIdCounter; - - @NonNull private final App app; @NonNull private final Apk curApk; @NonNull private final Context context; - @NonNull private final String repoAddress; - @NonNull private final SanitizedFile localFile; + @NonNull private SanitizedFile localFile; @NonNull private final SanitizedFile potentiallyCachedFile; - - private ProgressListener listener; - private AsyncDownloader dlWrapper; - private boolean isComplete; - - private final long id = ++downloadIdCounter; - - public void setProgressListener(ProgressListener listener) { - this.listener = listener; - } - - public void removeProgressListener() { - setProgressListener(null); - } + private final LocalBroadcastManager localBroadcastManager; public ApkDownloader(@NonNull final Context context, @NonNull final App app, @NonNull final Apk apk, @NonNull final String repoAddress) { this.context = context; - this.app = app; curApk = apk; - this.repoAddress = repoAddress; - localFile = new SanitizedFile(Utils.getApkDownloadDir(context), apk.apkName); potentiallyCachedFile = new SanitizedFile(Utils.getApkCacheDir(context), apk.apkName); - } - - /** - * The downloaded APK. Valid only when getStatus() has returned STATUS.DONE. - */ - public SanitizedFile localFile() { - return localFile; - } - - /** - * When stopping/starting downloaders multiple times (on different threads), it can - * get weird whereby different threads are sending progress events. It is important - * to be able to see which downloader these progress events are coming from. - */ - public boolean isEventFromThis(Event event) { - return event.getData().containsKey(EVENT_SOURCE_ID) && event.getData().getLong(EVENT_SOURCE_ID) == id; + urlString = Utils.getApkUrl(repoAddress, apk); + localBroadcastManager = LocalBroadcastManager.getInstance(context); } private Hasher createHasher(File apkFile) { @@ -150,115 +110,51 @@ public class ApkDownloader implements AsyncDownloader.Listener { } } - 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); + private void sendDownloadComplete() { + Utils.debugLog(TAG, "Download finished: " + localFile); + localBroadcastManager.unregisterReceiver(downloadCompleteReceiver); } - public boolean isComplete() { - return this.isComplete; - } - - /** - * If the download successfully spins up a new thread to start downloading, then we return - * true, otherwise false. This is useful, e.g. when we use a cached version, and so don't - * want to bother with progress dialogs et al. - */ - public boolean download() { - + public void download() { // Can we use the cached version? if (verifyOrDelete(potentiallyCachedFile)) { delete(localFile); Utils.copyQuietly(potentiallyCachedFile, localFile); - prepareApkFileAndSendCompleteMessage(); - return false; - } - - String remoteAddress = Utils.getApkUrl(repoAddress, curApk); - Utils.debugLog(TAG, "Downloading apk from " + remoteAddress + " to " + localFile); - - try { - dlWrapper = DownloaderFactory.createAsync(context, remoteAddress, localFile, this); - dlWrapper.download(); - return true; - } catch (IOException e) { - e.printStackTrace(); - onErrorDownloading(); - } - - return false; - } - - private void sendMessage(String type) { - sendProgressEvent(new ProgressListener.Event(type)); - } - - // TODO: Completely remove progress listener, only use broadcasts... - private void sendProgressEvent(Event event) { - - event.getData().putLong(EVENT_SOURCE_ID, id); - - if (listener != null) { - listener.onProgress(event); - } - - Intent intent = new Intent(ACTION_STATUS); - intent.setData(Uri.parse(Utils.getApkUrl(repoAddress, curApk))); - intent.putExtras(event.getData()); - LocalBroadcastManager.getInstance(context).sendBroadcast(intent); - } - - @Override - public void onErrorDownloading() { - delete(localFile); - } - - private void cacheIfRequired() { - if (Preferences.get().shouldCacheApks()) { - Utils.debugLog(TAG, "Copying .apk file to cache at " + potentiallyCachedFile.getAbsolutePath()); - Utils.copyQuietly(localFile, potentiallyCachedFile); - } - } - - @Override - public void onDownloadComplete() { - - if (!verifyOrDelete(localFile)) { - sendProgressEvent(new Event(EVENT_ERROR)); - Toast.makeText(context, R.string.corrupt_download, Toast.LENGTH_LONG).show(); + sendDownloadComplete(); return; } - cacheIfRequired(); + Utils.debugLog(TAG, "Downloading apk from " + urlString + " to " + localFile); + localBroadcastManager.registerReceiver(downloadCompleteReceiver, + DownloaderService.getIntentFilter(urlString, Downloader.ACTION_COMPLETE)); - Utils.debugLog(TAG, "Download finished: " + localFile); - prepareApkFileAndSendCompleteMessage(); + DownloaderService.queue(context, urlString); } - @Override - public void onProgress(Event event) { - sendProgressEvent(event); + private void sendProgressEvent(String status) { + Intent intent = new Intent(status); + intent.setData(Uri.parse(urlString)); + intent.putExtra(Downloader.EXTRA_DOWNLOAD_PATH, localFile.getAbsolutePath()); + localBroadcastManager.sendBroadcast(intent); } - /** - * Attempts to cancel the download (if in progress) and also removes the progress - * listener - */ - public void cancel() { - if (dlWrapper != null) { - dlWrapper.attemptCancel(); + // TODO move this code to somewhere more appropriate + BroadcastReceiver downloadCompleteReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + localFile = SanitizedFile.knownSanitized(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH)); + if (!verifyOrDelete(localFile)) { + sendProgressEvent(Downloader.ACTION_INTERRUPTED); + Toast.makeText(context, R.string.corrupt_download, Toast.LENGTH_LONG).show(); + return; + } + + if (Preferences.get().shouldCacheApks()) { + Utils.debugLog(TAG, "Copying .apk file to cache at " + potentiallyCachedFile.getAbsolutePath()); + Utils.copyQuietly(localFile, potentiallyCachedFile); + } + + sendDownloadComplete(); } - } - - public Apk getApk() { - return curApk; - } + }; } diff --git a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java deleted file mode 100644 index 37992a85a..000000000 --- a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java +++ /dev/null @@ -1,79 +0,0 @@ -package org.fdroid.fdroid.net; - -import android.os.Handler; -import android.os.Message; -import android.util.Log; - -import java.io.IOException; - -class AsyncDownloadWrapper extends Handler implements AsyncDownloader { - - private static final String TAG = "AsyncDownloadWrapper"; - - private static final int MSG_DOWNLOAD_COMPLETE = 2; - private static final int MSG_ERROR = 4; - - private final Downloader downloader; - private DownloadThread downloadThread; - - private final Listener listener; - - /** - * Normally the listener would be provided using a setListener method. - * However for the purposes of this async downloader, it doesn't make - * sense to have an async task without any way to notify the outside - * world about completion. Therefore, we require the listener as a - * parameter to the constructor. - */ - AsyncDownloadWrapper(Downloader downloader, Listener listener) { - this.downloader = downloader; - this.listener = listener; - } - - public void download() { - downloadThread = new DownloadThread(); - downloadThread.start(); - } - - public void attemptCancel() { - if (downloader != null) { - downloader.cancelDownload(); - } - } - - /** - * Receives "messages" from the download thread, and passes them onto the - * relevant {@link AsyncDownloader.Listener} - */ - public void handleMessage(Message message) { - switch (message.arg1) { - case MSG_DOWNLOAD_COMPLETE: - listener.onDownloadComplete(); - break; - case MSG_ERROR: - listener.onErrorDownloading(); - break; - } - } - - private class DownloadThread extends Thread { - - public void run() { - try { - downloader.download(); - sendMessage(MSG_DOWNLOAD_COMPLETE); - } catch (InterruptedException e) { - // ignored - } catch (IOException e) { - Log.e(TAG, "I/O exception in download thread", e); - sendMessage(MSG_ERROR); - } - } - - private void sendMessage(int messageType) { - Message message = new Message(); - message.arg1 = messageType; - AsyncDownloadWrapper.this.sendMessage(message); - } - } -} diff --git a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java deleted file mode 100644 index 413b76a05..000000000 --- a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.fdroid.fdroid.net; - -import org.fdroid.fdroid.ProgressListener; - -public interface AsyncDownloader { - - interface Listener extends ProgressListener { - void onErrorDownloading(); - - void onDownloadComplete(); - } - - void download(); - - void attemptCancel(); - -} diff --git a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java index f3ed8e207..76b7849a6 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java @@ -16,10 +16,14 @@ public abstract class Downloader { private static final String TAG = "Downloader"; public static final String LOCAL_ACTION_PROGRESS = "Downloader.PROGRESS"; + public static final String ACTION_STARTED = "org.fdroid.fdroid.net.Downloader.action.STARTED"; + public static final String ACTION_INTERRUPTED = "org.fdroid.fdroid.net.Downloader.action.INTERRUPTED"; + public static final String ACTION_COMPLETE = "org.fdroid.fdroid.net.Downloader.action.COMPLETE"; public static final String EXTRA_ADDRESS = "extraAddress"; - public static final String EXTRA_BYTES_READ = "extraBytesRead"; - public static final String EXTRA_TOTAL_BYTES = "extraTotalBytes"; + public static final String EXTRA_DOWNLOAD_PATH = "org.fdroid.fdroid.net.Downloader.extra.DOWNLOAD_PATH"; + public static final String EXTRA_BYTES_READ = "org.fdroid.fdroid.net.Downloader.extra.BYTES_READ"; + public static final String EXTRA_TOTAL_BYTES = "org.fdroid.fdroid.net.Downloader.extra.TOTAL_BYTES"; private volatile boolean cancelled = false; diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java index 57ea2b7b4..e72eb0ed2 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java @@ -2,6 +2,7 @@ package org.fdroid.fdroid.net; import android.content.Context; import android.content.Intent; +import android.net.Uri; import android.support.v4.content.LocalBroadcastManager; import org.apache.commons.io.FilenameUtils; @@ -25,11 +26,17 @@ public class DownloaderFactory { throws IOException { File destFile = File.createTempFile("dl-", "", context.getCacheDir()); destFile.deleteOnExit(); // this probably does nothing, but maybe... - return create(context, new URL(urlString), destFile); + return create(context, urlString, destFile); } - public static Downloader create(Context context, URL url, File destFile) + public static Downloader create(Context context, Uri uri, File destFile) throws IOException { + return create(context, uri.toString(), destFile); + } + + public static Downloader create(Context context, String urlString, File destFile) + throws IOException { + URL url = new URL(urlString); Downloader downloader = null; if (localBroadcastManager == null) { localBroadcastManager = LocalBroadcastManager.getInstance(context); @@ -71,10 +78,4 @@ public class DownloaderFactory { private static boolean isLocalFile(URL url) { return "file".equalsIgnoreCase(url.getProtocol()); } - - public static AsyncDownloader createAsync(Context context, String urlString, File destFile, AsyncDownloader.Listener listener) - throws IOException { - URL url = new URL(urlString); - return new AsyncDownloadWrapper(create(context, url, destFile), listener); - } } diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java new file mode 100644 index 000000000..0c3986610 --- /dev/null +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -0,0 +1,276 @@ +/* + * Copyright (C) 2008 The Android Open Source Project + * Copyright (C) 2016 Hans-Christoph Steiner + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fdroid.fdroid.net; + +import android.app.Service; +import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; +import android.net.Uri; +import android.os.Handler; +import android.os.HandlerThread; +import android.os.IBinder; +import android.os.Looper; +import android.os.Message; +import android.os.PatternMatcher; +import android.os.Process; +import android.support.v4.content.LocalBroadcastManager; +import android.text.TextUtils; +import android.util.Log; + +import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.compat.FileCompat; +import org.fdroid.fdroid.data.SanitizedFile; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.util.HashMap; + +/** + * DownloaderService is a service that handles asynchronous download requests + * (expressed as {@link Intent}s) on demand. Clients send download requests + * through {@link android.content.Context#startService(Intent)} calls; the + * service is started as needed, handles each Intent in turn using a worker + * thread, and stops itself when it runs out of work. + *

+ *

This "work queue processor" pattern is commonly used to offload tasks + * from an application's main thread. The DownloaderService class exists to + * simplify this pattern and take care of the mechanics. DownloaderService + * will receive the Intents, launch a worker thread, and stop the service as + * appropriate. + *

+ *

All requests are handled on a single worker thread -- they may take as + * long as necessary (and will not block the application's main loop), but + * only one request will be processed at a time. + *

+ *

+ *

Developer Guides

+ *

For a detailed discussion about how to create services, read the + * Services developer guide.

+ *
+ * + * @see android.os.AsyncTask + */ +public class DownloaderService extends Service { + public static final String TAG = "DownloaderService"; + + private static final String ACTION_QUEUE = "org.fdroid.fdroid.net.DownloaderService.action.QUEUE"; + private static final String ACTION_CANCEL = "org.fdroid.fdroid.net.DownloaderService.action.CANCEL"; + + private volatile Looper serviceLooper; + private volatile ServiceHandler serviceHandler; + private volatile Downloader downloader; + private LocalBroadcastManager localBroadcastManager; + + private final HashMap queueWhats = new HashMap(); + private int what; + + private final class ServiceHandler extends Handler { + ServiceHandler(Looper looper) { + super(looper); + } + + @Override + public void handleMessage(Message msg) { + Log.i(TAG, "handleMessage " + msg); + handleIntent((Intent) msg.obj); + stopSelf(msg.arg1); + } + } + + @Override + public void onCreate() { + super.onCreate(); + Log.i(TAG, "onCreate"); + + HandlerThread thread = new HandlerThread(TAG, Process.THREAD_PRIORITY_BACKGROUND); + thread.start(); + + serviceLooper = thread.getLooper(); + serviceHandler = new ServiceHandler(serviceLooper); + localBroadcastManager = LocalBroadcastManager.getInstance(this); + } + + @Override + public void onStart(Intent intent, int startId) { + super.onStart(intent, startId); + Log.i(TAG, "onStart " + startId + " " + intent); + String uriString = intent.getDataString(); + if (uriString == null) { + Log.e(TAG, "Received Intent with no URI: " + intent); + return; + } + if (ACTION_CANCEL.equals(intent.getAction())) { + Log.i(TAG, "Removed " + intent); + Integer what = queueWhats.remove(uriString); + if (what != null && serviceHandler.hasMessages(what)) { + // the URL is in the queue, remove it + serviceHandler.removeMessages(what); + } else if (downloader != null && TextUtils.equals(uriString, downloader.sourceUrl.toString())) { + // the URL is being downloaded, cancel it + downloader.cancelDownload(); + } else { + Log.e(TAG, "CANCEL called on something not queued or running: " + startId + " " + intent); + } + } else if (ACTION_QUEUE.equals(intent.getAction())) { + Log.i(TAG, "Queued " + intent); + Message msg = serviceHandler.obtainMessage(); + msg.arg1 = startId; + msg.obj = intent; + msg.what = what++; + serviceHandler.sendMessage(msg); + Log.i(TAG, "queueWhats.put(" + uriString + ", " + msg.what); + queueWhats.put(uriString, msg.what); + } else { + Log.e(TAG, "Received Intent with unknown action: " + intent); + } + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + onStart(intent, startId); + Log.i(TAG, "onStartCommand " + intent); + return START_REDELIVER_INTENT; // if killed before completion, retry Intent + } + + @Override + public void onDestroy() { + Log.i(TAG, "onDestroy"); + serviceLooper.quit(); //NOPMD - this is copied from IntentService, no super call needed + } + + /** + * This service does not use binding, so no need to implement this method + */ + @Override + public IBinder onBind(Intent intent) { + return null; + } + + /** + * This method is invoked on the worker thread with a request to process. + * Only one Intent is processed at a time, but the processing happens on a + * worker thread that runs independently from other application logic. + * So, if this code takes a long time, it will hold up other requests to + * the same DownloaderService, but it will not hold up anything else. + * When all requests have been handled, the DownloaderService stops itself, + * so you should not ever call {@link #stopSelf}. + * + * @param intent The {@link Intent} passed via {@link + * android.content.Context#startService(Intent)}. + */ + protected void handleIntent(Intent intent) { + final Uri uri = intent.getData(); + Log.i(TAG, "handleIntent " + uri); + final SanitizedFile localFile = new SanitizedFile(Utils.getApkDownloadDir(this), uri.getLastPathSegment()); + sendBroadcast(uri, Downloader.ACTION_STARTED, localFile); + try { + downloader = DownloaderFactory.create(this, uri, localFile); + downloader.setListener(new Downloader.DownloaderProgressListener() { + @Override + public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) { + //Log.i(TAG, "sendProgress " + sourceUrl + " " + bytesRead + " / " + totalBytes); + Intent intent = new Intent(Downloader.LOCAL_ACTION_PROGRESS); + intent.setData(uri); + intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); + intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); + localBroadcastManager.sendBroadcast(intent); + } + }); + downloader.download(); + // 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); + sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile); + } catch (InterruptedException e) { + sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); + } catch (IOException e) { + sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); + e.printStackTrace(); + } finally { + if (downloader != null) { + downloader.close(); + } + } + downloader = null; + Log.i(TAG, "handleIntent DONE " + uri); + } + + private void sendBroadcast(Uri uri, String action, File file) { + Log.i(TAG, "sendBroadcast " + uri + " " + action + " " + file); + Intent intent = new Intent(action); + intent.setData(uri); + intent.putExtra(Downloader.EXTRA_DOWNLOAD_PATH, file.getAbsolutePath()); + localBroadcastManager.sendBroadcast(intent); + } + + /** + * Add a URL to the download queue. + *

+ * All notifications are sent as an {@link Intent} via local broadcasts to be received by + * + * @param context + * @param urlString The URL to add to the download queue + * @see #cancel(Context, String) + */ + public static void queue(Context context, String urlString) { + Log.i(TAG, "queue " + urlString); + Intent intent = new Intent(context, DownloaderService.class); + intent.setAction(ACTION_QUEUE); + intent.setData(Uri.parse(urlString)); + context.startService(intent); + } + + /** + * Remove a URL to the download queue, even if it is currently downloading. + *

+ * All notifications are sent as an {@link Intent} via local broadcasts to be received by + * + * @param context + * @param urlString The URL to remove from the download queue + * @see #queue(Context, String) + */ + public static void cancel(Context context, String urlString) { + Log.i(TAG, "cancel " + urlString); + Intent intent = new Intent(context, DownloaderService.class); + intent.setAction(ACTION_CANCEL); + intent.setData(Uri.parse(urlString)); + context.startService(intent); + } + + /** + * Get a prepared {@link IntentFilter} for use for matching this service's action events. + * + * @param urlString The full file URL to match. + * @param action {@link Downloader#ACTION_STARTED}, {@link Downloader#LOCAL_ACTION_PROGRESS}, + * {@link Downloader#ACTION_INTERRUPTED}, or {@link Downloader#ACTION_COMPLETE}, + * @return + */ + public static IntentFilter getIntentFilter(String urlString, String action) { + Uri uri = Uri.parse(urlString); + IntentFilter intentFilter = new IntentFilter(action); + intentFilter.addDataScheme(uri.getScheme()); + intentFilter.addDataAuthority(uri.getHost(), String.valueOf(uri.getPort())); + intentFilter.addDataPath(uri.getPath(), PatternMatcher.PATTERN_LITERAL); + return intentFilter; + } +} diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java index 8672b7531..3a4df7e85 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -49,8 +49,8 @@ import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.localrepo.SwapService; -import org.fdroid.fdroid.net.ApkDownloader; import org.fdroid.fdroid.net.Downloader; +import org.fdroid.fdroid.net.DownloaderService; import java.util.Timer; import java.util.TimerTask; @@ -231,6 +231,7 @@ public class SwapAppsView extends ListView implements private class ViewHolder { + private final LocalBroadcastManager localBroadcastManager; private App app; @Nullable @@ -247,13 +248,6 @@ public class SwapAppsView extends ListView implements private final BroadcastReceiver downloadProgressReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - Apk apk = getApkToInstall(); - String broadcastUrl = intent.getStringExtra(Downloader.EXTRA_ADDRESS); - - if (apk != null && apk.repoAddress != null && !TextUtils.equals(Utils.getApkUrl(apk.repoAddress, apk), broadcastUrl)) { - return; - } - int read = intent.getIntExtra(Downloader.EXTRA_BYTES_READ, 0); int total = intent.getIntExtra(Downloader.EXTRA_TOTAL_BYTES, 0); if (total > 0) { @@ -267,21 +261,10 @@ public class SwapAppsView extends ListView implements } }; - private final BroadcastReceiver apkDownloadReceiver = new BroadcastReceiver() { + private final BroadcastReceiver appListViewResetReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - Apk apk = getApkToInstall(); - - // Note: This can also be done by using the build in IntentFilter.matchData() - // functionality, matching against the Intent.getData() of the incoming intent. - // I've chosen to do this way, because otherwise we need to query the database - // once for each ViewHolder in order to get the repository address for the - // apkToInstall. This way, we can wait until we receive an incoming intent (if - // at all) and then lazily load the apk to install. - String broadcastUrl = intent.getDataString(); - if (TextUtils.equals(Utils.getApkUrl(apk.repoAddress, apk), broadcastUrl)) { - resetView(); - } + resetView(); } }; @@ -299,11 +282,19 @@ public class SwapAppsView extends ListView implements ViewHolder() { // TODO: Unregister receivers correctly... - IntentFilter apkFilter = new IntentFilter(ApkDownloader.ACTION_STATUS); - LocalBroadcastManager.getInstance(getActivity()).registerReceiver(apkDownloadReceiver, apkFilter); - IntentFilter progressFilter = new IntentFilter(Downloader.LOCAL_ACTION_PROGRESS); - LocalBroadcastManager.getInstance(getActivity()).registerReceiver(downloadProgressReceiver, progressFilter); + Apk apk = getApkToInstall(); + String url = Utils.getApkUrl(apk.repoAddress, apk); + + localBroadcastManager = LocalBroadcastManager.getInstance(getActivity()); + localBroadcastManager.registerReceiver(appListViewResetReceiver, + DownloaderService.getIntentFilter(url, Downloader.ACTION_STARTED)); + localBroadcastManager.registerReceiver(downloadProgressReceiver, + DownloaderService.getIntentFilter(url, Downloader.LOCAL_ACTION_PROGRESS)); + localBroadcastManager.registerReceiver(appListViewResetReceiver, + DownloaderService.getIntentFilter(url, Downloader.ACTION_COMPLETE)); + localBroadcastManager.registerReceiver(appListViewResetReceiver, + DownloaderService.getIntentFilter(url, Downloader.ACTION_INTERRUPTED)); } public void setApp(@NonNull App app) { 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 7edf430c3..7af7262f4 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 @@ -2,6 +2,7 @@ package org.fdroid.fdroid.views.swap; import android.app.Activity; import android.bluetooth.BluetoothAdapter; +import android.content.BroadcastReceiver; import android.content.ComponentName; import android.content.Context; import android.content.DialogInterface; @@ -34,7 +35,6 @@ import com.google.zxing.integration.android.IntentResult; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.NfcHelper; import org.fdroid.fdroid.Preferences; -import org.fdroid.fdroid.ProgressListener; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Apk; @@ -46,6 +46,8 @@ import org.fdroid.fdroid.localrepo.LocalRepoManager; import org.fdroid.fdroid.localrepo.SwapService; import org.fdroid.fdroid.localrepo.peers.Peer; import org.fdroid.fdroid.net.ApkDownloader; +import org.fdroid.fdroid.net.Downloader; +import org.fdroid.fdroid.net.DownloaderService; import java.io.File; import java.util.Arrays; @@ -116,6 +118,8 @@ public class SwapWorkflowActivity extends AppCompatActivity { private boolean hasPreparedLocalRepo; private PrepareSwapRepo updateSwappableAppsTask; private NewRepoConfig confirmSwapConfig; + private LocalBroadcastManager localBroadcastManager; + private BroadcastReceiver downloadCompleteReceiver; @NonNull private final ServiceConnection serviceConnection = new ServiceConnection() { @@ -181,6 +185,8 @@ public class SwapWorkflowActivity extends AppCompatActivity { container = (ViewGroup) findViewById(R.id.fragment_container); + localBroadcastManager = LocalBroadcastManager.getInstance(this); + new SwapDebug().logStatus(); } @@ -779,16 +785,15 @@ public class SwapWorkflowActivity extends AppCompatActivity { public void install(@NonNull final App app) { final Apk apkToInstall = ApkProvider.Helper.find(this, app.packageName, app.suggestedVercode); final ApkDownloader downloader = new ApkDownloader(this, app, apkToInstall, apkToInstall.repoAddress); - downloader.setProgressListener(new ProgressListener() { + downloadCompleteReceiver = new BroadcastReceiver() { @Override - public void onProgress(Event event) { - switch (event.type) { - case ApkDownloader.EVENT_APK_DOWNLOAD_COMPLETE: - handleDownloadComplete(downloader.localFile(), app.packageName); - break; - } + public void onReceive(Context context, Intent intent) { + String path = intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH); + handleDownloadComplete(new File(path), app.packageName); } - }); + }; + localBroadcastManager.registerReceiver(downloadCompleteReceiver, + DownloaderService.getIntentFilter(downloader.urlString, Downloader.ACTION_COMPLETE)); downloader.download(); } @@ -808,6 +813,7 @@ public class SwapWorkflowActivity extends AppCompatActivity { // TODO: Boo! } }).installPackage(apkFile, packageName); + localBroadcastManager.unregisterReceiver(downloadCompleteReceiver); } catch (Installer.AndroidNotCompatibleException e) { // TODO: Handle exception properly } From 2e3d2c84aea28ef25cb9ebb1e1a56688ac1d9a35 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 12 Apr 2016 12:30:39 -0400 Subject: [PATCH 02/15] eliminate Interfaces used only internal to AppDetails class When dealing with complex lifecycles like Fragments, it is important to expose the reliance of the Fragment on the Activity, since they have different lifecycles. Just cast to AppDetails instead of adding complexity with unneeded Interfaces. The actual instance and class will be the same with or without the Interfaces, so it does not help with lifecycle issues. The methods that implement the interfaces only hide the fact that they rely on an active instance of AppDetails, which can lead to lifecycle-related crashes. This is a step along the way to streamlining AppDetails Activity so that it only uses Fragments when they are beneficial. --- .../java/org/fdroid/fdroid/AppDetails.java | 123 +++++------------- 1 file changed, 34 insertions(+), 89 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 690bd0618..5b3dc86a4 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -97,27 +97,7 @@ import java.io.File; import java.util.Iterator; import java.util.List; -interface AppDetailsData { - App getApp(); - - AppDetails.ApkListAdapter getApks(); -} - -/** - * Interface which allows the apk list fragment to communicate with the activity when - * a user requests to install/remove an apk by clicking on an item in the list. - *

- * NOTE: This is not to do with with the sudo/packagemanager/other installer - * stuff which allows multiple ways to install apps. It is only here to make fragment- - * activity communication possible. - */ -interface AppInstallListener { - void install(final Apk apk); - - void removeApk(String packageName); -} - -public class AppDetails extends AppCompatActivity implements AppDetailsData, AppInstallListener { +public class AppDetails extends AppCompatActivity { private static final String TAG = "AppDetails"; @@ -807,7 +787,6 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App } // Install the version of this app denoted by 'app.curApk'. - @Override public void install(final Apk apk) { if (isFinishing()) { return; @@ -881,7 +860,6 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App } } - @Override public void removeApk(String packageName) { try { installer.deletePackage(packageName); @@ -976,12 +954,10 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App } } - @Override public App getApp() { return app; } - @Override public ApkListAdapter getApks() { return adapter; } @@ -989,7 +965,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App public static class AppDetailsSummaryFragment extends Fragment { final Preferences prefs; - private AppDetailsData data; + private AppDetails appDetails; private static final int MAX_LINES = 5; private static boolean viewAllDescription; private static LinearLayout llViewMoreDescription; @@ -1018,15 +994,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App @Override public void onAttach(Activity activity) { super.onAttach(activity); - data = (AppDetailsData) activity; - } - - App getApp() { - return data.getApp(); - } - - ApkListAdapter getApks() { - return data.getApks(); + appDetails = (AppDetails) activity; } @Override @@ -1085,7 +1053,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App private final View.OnClickListener mOnClickListener = new View.OnClickListener() { public void onClick(View v) { String url = null; - App app = getApp(); + App app = appDetails.getApp(); switch (v.getId()) { case R.id.website: url = app.webURL; @@ -1138,7 +1106,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App }; private void setupView(final View view) { - App app = getApp(); + App app = appDetails.getApp(); // Expandable description final TextView description = (TextView) view.findViewById(R.id.description); final Spanned desc = Html.fromHtml(app.description, null, new Utils.HtmlTagHandler()); @@ -1264,8 +1232,8 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App } Apk curApk = null; - for (int i = 0; i < getApks().getCount(); i++) { - final Apk apk = getApks().getItem(i); + for (int i = 0; i < appDetails.getApks().getCount(); i++) { + final Apk apk = appDetails.getApks().getItem(i); if (apk.vercode == app.suggestedVercode) { curApk = apk; break; @@ -1277,7 +1245,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App final TextView permissionHeader = (TextView) view.findViewById(R.id.permissions); final boolean curApkCompatible = curApk != null && curApk.compatible; - if (!getApks().isEmpty() && (curApkCompatible || prefs.showIncompatibleVersions())) { + if (!appDetails.getApks().isEmpty() && (curApkCompatible || prefs.showIncompatibleVersions())) { // build and set the string once buildPermissionInfo(); permissionHeader.setOnClickListener(expanderPermissions); @@ -1310,7 +1278,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App private void buildPermissionInfo() { final TextView permissionListView = (TextView) llViewMorePermissions.findViewById(R.id.permissions_list); - CommaSeparatedList permsList = getApks().getItem(0).permissions; + CommaSeparatedList permsList = appDetails.getApks().getItem(0).permissions; if (permsList == null) { permissionListView.setText(R.string.no_permissions); } else { @@ -1362,7 +1330,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App return; } - App app = getApp(); + App app = appDetails.getApp(); TextView signatureView = (TextView) view.findViewById(R.id.signature); if (prefs.expertMode() && !TextUtils.isEmpty(app.installedSig)) { signatureView.setVisibility(View.VISIBLE); @@ -1375,7 +1343,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App public static class AppDetailsHeaderFragment extends Fragment implements View.OnClickListener { - private AppDetailsData data; + private AppDetails appDetails; private Button btMain; private ProgressBar progressBar; private TextView progressSize; @@ -1396,14 +1364,6 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App .build(); } - private App getApp() { - return data.getApp(); - } - - private ApkListAdapter getApks() { - return data.getApks(); - } - @Override public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { View view = inflater.inflate(R.layout.app_details_header, container, false); @@ -1414,11 +1374,11 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App @Override public void onAttach(Activity activity) { super.onAttach(activity); - data = (AppDetailsData) activity; + appDetails = (AppDetails) activity; } private void setupView(View view) { - App app = getApp(); + App app = appDetails.getApp(); // Set the icon... ImageView iv = (ImageView) view.findViewById(R.id.icon); @@ -1506,13 +1466,13 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App */ @Override public void onClick(View view) { - AppDetails activity = (AppDetails) getActivity(); - if (activity == null || activity.downloadHandler == null) { + AppDetails appDetails = (AppDetails) getActivity(); + if (appDetails == null || appDetails.downloadHandler == null) { return; } - DownloaderService.cancel(getContext(), activity.downloadHandler.urlString); - activity.cleanUpFinishedDownload(); + DownloaderService.cancel(getContext(), appDetails.downloadHandler.urlString); + appDetails.cleanUpFinishedDownload(); setProgressVisible(false); updateViews(); } @@ -1522,21 +1482,20 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App } public void updateViews(View view) { - App app = getApp(); + App app = appDetails.getApp(); TextView statusView = (TextView) view.findViewById(R.id.status); btMain.setVisibility(View.VISIBLE); - AppDetails activity = (AppDetails) getActivity(); - if (activity.downloadHandler != null) { + if (appDetails.downloadHandler != null) { btMain.setText(R.string.downloading); btMain.setEnabled(false); } else if (!app.isInstalled() && app.suggestedVercode > 0 && - activity.adapter.getCount() > 0) { + appDetails.adapter.getCount() > 0) { // Check count > 0 due to incompatible apps resulting in an empty list. // If App isn't installed installed = false; statusView.setText(R.string.details_notinstalled); - NfcHelper.disableAndroidBeam(activity); + NfcHelper.disableAndroidBeam(appDetails); // Set Install button and hide second button btMain.setText(R.string.menu_install); btMain.setOnClickListener(mOnClickListener); @@ -1545,13 +1504,13 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App // If App is installed installed = true; statusView.setText(getString(R.string.details_installed, app.installedVersionName)); - NfcHelper.setAndroidBeam(activity, app.packageName); + NfcHelper.setAndroidBeam(appDetails, app.packageName); if (app.canAndWantToUpdate()) { updateWanted = true; btMain.setText(R.string.menu_upgrade); } else { updateWanted = false; - if (activity.packageManager.getLaunchIntentForPackage(app.packageName) != null) { + if (appDetails.packageManager.getLaunchIntentForPackage(app.packageName) != null) { btMain.setText(R.string.menu_launch); } else { btMain.setText(R.string.menu_uninstall); @@ -1569,8 +1528,8 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App author.setVisibility(View.VISIBLE); } TextView currentVersion = (TextView) view.findViewById(R.id.current_version); - if (!getApks().isEmpty()) { - currentVersion.setText(getApks().getItem(0).version + " (" + app.license + ")"); + if (!appDetails.getApks().isEmpty()) { + currentVersion.setText(appDetails.getApks().getItem(0).version + " (" + app.license + ")"); } else { currentVersion.setVisibility(View.GONE); btMain.setVisibility(View.GONE); @@ -1580,7 +1539,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App private final View.OnClickListener mOnClickListener = new View.OnClickListener() { public void onClick(View v) { - App app = getApp(); + App app = appDetails.getApp(); AppDetails activity = (AppDetails) getActivity(); if (updateWanted && app.suggestedVercode > 0) { Apk apkToInstall = ApkProvider.Helper.find(activity, app.packageName, app.suggestedVercode); @@ -1610,8 +1569,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App private static final String SUMMARY_TAG = "summary"; - private AppDetailsData data; - private AppInstallListener installListener; + private AppDetails appDetails; private AppDetailsSummaryFragment summaryFragment; private FrameLayout headerView; @@ -1619,24 +1577,11 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App @Override public void onAttach(Activity activity) { super.onAttach(activity); - data = (AppDetailsData) activity; - installListener = (AppInstallListener) activity; - } - - void install(final Apk apk) { - installListener.install(apk); + appDetails = (AppDetails) activity; } void remove() { - installListener.removeApk(getApp().packageName); - } - - App getApp() { - return data.getApp(); - } - - ApkListAdapter getApks() { - return data.getApks(); + appDetails.removeApk(appDetails.getApp().packageName); } @Override @@ -1658,13 +1603,13 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App setListAdapter(null); getListView().addHeaderView(headerView); - setListAdapter(getApks()); + setListAdapter(appDetails.getApks()); } @Override public void onListItemClick(ListView l, View v, int position, long id) { - App app = getApp(); - final Apk apk = getApks().getItem(position - l.getHeaderViewsCount()); + App app = appDetails.getApp(); + final Apk apk = appDetails.getApks().getItem(position - l.getHeaderViewsCount()); if (app.installedVersionCode == apk.vercode) { remove(); } else if (app.installedVersionCode > apk.vercode) { @@ -1675,7 +1620,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App @Override public void onClick(DialogInterface dialog, int whichButton) { - install(apk); + appDetails.install(apk); } }); builder.setNegativeButton(R.string.no, @@ -1688,7 +1633,7 @@ public class AppDetails extends AppCompatActivity implements AppDetailsData, App AlertDialog alert = builder.create(); alert.show(); } else { - install(apk); + appDetails.install(apk); } } From 5e59f812ced860383db345f92408b24ed9e1ad12 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 31 Mar 2016 22:01:07 +0200 Subject: [PATCH 03/15] port everything over to new Downloader.ACTION_PROGRESS This replaces all of the previous progress events and listeners. --- .../main/java/org/fdroid/fdroid/AppDetails.java | 2 +- .../java/org/fdroid/fdroid/UpdateService.java | 17 +++++------------ .../java/org/fdroid/fdroid/net/Downloader.java | 3 +-- .../fdroid/fdroid/net/DownloaderFactory.java | 12 ------------ .../fdroid/fdroid/net/DownloaderService.java | 4 ++-- .../fdroid/fdroid/views/swap/SwapAppsView.java | 2 +- 6 files changed, 10 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 5b3dc86a4..99d8c6e01 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -474,7 +474,7 @@ public class AppDetails extends AppCompatActivity { localBroadcastManager.registerReceiver(startedReceiver, DownloaderService.getIntentFilter(url, Downloader.ACTION_STARTED)); localBroadcastManager.registerReceiver(progressReceiver, - DownloaderService.getIntentFilter(url, Downloader.LOCAL_ACTION_PROGRESS)); + DownloaderService.getIntentFilter(url, Downloader.ACTION_PROGRESS)); localBroadcastManager.registerReceiver(completeReceiver, DownloaderService.getIntentFilter(url, Downloader.ACTION_COMPLETE)); localBroadcastManager.registerReceiver(interruptedReceiver, diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index 8f045744b..cfdd18012 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -48,6 +48,7 @@ import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.net.Downloader; +import org.fdroid.fdroid.net.DownloaderService; import java.util.ArrayList; import java.util.List; @@ -129,8 +130,6 @@ public class UpdateService extends IntentService implements ProgressListener { super.onCreate(); localBroadcastManager = LocalBroadcastManager.getInstance(this); - localBroadcastManager.registerReceiver(downloadProgressReceiver, - new IntentFilter(Downloader.LOCAL_ACTION_PROGRESS)); localBroadcastManager.registerReceiver(updateStatusReceiver, new IntentFilter(LOCAL_ACTION_STATUS)); @@ -192,16 +191,7 @@ public class UpdateService extends IntentService implements ProgressListener { private final BroadcastReceiver downloadProgressReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - String action = intent.getAction(); - if (TextUtils.isEmpty(action)) { - return; - } - - if (!action.equals(Downloader.LOCAL_ACTION_PROGRESS)) { - return; - } - - String repoAddress = intent.getStringExtra(Downloader.EXTRA_ADDRESS); + String repoAddress = intent.getDataString(); int downloadedSize = intent.getIntExtra(Downloader.EXTRA_BYTES_READ, -1); String downloadedSizeFriendly = Utils.getFriendlySize(downloadedSize); int totalSize = intent.getIntExtra(Downloader.EXTRA_TOTAL_BYTES, -1); @@ -381,6 +371,8 @@ public class UpdateService extends IntentService implements ProgressListener { sendStatus(this, STATUS_INFO, getString(R.string.status_connecting_to_repo, repo.address)); RepoUpdater updater = new RepoUpdater(getBaseContext(), repo); + localBroadcastManager.registerReceiver(downloadProgressReceiver, + DownloaderService.getIntentFilter(updater.indexUrl, Downloader.ACTION_PROGRESS)); updater.setProgressListener(this); try { updater.update(); @@ -395,6 +387,7 @@ public class UpdateService extends IntentService implements ProgressListener { repoErrors.add(e.getMessage()); Log.e(TAG, "Error updating repository " + repo.address, e); } + localBroadcastManager.unregisterReceiver(downloadProgressReceiver); } if (!changes) { diff --git a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java index 76b7849a6..5dfc80f6e 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java @@ -15,12 +15,11 @@ public abstract class Downloader { private static final String TAG = "Downloader"; - public static final String LOCAL_ACTION_PROGRESS = "Downloader.PROGRESS"; public static final String ACTION_STARTED = "org.fdroid.fdroid.net.Downloader.action.STARTED"; + public static final String ACTION_PROGRESS = "org.fdroid.fdroid.net.Downloader.action.PROGRESS"; public static final String ACTION_INTERRUPTED = "org.fdroid.fdroid.net.Downloader.action.INTERRUPTED"; public static final String ACTION_COMPLETE = "org.fdroid.fdroid.net.Downloader.action.COMPLETE"; - public static final String EXTRA_ADDRESS = "extraAddress"; public static final String EXTRA_DOWNLOAD_PATH = "org.fdroid.fdroid.net.Downloader.extra.DOWNLOAD_PATH"; public static final String EXTRA_BYTES_READ = "org.fdroid.fdroid.net.Downloader.extra.BYTES_READ"; public static final String EXTRA_TOTAL_BYTES = "org.fdroid.fdroid.net.Downloader.extra.TOTAL_BYTES"; diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java index e72eb0ed2..2c2b8cb65 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java @@ -1,7 +1,6 @@ package org.fdroid.fdroid.net; import android.content.Context; -import android.content.Intent; import android.net.Uri; import android.support.v4.content.LocalBroadcastManager; @@ -57,17 +56,6 @@ public class DownloaderFactory { downloader = new HttpDownloader(url, destFile, repo.username, repo.password); } } - - downloader.setListener(new Downloader.DownloaderProgressListener() { - @Override - public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) { - Intent intent = new Intent(Downloader.LOCAL_ACTION_PROGRESS); - intent.putExtra(Downloader.EXTRA_ADDRESS, sourceUrl.toString()); - intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); - intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); - localBroadcastManager.sendBroadcast(intent); - } - }); return downloader; } diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 0c3986610..7f308433a 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -186,7 +186,7 @@ public class DownloaderService extends Service { @Override public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) { //Log.i(TAG, "sendProgress " + sourceUrl + " " + bytesRead + " / " + totalBytes); - Intent intent = new Intent(Downloader.LOCAL_ACTION_PROGRESS); + Intent intent = new Intent(Downloader.ACTION_PROGRESS); intent.setData(uri); intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); @@ -261,7 +261,7 @@ public class DownloaderService extends Service { * Get a prepared {@link IntentFilter} for use for matching this service's action events. * * @param urlString The full file URL to match. - * @param action {@link Downloader#ACTION_STARTED}, {@link Downloader#LOCAL_ACTION_PROGRESS}, + * @param action {@link Downloader#ACTION_STARTED}, {@link Downloader#ACTION_PROGRESS}, * {@link Downloader#ACTION_INTERRUPTED}, or {@link Downloader#ACTION_COMPLETE}, * @return */ diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java index 3a4df7e85..190cd747e 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -290,7 +290,7 @@ public class SwapAppsView extends ListView implements localBroadcastManager.registerReceiver(appListViewResetReceiver, DownloaderService.getIntentFilter(url, Downloader.ACTION_STARTED)); localBroadcastManager.registerReceiver(downloadProgressReceiver, - DownloaderService.getIntentFilter(url, Downloader.LOCAL_ACTION_PROGRESS)); + DownloaderService.getIntentFilter(url, Downloader.ACTION_PROGRESS)); localBroadcastManager.registerReceiver(appListViewResetReceiver, DownloaderService.getIntentFilter(url, Downloader.ACTION_COMPLETE)); localBroadcastManager.registerReceiver(appListViewResetReceiver, From fc9459d6c5b48423a726e37be5caf93e3a75bae8 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 5 Apr 2016 11:08:11 +0200 Subject: [PATCH 04/15] send Downloader progress on a 100 ms timer No need to flood receivers with progress events since they are basically always going to the UI, and the UI will only refresh every so often. If the refresh rate is 50Hz, then that's every 20ms. 100ms seems to make a smooth enough progress bar, and saves some CPU time. This becomes more important if there are multiple downloads happening in the background, like if we make DownloaderService support parallel downloads from different repos --- .../org/fdroid/fdroid/net/Downloader.java | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java index 5dfc80f6e..8be443376 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java @@ -10,6 +10,8 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.MalformedURLException; import java.net.URL; +import java.util.Timer; +import java.util.TimerTask; public abstract class Downloader { @@ -25,6 +27,9 @@ public abstract class Downloader { public static final String EXTRA_TOTAL_BYTES = "org.fdroid.fdroid.net.Downloader.extra.TOTAL_BYTES"; private volatile boolean cancelled = false; + private volatile int bytesRead; + private volatile int totalBytes; + private Timer timer; private final OutputStream outputStream; @@ -42,6 +47,9 @@ public abstract class Downloader { void sendProgress(URL sourceUrl, int bytesRead, int totalBytes); } + /** + * For sending download progress, should only be called in {@link #progressTask} + */ private DownloaderProgressListener downloaderProgressListener; protected abstract InputStream getDownloadersInputStream() throws IOException; @@ -123,6 +131,9 @@ public abstract class Downloader { private void throwExceptionIfInterrupted() throws InterruptedException { if (cancelled) { Utils.debugLog(TAG, "Received interrupt, cancelling download"); + if (timer != null) { + timer.cancel(); + } throw new InterruptedException(); } } @@ -140,16 +151,17 @@ public abstract class Downloader { * progress counter. */ private void copyInputToOutputStream(InputStream input, int bufferSize) throws IOException, InterruptedException { - - int bytesRead = 0; - int totalBytes = totalDownloadSize(); + bytesRead = 0; + totalBytes = totalDownloadSize(); byte[] buffer = new byte[bufferSize]; + timer = new Timer(); + timer.scheduleAtFixedRate(progressTask, 0, 100); + // Getting the total download size could potentially take time, depending on how // it is implemented, so we may as well check this before we proceed. throwExceptionIfInterrupted(); - sendProgress(bytesRead, totalBytes); while (true) { int count; @@ -168,19 +180,25 @@ public abstract class Downloader { } bytesRead += count; - sendProgress(bytesRead, totalBytes); outputStream.write(buffer, 0, count); - } + timer.cancel(); + timer.purge(); outputStream.flush(); outputStream.close(); } - private void sendProgress(int bytesRead, int totalBytes) { - if (downloaderProgressListener != null) { - downloaderProgressListener.sendProgress(sourceUrl, bytesRead, totalBytes); + /** + * Send progress updates on a timer to avoid flooding receivers with pointless events. + */ + private final TimerTask progressTask = new TimerTask() { + @Override + public void run() { + if (downloaderProgressListener != null) { + downloaderProgressListener.sendProgress(sourceUrl, bytesRead, totalBytes); + } } - } + }; /** * Overrides every method in {@link InputStream} and delegates to the wrapped stream. From 0b8796e56fa454a1bc635d74ebeb5eff796d56b3 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 1 Apr 2016 13:19:29 +0200 Subject: [PATCH 05/15] add AndroidManifest parser for reading directly from APKs This will make our algorithm choices a lot more flexible both in terms of the Installer process and the swap repo process. --- .../fdroid/fdroid/AndroidXMLDecompress.java | 177 ++++++++++++++++++ app/src/test/assets/urzip.apk | Bin 0 -> 9969 bytes .../fdroid/AndroidXMLDecompressTest.java | 51 +++++ 3 files changed, 228 insertions(+) create mode 100644 app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java create mode 100644 app/src/test/assets/urzip.apk create mode 100644 app/src/test/java/org/fdroid/fdroid/AndroidXMLDecompressTest.java diff --git a/app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java b/app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java new file mode 100644 index 000000000..b8569f246 --- /dev/null +++ b/app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java @@ -0,0 +1,177 @@ +/* + * Copyright (C) 2016 Hans-Christoph Steiner + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 3 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +/* + Copyright (c) 2016, Liu Dong + All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + + * Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + + * Neither the name of apk-parser nor the names of its + contributors may be used to endorse or promote products derived from + this software without specific prior written permission. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE + FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.fdroid.fdroid; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +/** + * Parse the 'compressed' binary form of Android XML docs such as for + * {@code AndroidManifest.xml} in APK files. This is a very truncated + * version of apk-parser since currently, we only need the header from + * the binary XML AndroidManifest.xml. apk-parser provides full APK + * parsing, which is a lot more than what is needed. + * + * @see apk-parser + * @see Android Internals: Binary XML + * @see a binary XML parser + */ +public class AndroidXMLDecompress { + public static int startTag = 0x00100102; + + /** + * Just get the XML attributes from the {@code } element. + * + * @return A key value map of the attributes, with values as {@link Object}s + */ + public static Map getManifestHeaderAttributes(String filename) throws IOException { + byte[] binaryXml = getManifestFromFilename(filename); + int numbStrings = littleEndianWord(binaryXml, 4 * 4); + int stringIndexTableOffset = 0x24; + int stringTableOffset = stringIndexTableOffset + numbStrings * 4; + int xmlTagOffset = littleEndianWord(binaryXml, 3 * 4); + for (int i = xmlTagOffset; i < binaryXml.length - 4; i += 4) { + if (littleEndianWord(binaryXml, i) == startTag) { + xmlTagOffset = i; + break; + } + } + int offset = xmlTagOffset; + + while (offset < binaryXml.length) { + int tag0 = littleEndianWord(binaryXml, offset); + int nameStringIndex = littleEndianWord(binaryXml, offset + 5 * 4); + + if (tag0 == startTag) { + int numbAttrs = littleEndianWord(binaryXml, offset + 7 * 4); + offset += 9 * 4; + + HashMap attributes = new HashMap(3); + for (int i = 0; i < numbAttrs; i++) { + int attributeNameStringIndex = littleEndianWord(binaryXml, offset + 1 * 4); + int attributeValueStringIndex = littleEndianWord(binaryXml, offset + 2 * 4); + int attributeResourceId = littleEndianWord(binaryXml, offset + 4 * 4); + offset += 5 * 4; + + String attributeName = getString(binaryXml, stringIndexTableOffset, stringTableOffset, attributeNameStringIndex); + Object attributeValue; + if (attributeValueStringIndex != -1) { + attributeValue = getString(binaryXml, stringIndexTableOffset, stringTableOffset, attributeValueStringIndex); + } else { + attributeValue = attributeResourceId; + } + attributes.put(attributeName, attributeValue); + } + return attributes; + } else { + // we only need the first start tag + break; + } + } + return new HashMap(0); + } + + public static byte[] getManifestFromFilename(String filename) throws IOException { + InputStream is = null; + ZipFile zip = null; + int size = 0; + + if (filename.endsWith(".apk")) { + zip = new ZipFile(filename); + ZipEntry ze = zip.getEntry("AndroidManifest.xml"); + is = zip.getInputStream(ze); + size = (int) ze.getSize(); + } else { + throw new RuntimeException("This only works on APK files!"); + } + byte[] buf = new byte[size]; + is.read(buf); + + is.close(); + if (zip != null) { + zip.close(); + } + return buf; + } + + public static String getString(byte[] bytes, int stringIndexTableOffset, int stringTableOffset, int stringIndex) { + if (stringIndex < 0) { + return null; + } + int stringOffset = stringTableOffset + littleEndianWord(bytes, stringIndexTableOffset + stringIndex * 4); + return getStringAt(bytes, stringOffset); + } + + public static String getStringAt(byte[] bytes, int stringOffset) { + int length = bytes[stringOffset + 1] << 8 & 0xff00 | bytes[stringOffset] & 0xff; + byte[] chars = new byte[length]; + for (int i = 0; i < length; i++) { + chars[i] = bytes[stringOffset + 2 + i * 2]; + } + return new String(chars); + } + + /** + * Return the little endian 32-bit word from the byte array at offset + */ + public static int littleEndianWord(byte[] bytes, int offset) { + return bytes[offset + 3] + << 24 & 0xff000000 + | bytes[offset + 2] + << 16 & 0xff0000 + | bytes[offset + 1] + << 8 & 0xff00 + | bytes[offset] & 0xFF; + } +} diff --git a/app/src/test/assets/urzip.apk b/app/src/test/assets/urzip.apk new file mode 100644 index 0000000000000000000000000000000000000000..ee5e5cba868321f3eb85d1e3cc8b50a0a7fd560f GIT binary patch literal 9969 zcmdsdbzB@vv*_aP8iKoPkf6bXI|K;sK^9nCgF~=DZ~_Dft|1U0c<|uvVH4cKA_0QF z*_`iuC->a%-22|Yul85d)z#fqJ=L`}J=LSBf`p6-f14y>U}ffH;pt*!Z{yDG>SA~M83c;aR9D2oq<{;+QF<(=4V2_?L`Ma# zP;ZF~5C}z8Nlse#aeq;YujaHTd0!rle{JWdx*_hCp}HYNERmkCq0yzZ$f0CH{7@0Z zf+AdBV)w|Gw3`qK%jw(G(4tREE0_; z%K-G7?WV5@- zJ{kyGmGX$Sw!?kv7Btj1d-fxG8)cSn4V^o1_1T6fdV+>SZlffI95UE@P?@D&w(-@g z=i1u($;xH*m^Qm=AkEW3-vIvr;gsr8^dktKcGKXJ`s}RM#C_j&)|A)i@hYV1F701q znithlD2a)u{kL6FW@d7VCzGj&sfA)M5Va~q?Q>g(Iq<1SIlgeIqJew{!WR$_Le}%o z_8})TkxIHjuhiPV#WX)np`@f-_Rr6c$<@ER@z>RT@`y0AU@_RMQZ*mr6DvVMJw;FC zPv4`1rkpcGHxs`>X%`uVFVmYw&X2)X5G(=$9Bl0I`R*udp=<|x`)F)g*;89Bx2~upIlH#Kl^q?PCjL;h@@MX_rRjZge6KZ_Q zNKYTTAlEXa?BMx~V#69-IDINOJ+-80Rj9CN`cjcT=DO+8Jz9OApJCDDsZRrR8FlOu zH4gGUV(~F@$V5fwZ59{YHB_FOM2+QMS$_JkJh^8-Rgqcma$r*@cloiAP(cEN z6sr>$HZ5N)`Sn?9D0t&~2+i-IU6=3mWcJY@Gr!mC`nniu*WVqS^s4h7#?cWcchm?S z1s>=Ose3y)A@{aQiw|oJCk$zOcUR%fUECIGz>OY}vRAw3RHTJ!53kS~#wA*OuT`Sz zEtTI*hWcE><)z<~s?+67!-eL{v+wPy?NnQepkRb>nJ)Lt3X!^!7KRDli#OM*&T1Mn zgE~{fU>7-kuj<>rk=*#Oz!e<19@WK+WPF4I?!Gx$inXo1D}S3GgBWv1C+{2TnXRY{ zIcJZIRCJIW8jK|7lqk^5C!>U@i!QI3_Lof5opS<5IHzEJRJ{=gdV&o)>R%?8qn(>k zRvHiQH;N0L3hoCU9_+|}X=FhI@pj98?3|k`sjAzY4o9|)jmV29K=4|2M!tLYi zf~ic5;rLXvwdmhiw483)eHbyRBNRhOGsJRiAfaOM-e6nrww8N=m}q?&YO*zF`ySWt zYPt3?6+czLm|L-ssvmB|JoKKXJn3ZGI<}phC3IF``yuloEo;yCnEX2Tn@eh=Zsk~1 z6~5m4@w}3cBzAOn3P~id7=!3Nxt-aAx>EPG-|6|>Rqu_7UTFt{8|M9g%9m(#M1{>S zknSn zcoGUt&+hgbEl4%A-k^td-0=2k z6lCp?6hwNu6_&f%WC}oNv|i|lO0R2W<$4m0=&YSji)ct=jjL@VoFZT1UJ_iATvA;k z`Qf&OCsS*Y`e6s+1R~uaON8A<(i0Rs&<&-QU6qxR4VJ!pL{=bY_Ne0#Pd*l3fsH+m zQB-?Gu1v5D&zl~Cb+kZiiIAW$zZNLo4d@5$9#{DUwVr+Y_bH}p@F(}CEs9hJ#UH2n zRh@N+WB&DCow4>WcVU$iRP%eddDK2mYx6~84g_2GVGAYfewMHW{2B%r>y)TvqnoGh zMgwk#n8-Ef9H0LjV{QRo_ zd*6@1xF=y?6fS)n(|xTX@3QDZX|A>9#_K@ASgMOu0yjKooOb`+OWO2ld%ZFl!x^p& z41fLY4=*Yz+WPc5;vdtPUX_2&u6<{wM<5k;h#C-hI=fAG*;K5>-o;;2dS%X613LPQ z?5NfIu8Mt3Z@hxo;Nj5^q|$^Jgd-j)VHz}@VGKOf^k-~h2Dx`w<_0fSY6)BYM5Aa+ zrt?L!CvFkI)IaD=dPg^!ks(Vmvn|n^N&QB(`{;RTT`eeIm_Ev=?B;@b@<`-N3WuUq zyono?=LoWQSkH)b*GO$^U%Xg;)Pg)|zeLg$*L1E&lF{QmRea1=5t!C{JId}@`@-1S zz#eCTYa%N8K3J?#zH-2`r<%4yJ~O}Mct-)8Ci$tOoyG6AVO|0$%FiirO&xCuQd(3( z#f>=hia{oz+I*_5#;Xv1T&Gu@wZdzImvIn4y9z}GKp`{eRI)p9%%J|s*jnX#o!3c_S`ySzzf9I23(;}PU9`F$F4%V4 zp^?kRH(FLu5Q%%g4bE@#(faBJVP;vyfu5>WSm~NFn}~`l{Tl ztwqgPZsFto56F!-yGm~3#an{ralYk)ty%H+47{Ie7Sas{Yws2tDp~B%T(xrjn3e4Q zwsvo=cf&YwfvwQi>-A>NgZ&b!)_T|^>R7_mna(Wb>2jpZFxNdx7dysufxZl-{^dwn zm_oR)B`lJvRTuqBDNF&!^mCU=ROw9ytPeYvs^t7zv;_^gj_hZ9CjOW3`GoQ<2lF9v zt2jeWXBzLyL%HZ5BjKg1Z`b)9;xgT)!R6hTYJz1nC&jX`;&5RbSTR-W0`s25u^(fa z?-YC%t<9w3b4H^NeHKW?{X6VUXS;?Ppg+omT{^S*d>*3l(1Z$ev>Jw`d&4m4d?25) z1$2c2`Tkb%uS;U6*WkF9275M;kIyxSRsP2f3*PN z5QG4{3Ty)<@J{e=;QcoR0*M2(4}g&XW&ubEatGOfARr!)H9%Q_yaBWXIRQK#kORmH zWCqX{AWx7BK-&XccMvzo6{y+$m$C3Zz92Br&jT1|0rd3%MtcG10r1U0&H&E=7{v|f zb_OKE=jRE?@&mZ8Kn)$F1IYIHH3}{p0`Rawj{%+&@B@Qrfq8*}{!YLw>3}u>5GfJf zAb~)^0120D3$$|sBRv64?m#aGpmYIby94}ZfE@UYxc`gJUsC?RxKIKM4>$%W8YtoG z4)+1Ljp4jMf%8uSD6s*g1rY8R$bg;Tc6SG8^#A4qAP_1b>94hg*CD@p!)*@t0l2Ov zp!_F)qX61O08|Cg1wbrdW&V>_0q>-Mo`3Qi_}naj)pP}X$pxUCffWUT@cuml3~aC1|5ZL*4qQIGgwG$ogM_d7FNHw)C&2Z10F^)6w~w$<-_an@}Ku)SU{_llLZ6K(MwoF?jJA0{cb$hyDX7=GrJUa((o ziCyGXIqi9%t9Kx(_f%MKe7ZJ7#C4Lx-lU!l@d;P5jI++0h#N&r#wvf>$MJ|e=iaUl z3z!-hB~QK_wyg>tdXEf~?r+SG=Xg&wN?tF~Jbf$0Eje{O5w_|i0eiJ275sIe<9oo} zT1Ac=6TMMxGl$adpB9^=7M*Vu!C_?KpLF}rk$>oPGX6vs^ngFxwh5L%JBLkzkMI`2arq32yo&=x2XOa~v#yoi1FJnA`~ zDdOSt!?46hq}~F!>Y?dfja^bmkwM>)^H7JN{7sl&knTXYNIga|AG;)=(zT!pEzev1--U2qUh z#Vai}kMJFZ!5mv0nHn`Q1f>>zG{mm!1Zo4ryl{s3L!Du0(6O#7sOSaX1sGpFpKBcH zL+FY*F(ryds5ong095>f{sPC9q!r~0N-#`TNIbE0jeFsNxQ*#mP^`nd2V29F5y0?y89 zW>w!s(8ou@ywZ5vOb^F|Lq^Q8wvlekK~5;%&z+x{V24tYBZ_sZyRtN4E#rJgI0t5i zM~=z>)iI|ON5>6ib4Bl&xy*KkqhnVY!nA2A5{@Od~dWO|zTB3<-GtYzMQ&&nqCsgQ+l*xLUDAuF- z;-?(=awlmN5)A1C{j`fXl-rpvSx$N#`DNtkWY;m-_7V%XcrX`~(j)c@3}VK346Bs} z!;?ppnNyv*c9ZK&28jcW2FL^T2T+;1Q^@#LIvA~^l9^2i&pV`3!19537z=8N==Vxq zDd!T_MdWH7qc8=lRUXx|u121&$?SS)RbIu_Hgfoj*{+eiBNdI1C|Xprm>~;~d&Occ z_u79sh?*aVi*Z&lB}khb-@s4$kS0(_VVkKXwnss-CG$zW=W4Mx5iEuow!tlH3^U8WUZY zB;@SC3!v!>qvBujAi9f!Nnr+;%8s7@xV;Nq4dt1o3*5V*)BQLwJ3!!rK z$14kryQ6~gh29x%fc4Hlut8};?x-$CaGQBd zO-P5qSsv?89lFf%!?A((R6_4qz4y=Z$`_R88SEqGe!42;BD9f@B{x$!HX0mCSFgZ6 z#`sm9MZR>zogx?9UU;cc{gB>6xJ!fVyJOFn!Hww8-H5)D5k?eGYG##-@?Au`p1wz` z7*KPP-ix=afu0);4?xBQ?rj!i+dtB{PupA*a$w1~E&9Dcnj=#!+KIuYp)F-bN~2^1 zUA^%(@5ukv`H8B+XcDPVbkH)#>FO z%%f$~7I|^Aj))s?u*Kaa*FL!B{YkXzm9T*O0j1TG^O%>go$|5ZCL>fli`JUR7i)QC z?>vYeURc#*{UqR^C^DT5%b3aUp|13h^op+S>n}qi*jx+WuA8!@+Jz8_$?vS3c8&SJ z%NwXmnyJgcx6W=g%sgdra3xvlHp+VE^(9M*8pr zuMUM>oEdS{^MGzwA~By;>>xbU&xb+<_0L`v)uDc8iIbGymCs9)%WHq}(KqkY%WFxe z(-xdux63Tng(#g`1F)jmB&#ROrqkf^i&wS1w2-x=s)2{kuVNg!DZq!gd{M?)zx^c zq`ejE0W&U4u=r<88!HLcA+F-K$2acEu}D|Qyv8z`W7wA}aQ-Y0r5 zs?d+W)8MK3`SZgAW}ynt&Ys)mw`c7;a=H=*%2!i@{;KmoMPR4O`jT_iBbNr#!#ULP z6T5L^1uYeqr}in5&4k#e?PM+T$*Gar^@&XWLPFrew?Y!6g|ebk$%8xZO2?l%?O_n0 z-R9oA#*zoCvdk8V%@tkE6-DW0l|7Jpb=Gy(`7uDXjjVEWs;3$J!pC)?BEUGcRd$d| z8JD%03${EuEr6_Zz)xVsW~?W9TiHDg5!V@mskROA{@AaMd0(q?T7<`ywHs(&5%<#Y zNOckwtH-@ZfmQrllR65A#UU?~LWTM}-pyvn=rvTyaj+nbZ%ELj#&R=oseInH>qzZI z;!*h8=+Ij?^MKQdt1`&)xx}jYdnmEqk-7a?Shb8W5 zu@|(Ix!L!SFM+6;RlupfUw(alINF8{Lz6ZEWpS<|cpdZDHTCR>d9HjxfqA4DIevnjd z>_&QEoFY;l$$Wy5o$*}Uugj}$9Fb4fdYsU2KaW)1X_9&WKiOiw!gQk&xv|F9NYMQ2eOMSMx!nbQIFd z=AvV(CxAsQRipI`!QIvM4}eG0`QO^od_rUisRK?I{IAr3+>#}{m71u7hCYCj|qv|@F1 z(#QMb?bcV)OfNWc*5B;lQb18Cb8>BEBS)V+DG_VH+K+j>rsFfO7~vMA1XIE4=((^F zBQg-5mVRxS7yrQTUYd(LjP_b|C>Wti#OO?;;njWao%}7sOYdCEDz$VEui_hM?v=;Q z!?di)og*>a-NlC=tgOFfNJq{_;C{CzGqhgvlBgdSD1AS6N;&1gEKza4=6w+9q@d%J z11bN5?(sC?@P^v2+kWj}Ow<+LkI?Tj96tG91GCIK;?##n%dMk??lK>*4V)dYw`H37 zUD$xgW|w*vvmeLS^1S!Zu9(7&;^*KL)R{_{t1od}tg+X!KEy7%Kc%)K_t7xTg(L{) zmXE9N=But~QOS4p1LcFq_lzWj-+3sc+3b*d(Wo5sR?9E?2F;A^p(x~^Xu5UXU-wO; zAmLHS+}_fcnM;m)kYi&x9-Q)^J~ho}TZW_1v5LajVGVC-1oqTnjj3Lh@Ulf_PJpY` z<9;v-mJRu>u`f=6-2M7gFUx$*b@}%cxEAXgEt1BtU>*#W;DL9XhxP(3;j{GWtao$-MPjD^FwMfiNeNZY|rLOR-XDBSSq1& zQU(z-TE}U^I$dTo8Hsc=CJ9p{Om=il9%{WU?vPR%Xxd>n@wvYovsm-^so_gjzx?uL zn@0;$WC^nAW*J$|OVybC?#C{tJrUQ7LF%`m;$#?U%;=fA=yo$yZ0E}d8e*|bG5b~? zSiWwa+-|3_kF}emWV6uCIIap(XDe|qmbGYcg^qHpL|8uOhPL_ccObwGyT5nDNd_b* z@&Wv3!@@HR|2C|wytXd4j)L>35;*h`2n8w@a~EQjjf(6{tmbF_z7&Csp38KY7h6Qs zNgyn=qrF(AptvzTy_ILI-vQT0Es2=+vujeKyq>;js1gB`H7)L}1wZdpyci`RPb~Cs zyWILKF&}p`ZU3NPTrV@8e!51ZYMaN9M%2M@fP;UQWN6g4Dh3Z^JZpQ-TlQdq=D`6f z5mLMScW&oc5FL3_6ZI6owyvJQIc1?XH&)A)j8K7S^j*s)hBu>MskNGir_jc=FVzL* zY0{*h*3smHF4U{|_9w$*!pMxz-*Mm^*&QWw&9mb7JN(?>;e38(KvQH$fj$s*4~e7A~d%dEi6Od7nT zpo3)EM4_Pnh7?kZF`Uoo^hq_pD-sbkfEG8Mx5dg-Su4o)MSeyI`r~lJ3^MitXmXZr zmJ2P*sH45LxwXBmwY9am6E)MQBjP&6lSj?#_m@Rz7UJV7v4`W^uuoOSe?R*MEe8~fg!Okks6D2N@~o)K5T^<%ksSMTNP5t0@ z*Y6`|?q!e~{rwYR!_U=$qHS#r3M#7_LW7t2`-}ehb9#w8Lr#nS#^viE$MQDK549g| zUVln!-gTVgpD%Xf`C7w&8|>I01Dah&;|OY(=m}&zpu1Y?U}stxSiPBw7`GPh@1 zeIF|S^2XGHT4m&J*uv=5vqo|uNk01kbX7dmGix;Q2Z6LqEYtSp4oli-h)#xrf}?uIQ2&`XEq{Yim4?zEqc z24A;gM{ZUsC~WLDmr0tsh<+ZAAJ1y+<+-dyZ?t^Y6tzp7&ovg_NTlKns1I`hd(Su& znl_HsBysJLR$bTnj7IDy_UD;#lL(;!Api}16z3A(49Fj%135!Y6$C^~&>x2!|LJ=s zz(s$9|LyB0xHMuQh5P5x`tJwC@Wkj}&<*6O|EGiYzf1c4SO$Ke{1=d+{m0|w-*424l?>HLH&OK%L#iP literal 0 HcmV?d00001 diff --git a/app/src/test/java/org/fdroid/fdroid/AndroidXMLDecompressTest.java b/app/src/test/java/org/fdroid/fdroid/AndroidXMLDecompressTest.java new file mode 100644 index 000000000..ead537730 --- /dev/null +++ b/app/src/test/java/org/fdroid/fdroid/AndroidXMLDecompressTest.java @@ -0,0 +1,51 @@ +package org.fdroid.fdroid; + +import org.junit.Test; + +import java.io.File; +import java.io.FilenameFilter; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +public class AndroidXMLDecompressTest { + + String[] testDirNames = { + System.getProperty("user.dir") + "/src/test/assets", + System.getProperty("user.dir") + "/build/outputs/apk", + System.getenv("HOME") + "/fdroid/repo", + }; + + FilenameFilter apkFilter = new FilenameFilter() { + @Override + public boolean accept(File dir, String filename) { + return filename.endsWith(".apk"); + } + }; + + @Test + public void testParseVersionCode() throws IOException { + for (File f : getFilesToTest()) { + System.out.println("\n" + f); + Map map = AndroidXMLDecompress.getManifestHeaderAttributes(f.getAbsolutePath()); + for (String key : map.keySet()) { + System.out.println(key + "=\"" + map.get(key) + "\""); + } + } + } + + private List getFilesToTest() { + ArrayList apkFiles = new ArrayList(5); + for (String dirName : testDirNames) { + System.out.println("looking in " + dirName); + File dir = new File(dirName); + File[] files = dir.listFiles(apkFilter); + if (files != null) { + apkFiles.addAll(Arrays.asList(files)); + } + } + return apkFiles; + } +} From d07c8b5d744c460b0444840b1fadb3de199814dc Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 1 Apr 2016 13:25:06 +0200 Subject: [PATCH 06/15] convert Installer's Exception into generic InstallFailedException AndroidNotCompatibleException is not used for anything specific right now, and in the process of adding verification to the start of the install process, there will be other kinds of failures. So convert that Exception into a general usage InstallFailedException. --- .../java/org/fdroid/fdroid/AppDetails.java | 21 ++++++++-------- .../fdroid/installer/DefaultInstaller.java | 10 ++++---- .../installer/DefaultSdk14Installer.java | 10 ++++---- .../fdroid/fdroid/installer/Installer.java | 24 +++++++++++-------- .../fdroid/installer/PrivilegedInstaller.java | 16 ++++++------- .../views/swap/SwapWorkflowActivity.java | 2 +- 6 files changed, 43 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 99d8c6e01..f06c80995 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -87,7 +87,7 @@ import org.fdroid.fdroid.data.InstalledAppProvider; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.installer.Installer; -import org.fdroid.fdroid.installer.Installer.AndroidNotCompatibleException; +import org.fdroid.fdroid.installer.Installer.InstallFailedException; import org.fdroid.fdroid.installer.Installer.InstallerCallback; import org.fdroid.fdroid.net.ApkDownloader; import org.fdroid.fdroid.net.Downloader; @@ -501,11 +501,18 @@ public class AppDetails extends AppCompatActivity { } }; + /** + * Starts the install process one the download is complete. + */ private final BroadcastReceiver completeReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { File localFile = new File(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH)); - installApk(localFile); + try { + installer.installPackage(localFile, app.packageName); + } catch (InstallFailedException e) { + Log.e(TAG, "Android not compatible with this Installer!", e); + } cleanUpFinishedDownload(); } }; @@ -852,18 +859,10 @@ public class AppDetails extends AppCompatActivity { headerFragment.startProgress(); } - private void installApk(File file) { - try { - installer.installPackage(file, app.packageName); - } catch (AndroidNotCompatibleException e) { - Log.e(TAG, "Android not compatible with this Installer!", e); - } - } - public void removeApk(String packageName) { try { installer.deletePackage(packageName); - } catch (AndroidNotCompatibleException e) { + } catch (InstallFailedException e) { Log.e(TAG, "Android not compatible with this Installer!", e); } } 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 34c214a88..a6fa2b9be 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java @@ -39,7 +39,7 @@ public class DefaultInstaller extends Installer { private final Activity mActivity; public DefaultInstaller(Activity activity, PackageManager pm, InstallerCallback callback) - throws AndroidNotCompatibleException { + throws InstallFailedException { super(activity, pm, callback); this.mActivity = activity; } @@ -48,7 +48,7 @@ public class DefaultInstaller extends Installer { private static final int REQUEST_CODE_DELETE = 1; @Override - protected void installPackageInternal(File apkFile) throws AndroidNotCompatibleException { + protected void installPackageInternal(File apkFile) throws InstallFailedException { Intent intent = new Intent(); intent.setAction(Intent.ACTION_VIEW); intent.setDataAndType(Uri.fromFile(apkFile), @@ -56,12 +56,12 @@ public class DefaultInstaller extends Installer { try { mActivity.startActivityForResult(intent, REQUEST_CODE_INSTALL); } catch (ActivityNotFoundException e) { - throw new AndroidNotCompatibleException(e); + throw new InstallFailedException(e); } } @Override - protected void deletePackageInternal(String packageName) throws AndroidNotCompatibleException { + protected void deletePackageInternal(String packageName) throws InstallFailedException { try { PackageInfo pkgInfo = mPm.getPackageInfo(packageName, 0); @@ -70,7 +70,7 @@ public class DefaultInstaller extends Installer { try { mActivity.startActivityForResult(intent, REQUEST_CODE_DELETE); } catch (ActivityNotFoundException e) { - throw new AndroidNotCompatibleException(e); + throw new InstallFailedException(e); } } catch (PackageManager.NameNotFoundException e) { // already checked in super class diff --git a/app/src/main/java/org/fdroid/fdroid/installer/DefaultSdk14Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/DefaultSdk14Installer.java index 6f4d18f4d..c0b1d09d0 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultSdk14Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultSdk14Installer.java @@ -42,7 +42,7 @@ public class DefaultSdk14Installer extends Installer { private final Activity mActivity; public DefaultSdk14Installer(Activity activity, PackageManager pm, InstallerCallback callback) - throws AndroidNotCompatibleException { + throws InstallFailedException { super(activity, pm, callback); this.mActivity = activity; } @@ -52,7 +52,7 @@ public class DefaultSdk14Installer extends Installer { @SuppressWarnings("deprecation") @Override - protected void installPackageInternal(File apkFile) throws AndroidNotCompatibleException { + protected void installPackageInternal(File apkFile) throws InstallFailedException { Intent intent = new Intent(); intent.setAction(Intent.ACTION_INSTALL_PACKAGE); intent.setData(Uri.fromFile(apkFile)); @@ -68,12 +68,12 @@ public class DefaultSdk14Installer extends Installer { try { mActivity.startActivityForResult(intent, REQUEST_CODE_INSTALL); } catch (ActivityNotFoundException e) { - throw new AndroidNotCompatibleException(e); + throw new InstallFailedException(e); } } @Override - protected void deletePackageInternal(String packageName) throws AndroidNotCompatibleException { + protected void deletePackageInternal(String packageName) throws InstallFailedException { try { PackageInfo pkgInfo = mPm.getPackageInfo(packageName, 0); @@ -83,7 +83,7 @@ public class DefaultSdk14Installer extends Installer { try { mActivity.startActivityForResult(intent, REQUEST_CODE_DELETE); } catch (ActivityNotFoundException e) { - throw new AndroidNotCompatibleException(e); + throw new InstallFailedException(e); } } catch (PackageManager.NameNotFoundException e) { // already checked in super class 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 5eeb5d997..9517c6d78 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -49,11 +49,15 @@ public abstract class Installer { * RootInstaller or due to an incompatible Android version in case of * SystemPermissionInstaller */ - public static class AndroidNotCompatibleException extends Exception { + public static class InstallFailedException extends Exception { private static final long serialVersionUID = -8343133906463328027L; - public AndroidNotCompatibleException(Throwable cause) { + public InstallFailedException(String message) { + super(message); + } + + public InstallFailedException(Throwable cause) { super(cause); } } @@ -78,7 +82,7 @@ public abstract class Installer { } Installer(Context context, PackageManager pm, InstallerCallback callback) - throws AndroidNotCompatibleException { + throws InstallFailedException { this.mContext = context; this.mPm = pm; this.mCallback = callback; @@ -104,7 +108,7 @@ public abstract class Installer { try { return new PrivilegedInstaller(activity, pm, callback); - } catch (AndroidNotCompatibleException e) { + } catch (InstallFailedException e) { Log.e(TAG, "Android not compatible with SystemInstaller!", e); } } else { @@ -119,7 +123,7 @@ public abstract class Installer { Utils.debugLog(TAG, "try default installer for Android >= 4"); return new DefaultSdk14Installer(activity, pm, callback); - } catch (AndroidNotCompatibleException e) { + } catch (InstallFailedException e) { Log.e(TAG, "Android not compatible with DefaultInstallerSdk14!", e); } } else { @@ -128,7 +132,7 @@ public abstract class Installer { Utils.debugLog(TAG, "try default installer for Android < 4"); return new DefaultInstaller(activity, pm, callback); - } catch (AndroidNotCompatibleException e) { + } catch (InstallFailedException e) { Log.e(TAG, "Android not compatible with DefaultInstaller!", e); } } @@ -137,7 +141,7 @@ public abstract class Installer { return null; } - public void installPackage(File apkFile, String packageName) throws AndroidNotCompatibleException { + public void installPackage(File apkFile, String packageName) throws InstallFailedException { // check if file exists... if (!apkFile.exists()) { Log.e(TAG, "Couldn't find file " + apkFile + " to install."); @@ -172,7 +176,7 @@ public abstract class Installer { installPackageInternal(apkFile); } - public void deletePackage(String packageName) throws AndroidNotCompatibleException { + public void deletePackage(String packageName) throws InstallFailedException { // check if package exists before proceeding... try { mPm.getPackageInfo(packageName, 0); @@ -201,10 +205,10 @@ public abstract class Installer { } protected abstract void installPackageInternal(File apkFile) - throws AndroidNotCompatibleException; + throws InstallFailedException; protected abstract void deletePackageInternal(String packageName) - throws AndroidNotCompatibleException; + throws InstallFailedException; public abstract boolean handleOnActivityResult(int requestCode, int resultCode, Intent data); } 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 887115124..ed9419d4c 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java @@ -86,7 +86,7 @@ public class PrivilegedInstaller extends Installer { public static final int IS_EXTENSION_INSTALLED_PERMISSIONS_PROBLEM = 3; public PrivilegedInstaller(Activity activity, PackageManager pm, - InstallerCallback callback) throws AndroidNotCompatibleException { + InstallerCallback callback) throws InstallFailedException { super(activity, pm, callback); this.mActivity = activity; } @@ -156,7 +156,7 @@ public class PrivilegedInstaller extends Installer { } @Override - protected void installPackageInternal(File apkFile) throws AndroidNotCompatibleException { + protected void installPackageInternal(File apkFile) throws InstallFailedException { Uri packageUri = Uri.fromFile(apkFile); int count = newPermissionCount(packageUri); if (count < 0) { @@ -171,7 +171,7 @@ public class PrivilegedInstaller extends Installer { } else { try { doInstallPackageInternal(packageUri); - } catch (AndroidNotCompatibleException e) { + } catch (InstallFailedException e) { mCallback.onError(InstallerCallback.OPERATION_INSTALL, InstallerCallback.ERROR_CODE_OTHER); } @@ -194,7 +194,7 @@ public class PrivilegedInstaller extends Installer { return 1; } - private void doInstallPackageInternal(final Uri packageURI) throws AndroidNotCompatibleException { + private void doInstallPackageInternal(final Uri packageURI) throws InstallFailedException { ServiceConnection mServiceConnection = new ServiceConnection() { public void onServiceConnected(ComponentName name, IBinder service) { IPrivilegedService privService = IPrivilegedService.Stub.asInterface(service); @@ -238,7 +238,7 @@ public class PrivilegedInstaller extends Installer { @Override protected void deletePackageInternal(final String packageName) - throws AndroidNotCompatibleException { + throws InstallFailedException { ApplicationInfo appInfo; try { appInfo = mPm.getApplicationInfo(packageName, PackageManager.GET_UNINSTALLED_PACKAGES); @@ -273,7 +273,7 @@ public class PrivilegedInstaller extends Installer { public void onClick(DialogInterface dialog, int which) { try { doDeletePackageInternal(packageName); - } catch (AndroidNotCompatibleException e) { + } catch (InstallFailedException e) { mCallback.onError(InstallerCallback.OPERATION_DELETE, InstallerCallback.ERROR_CODE_OTHER); } @@ -293,7 +293,7 @@ public class PrivilegedInstaller extends Installer { } private void doDeletePackageInternal(final String packageName) - throws AndroidNotCompatibleException { + throws InstallFailedException { ServiceConnection mServiceConnection = new ServiceConnection() { public void onServiceConnected(ComponentName name, IBinder service) { IPrivilegedService privService = IPrivilegedService.Stub.asInterface(service); @@ -344,7 +344,7 @@ public class PrivilegedInstaller extends Installer { final Uri packageUri = data.getData(); try { doInstallPackageInternal(packageUri); - } catch (AndroidNotCompatibleException e) { + } catch (InstallFailedException e) { mCallback.onError(InstallerCallback.OPERATION_INSTALL, InstallerCallback.ERROR_CODE_OTHER); } 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 7af7262f4..2137ccf5e 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 @@ -814,7 +814,7 @@ public class SwapWorkflowActivity extends AppCompatActivity { } }).installPackage(apkFile, packageName); localBroadcastManager.unregisterReceiver(downloadCompleteReceiver); - } catch (Installer.AndroidNotCompatibleException e) { + } catch (Installer.InstallFailedException e) { // TODO: Handle exception properly } } From ee37f26c4d5dffd2301f668a185171e33a035428 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 12 Apr 2016 12:31:30 -0400 Subject: [PATCH 07/15] verify APKs on install, and purge ApkDownloader This moves the last piece of code to the DownloaderService model. Moving the APK prep and verification to Installer.installPackage() makes it hard to mess up and make bugs like installing from the External Storage issue found by the Cure53 audit. --- .../java/org/fdroid/fdroid/AppDetails.java | 50 +++--- .../java/org/fdroid/fdroid/FDroidApp.java | 1 - .../main/java/org/fdroid/fdroid/Utils.java | 24 +-- .../fdroid/fdroid/installer/Installer.java | 111 +++++++++--- .../org/fdroid/fdroid/net/ApkDownloader.java | 160 ------------------ .../fdroid/fdroid/net/DownloaderService.java | 7 - .../views/swap/SwapWorkflowActivity.java | 9 +- 7 files changed, 113 insertions(+), 249 deletions(-) delete mode 100644 app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index f06c80995..6434eaf7c 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -89,7 +89,6 @@ import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.installer.Installer.InstallFailedException; import org.fdroid.fdroid.installer.Installer.InstallerCallback; -import org.fdroid.fdroid.net.ApkDownloader; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.DownloaderService; @@ -304,7 +303,7 @@ public class AppDetails extends AppCompatActivity { private App app; private PackageManager packageManager; - private ApkDownloader downloadHandler; + private String activeDownloadUrlString; private LocalBroadcastManager localBroadcastManager; private boolean startingIgnoreAll; @@ -324,17 +323,15 @@ public class AppDetails extends AppCompatActivity { */ private static class ConfigurationChangeHelper { - public final ApkDownloader downloader; + public final String urlString; public final App app; - ConfigurationChangeHelper(ApkDownloader downloader, App app) { - this.downloader = downloader; + ConfigurationChangeHelper(String urlString, App app) { + this.urlString = urlString; this.app = app; } } - private boolean inProcessOfChangingConfiguration; - /** * Attempt to extract the packageName from the intent which launched this activity. * @return May return null, if we couldn't find the packageName. This should @@ -375,8 +372,8 @@ public class AppDetails extends AppCompatActivity { ConfigurationChangeHelper previousData = (ConfigurationChangeHelper) getLastCustomNonConfigurationInstance(); if (previousData != null) { Utils.debugLog(TAG, "Recreating view after configuration change."); - downloadHandler = previousData.downloader; - if (downloadHandler != null) { + activeDownloadUrlString = previousData.urlString; + if (activeDownloadUrlString != null) { Utils.debugLog(TAG, "Download was in progress before the configuration change, so we will start to listen to its events again."); } app = previousData.app; @@ -438,10 +435,9 @@ public class AppDetails extends AppCompatActivity { * Remove progress listener, suppress progress bar, set downloadHandler to null. */ private void cleanUpFinishedDownload() { - if (downloadHandler != null) { - headerFragment.removeProgress(); - downloadHandler = null; - } + activeDownloadUrlString = null; + headerFragment.removeProgress(); + unregisterDownloaderReceivers(); } protected void onStop() { @@ -458,7 +454,6 @@ public class AppDetails extends AppCompatActivity { setIgnoreUpdates(app.packageName, app.ignoreAllUpdates, app.ignoreThisUpdate); } unregisterDownloaderReceivers(); - headerFragment.removeProgress(); } private void unregisterDownloaderReceivers() { @@ -469,8 +464,8 @@ public class AppDetails extends AppCompatActivity { } private void registerDownloaderReceivers() { - if (downloadHandler != null) { // if a download is active - String url = downloadHandler.urlString; + if (activeDownloadUrlString != null) { // if a download is active + String url = activeDownloadUrlString; localBroadcastManager.registerReceiver(startedReceiver, DownloaderService.getIntentFilter(url, Downloader.ACTION_STARTED)); localBroadcastManager.registerReceiver(progressReceiver, @@ -550,17 +545,12 @@ public class AppDetails extends AppCompatActivity { @Override public Object onRetainCustomNonConfigurationInstance() { - inProcessOfChangingConfiguration = true; - return new ConfigurationChangeHelper(downloadHandler, app); + return new ConfigurationChangeHelper(activeDownloadUrlString, app); } @Override protected void onDestroy() { - if (downloadHandler != null && !inProcessOfChangingConfiguration) { - DownloaderService.cancel(context, downloadHandler.urlString); - cleanUpFinishedDownload(); - } - inProcessOfChangingConfiguration = false; + cleanUpFinishedDownload(); super.onDestroy(); } @@ -853,10 +843,11 @@ public class AppDetails extends AppCompatActivity { } private void startDownload(Apk apk, String repoAddress) { - downloadHandler = new ApkDownloader(getBaseContext(), app, apk, repoAddress); + String urlString = Utils.getApkUrl(repoAddress, apk); + activeDownloadUrlString = urlString; registerDownloaderReceivers(); - downloadHandler.download(); headerFragment.startProgress(); + DownloaderService.queue(this, activeDownloadUrlString); } public void removeApk(String packageName) { @@ -1466,14 +1457,11 @@ public class AppDetails extends AppCompatActivity { @Override public void onClick(View view) { AppDetails appDetails = (AppDetails) getActivity(); - if (appDetails == null || appDetails.downloadHandler == null) { + if (appDetails == null || appDetails.activeDownloadUrlString == null) { return; } - DownloaderService.cancel(getContext(), appDetails.downloadHandler.urlString); - appDetails.cleanUpFinishedDownload(); - setProgressVisible(false); - updateViews(); + DownloaderService.cancel(getContext(), appDetails.activeDownloadUrlString); } public void updateViews() { @@ -1485,7 +1473,7 @@ public class AppDetails extends AppCompatActivity { TextView statusView = (TextView) view.findViewById(R.id.status); btMain.setVisibility(View.VISIBLE); - if (appDetails.downloadHandler != null) { + if (appDetails.activeDownloadUrlString != null) { btMain.setText(R.string.downloading); btMain.setEnabled(false); } else if (!app.isInstalled() && app.suggestedVercode > 0 && diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 57862494b..d086d8e3f 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -234,7 +234,6 @@ public class FDroidApp extends Application { // 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. - Utils.deleteFiles(Utils.getApkDownloadDir(this), null, ".apk"); if (!Preferences.get().shouldCacheApks()) { Utils.deleteFiles(Utils.getApkCacheDir(this), null, ".apk"); } diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index 1f66c3c05..94ae4ed64 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -318,7 +318,11 @@ public final class Utils { } /** - * See {@link Utils#getApkDownloadDir(android.content.Context)} for why this is "unsafe". + * This location is only for caching, do not install directly from this location + * because if the file is on the External Storage, any other app could swap out + * the APK while the install was in process, allowing malware to install things. + * Using {@link org.fdroid.fdroid.installer.Installer#installPackage(File, 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"); @@ -328,24 +332,6 @@ public final class Utils { 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 getApkDownloadDir(Context context) { - final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "temp"); - if (!apkCacheDir.exists()) { - apkCacheDir.mkdir(); - } - - // All parent directories of the .apk file need to be executable for the package installer - // to be able to have permission to read our world-readable .apk files. - FileCompat.setExecutable(apkCacheDir, true, false); - return apkCacheDir; - } - public static String calcFingerprint(String keyHexString) { if (TextUtils.isEmpty(keyHexString) || keyHexString.matches(".*[^a-fA-F0-9].*")) { 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 9517c6d78..092f62e41 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -23,14 +23,25 @@ import android.app.Activity; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; +import android.text.TextUtils; import android.util.Log; +import org.apache.commons.io.FileUtils; +import org.fdroid.fdroid.AndroidXMLDecompress; import org.fdroid.fdroid.BuildConfig; +import org.fdroid.fdroid.Hasher; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.compat.FileCompat; +import org.fdroid.fdroid.data.Apk; +import org.fdroid.fdroid.data.ApkProvider; +import org.fdroid.fdroid.data.SanitizedFile; import org.fdroid.fdroid.privileged.install.InstallExtensionDialogActivity; import java.io.File; +import java.io.IOException; +import java.security.NoSuchAlgorithmException; +import java.util.Map; /** * Abstract Installer class. Also provides static methods to automatically @@ -120,16 +131,16 @@ public abstract class Installer { if (android.os.Build.VERSION.SDK_INT >= 14) { // Default installer on Android >= 4.0 try { - Utils.debugLog(TAG, "try default installer for Android >= 4"); + Utils.debugLog(TAG, "try default installer for android >= 14"); return new DefaultSdk14Installer(activity, pm, callback); } catch (InstallFailedException e) { Log.e(TAG, "Android not compatible with DefaultInstallerSdk14!", e); } } else { - // Default installer on Android < 4.0 + // Default installer on Android < 4.0 (android-14) try { - Utils.debugLog(TAG, "try default installer for Android < 4"); + Utils.debugLog(TAG, "try default installer for android < 14"); return new DefaultInstaller(activity, pm, callback); } catch (InstallFailedException e) { @@ -141,39 +152,87 @@ public abstract class Installer { return null; } - public void installPackage(File apkFile, String packageName) throws InstallFailedException { - // check if file exists... + /** + * Checks the APK file against the provided hash, returning whether it is a match. + */ + private static boolean verifyApkFile(File apkFile, String hash, String hashType) + throws NoSuchAlgorithmException { if (!apkFile.exists()) { - Log.e(TAG, "Couldn't find file " + apkFile + " to install."); - return; + return false; } + Hasher hasher = new Hasher(hashType, apkFile); + if (hasher != null && hasher.match(hash)) { + return true; + } + return false; + } - // special case: F-Droid Privileged Extension - if (packageName != null && packageName.equals(PrivilegedInstaller.PRIVILEGED_EXTENSION_PACKAGE_NAME)) { + /** + * This is the safe, single point of entry for submitting an APK file to be installed. + */ + public void installPackage(File apkFile, String packageName) throws InstallFailedException { + SanitizedFile apkToInstall = null; + try { + Map attributes = AndroidXMLDecompress.getManifestHeaderAttributes(apkFile.getAbsolutePath()); - // extension must be signed with the same public key as main F-Droid - // NOTE: Disabled for debug builds to be able to use official extension from repo - ApkSignatureVerifier signatureVerifier = new ApkSignatureVerifier(mContext); - if (!BuildConfig.DEBUG && !signatureVerifier.hasFDroidSignature(apkFile)) { - throw new SecurityException("APK signature of extension not correct!"); + /* This isn't really needed, but might as well since we have the data already */ + if (attributes.containsKey("packageName")) { + if (!TextUtils.equals(packageName, (String) attributes.get("packageName"))) { + throw new InstallFailedException(apkFile + " has packageName that clashes with " + packageName); + } } - Activity activity; - try { - activity = (Activity) mContext; - } catch (ClassCastException e) { - Utils.debugLog(TAG, "F-Droid Privileged can only be updated using an activity!"); + if (!attributes.containsKey("versionCode")) { + throw new InstallFailedException(apkFile + " is missing versionCode!"); + } + int versionCode = (Integer) attributes.get("versionCode"); + Apk apk = ApkProvider.Helper.find(mContext, packageName, versionCode, new String[]{ + ApkProvider.DataColumns.HASH, + ApkProvider.DataColumns.HASH_TYPE, + }); + /* Always copy the APK to the safe location inside of the protected area + * of the app to prevent attacks based on other apps swapping the file + * out during the install process. Most likely, apkFile was just downloaded, + * so it should still be in the RAM disk cache */ + apkToInstall = SanitizedFile.knownSanitized(File.createTempFile("install-", ".apk", mContext.getFilesDir())); + FileUtils.copyFile(apkFile, apkToInstall); + if (!verifyApkFile(apkToInstall, apk.hash, apk.hashType)) { + FileUtils.deleteQuietly(apkFile); + throw new InstallFailedException(apkFile + " failed to verify!"); + } + apkFile = null; // ensure this is not used now that its copied to apkToInstall + + // special case: F-Droid Privileged Extension + if (packageName != null && packageName.equals(PrivilegedInstaller.PRIVILEGED_EXTENSION_PACKAGE_NAME)) { + + // extension must be signed with the same public key as main F-Droid + // NOTE: Disabled for debug builds to be able to use official extension from repo + ApkSignatureVerifier signatureVerifier = new ApkSignatureVerifier(mContext); + if (!BuildConfig.DEBUG && !signatureVerifier.hasFDroidSignature(apkToInstall)) { + throw new InstallFailedException("APK signature of extension not correct!"); + } + + Activity activity = (Activity) mContext; + Intent installIntent = new Intent(activity, InstallExtensionDialogActivity.class); + installIntent.setAction(InstallExtensionDialogActivity.ACTION_INSTALL); + installIntent.putExtra(InstallExtensionDialogActivity.EXTRA_INSTALL_APK, apkToInstall.getAbsolutePath()); + activity.startActivity(installIntent); return; } - Intent installIntent = new Intent(activity, InstallExtensionDialogActivity.class); - installIntent.setAction(InstallExtensionDialogActivity.ACTION_INSTALL); - installIntent.putExtra(InstallExtensionDialogActivity.EXTRA_INSTALL_APK, apkFile.getAbsolutePath()); - activity.startActivity(installIntent); - 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(apkToInstall, true, false); + installPackageInternal(apkToInstall); - installPackageInternal(apkFile); + } catch (NumberFormatException | NoSuchAlgorithmException | IOException e) { + throw new InstallFailedException(e); + } catch (ClassCastException e) { + throw new InstallFailedException("F-Droid Privileged can only be updated using an activity!"); + } } public void deletePackage(String packageName) throws InstallFailedException { diff --git a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java deleted file mode 100644 index 0673a7fe4..000000000 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ /dev/null @@ -1,160 +0,0 @@ -/* - * Copyright (C) 2010-2012 Ciaran Gultnieks - * Copyright (C) 2011 Henrik Tunedal - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 3 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, - * MA 02110-1301, USA. - */ - -package org.fdroid.fdroid.net; - -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; -import android.net.Uri; -import android.support.annotation.NonNull; -import android.support.v4.content.LocalBroadcastManager; -import android.util.Log; -import android.widget.Toast; - -import org.fdroid.fdroid.Hasher; -import org.fdroid.fdroid.Preferences; -import org.fdroid.fdroid.R; -import org.fdroid.fdroid.Utils; -import org.fdroid.fdroid.data.Apk; -import org.fdroid.fdroid.data.App; -import org.fdroid.fdroid.data.SanitizedFile; - -import java.io.File; -import java.security.NoSuchAlgorithmException; - -/** - * Downloads and verifies (against the Apk.hash) the apk file. - * If the file has previously been downloaded, it will make use of that - * instead, without going to the network to download a new one. - */ -public class ApkDownloader { - - private static final String TAG = "ApkDownloader"; - - public final String urlString; - - @NonNull private final Apk curApk; - @NonNull private final Context context; - @NonNull private SanitizedFile localFile; - @NonNull private final SanitizedFile potentiallyCachedFile; - private final LocalBroadcastManager localBroadcastManager; - - public ApkDownloader(@NonNull final Context context, @NonNull final App app, @NonNull final Apk apk, @NonNull final String repoAddress) { - this.context = context; - curApk = apk; - potentiallyCachedFile = new SanitizedFile(Utils.getApkCacheDir(context), apk.apkName); - urlString = Utils.getApkUrl(repoAddress, apk); - localBroadcastManager = LocalBroadcastManager.getInstance(context); - } - - private Hasher createHasher(File apkFile) { - Hasher hasher; - try { - hasher = new Hasher(curApk.hashType, apkFile); - } catch (NoSuchAlgorithmException e) { - 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(@NonNull final File apkFile) { - if (!apkFile.exists()) { - return false; - } - Hasher hasher = createHasher(apkFile); - return hasher != null && hasher.match(curApk.hash); - } - - /** - * If an existing cached version exists, and matches the hash of the apk we - * 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 verifyOrDelete(@NonNull final File apkFile) { - if (apkFile.exists()) { - if (hashMatches(apkFile)) { - Utils.debugLog(TAG, "Using cached apk at " + apkFile); - return true; - } - Utils.debugLog(TAG, "Not using cached apk at " + apkFile + "(hash doesn't match, will delete file)"); - delete(apkFile); - } - return false; - } - - private void delete(@NonNull final File file) { - if (file.exists()) { - if (!file.delete()) { - Log.w(TAG, "Could not delete file " + file); - } - } - } - - private void sendDownloadComplete() { - Utils.debugLog(TAG, "Download finished: " + localFile); - localBroadcastManager.unregisterReceiver(downloadCompleteReceiver); - } - - public void download() { - // Can we use the cached version? - if (verifyOrDelete(potentiallyCachedFile)) { - delete(localFile); - Utils.copyQuietly(potentiallyCachedFile, localFile); - sendDownloadComplete(); - return; - } - - Utils.debugLog(TAG, "Downloading apk from " + urlString + " to " + localFile); - localBroadcastManager.registerReceiver(downloadCompleteReceiver, - DownloaderService.getIntentFilter(urlString, Downloader.ACTION_COMPLETE)); - - DownloaderService.queue(context, urlString); - } - - private void sendProgressEvent(String status) { - Intent intent = new Intent(status); - intent.setData(Uri.parse(urlString)); - intent.putExtra(Downloader.EXTRA_DOWNLOAD_PATH, localFile.getAbsolutePath()); - localBroadcastManager.sendBroadcast(intent); - } - - // TODO move this code to somewhere more appropriate - BroadcastReceiver downloadCompleteReceiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - localFile = SanitizedFile.knownSanitized(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH)); - if (!verifyOrDelete(localFile)) { - sendProgressEvent(Downloader.ACTION_INTERRUPTED); - Toast.makeText(context, R.string.corrupt_download, Toast.LENGTH_LONG).show(); - return; - } - - if (Preferences.get().shouldCacheApks()) { - Utils.debugLog(TAG, "Copying .apk file to cache at " + potentiallyCachedFile.getAbsolutePath()); - Utils.copyQuietly(localFile, potentiallyCachedFile); - } - - sendDownloadComplete(); - } - }; -} diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 7f308433a..1b0f68ed6 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -34,7 +34,6 @@ import android.text.TextUtils; import android.util.Log; import org.fdroid.fdroid.Utils; -import org.fdroid.fdroid.compat.FileCompat; import org.fdroid.fdroid.data.SanitizedFile; import java.io.File; @@ -194,12 +193,6 @@ public class DownloaderService extends Service { } }); downloader.download(); - // 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); sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile); } catch (InterruptedException e) { sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); 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 2137ccf5e..219f4c0b4 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 @@ -45,7 +45,6 @@ import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.localrepo.LocalRepoManager; import org.fdroid.fdroid.localrepo.SwapService; import org.fdroid.fdroid.localrepo.peers.Peer; -import org.fdroid.fdroid.net.ApkDownloader; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.DownloaderService; @@ -783,8 +782,8 @@ public class SwapWorkflowActivity extends AppCompatActivity { } public void install(@NonNull final App app) { - final Apk apkToInstall = ApkProvider.Helper.find(this, app.packageName, app.suggestedVercode); - final ApkDownloader downloader = new ApkDownloader(this, app, apkToInstall, apkToInstall.repoAddress); + final Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVercode); + String urlString = Utils.getApkUrl(apk.repoAddress, apk); downloadCompleteReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { @@ -793,8 +792,8 @@ public class SwapWorkflowActivity extends AppCompatActivity { } }; localBroadcastManager.registerReceiver(downloadCompleteReceiver, - DownloaderService.getIntentFilter(downloader.urlString, Downloader.ACTION_COMPLETE)); - downloader.download(); + DownloaderService.getIntentFilter(urlString, Downloader.ACTION_COMPLETE)); + DownloaderService.queue(this, urlString); } private void handleDownloadComplete(File apkFile, String packageName) { From fa766180bfff3597ab6a8304163a46c0fd765c33 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Sat, 2 Apr 2016 21:15:21 +0200 Subject: [PATCH 08/15] download APKs into dir named after repo's hostname This is to prevent conflicts if two repos have APKs with the same names, but are not the same app/version. --- .../java/org/fdroid/fdroid/net/DownloaderService.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 1b0f68ed6..aaa6ad423 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -170,6 +170,10 @@ public class DownloaderService extends Service { * the same DownloaderService, but it will not hold up anything else. * When all requests have been handled, the DownloaderService stops itself, * so you should not ever call {@link #stopSelf}. + *

+ * Downloads are put into subdirectories based on hostname/port of each repo + * to prevent files with the same names from conflicting. Each repo enforces + * unique APK file names on the server side. * * @param intent The {@link Intent} passed via {@link * android.content.Context#startService(Intent)}. @@ -177,7 +181,9 @@ public class DownloaderService extends Service { protected void handleIntent(Intent intent) { final Uri uri = intent.getData(); Log.i(TAG, "handleIntent " + uri); - final SanitizedFile localFile = new SanitizedFile(Utils.getApkDownloadDir(this), uri.getLastPathSegment()); + File downloadDir = new File(Utils.getApkCacheDir(this), uri.getHost() + "-" + uri.getPort()); + downloadDir.mkdirs(); + final SanitizedFile localFile = new SanitizedFile(downloadDir, uri.getLastPathSegment()); sendBroadcast(uri, Downloader.ACTION_STARTED, localFile); try { downloader = DownloaderFactory.create(this, uri, localFile); From 3265c8634c3fba9c9ca54889fcd9be1537a934f6 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Sat, 2 Apr 2016 23:44:07 +0200 Subject: [PATCH 09/15] implement resumable APK downloads for HttpDownloader Moving towards having the Downloader always download directly into the cache means its really easy to support resuming downloads. This should also work for updates and icons, since it fetches the Content Length via HTTP first. The icon and update downloading code needs to be adjusted to support that. For APKs, it probably makes sense to include the file size from the index db to save the query over the net. That would be probably more helpful on Bluetooth. --- .../fdroid/net/BluetoothDownloader.java | 2 +- .../org/fdroid/fdroid/net/Downloader.java | 18 +++--- .../org/fdroid/fdroid/net/HttpDownloader.java | 56 +++++++++++++++---- .../fdroid/net/LocalFileDownloader.java | 2 +- 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java index 63c125f44..c782d3ee6 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java @@ -80,7 +80,7 @@ public class BluetoothDownloader extends Downloader { @Override public void download() throws IOException, InterruptedException { - downloadFromStream(1024); + downloadFromStream(1024, false); connection.closeQuietly(); } diff --git a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java index 8be443376..ab5474ffb 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java @@ -31,8 +31,6 @@ public abstract class Downloader { private volatile int totalBytes; private Timer timer; - private final OutputStream outputStream; - public final File outputFile; protected final URL sourceUrl; @@ -60,7 +58,6 @@ public abstract class Downloader { throws FileNotFoundException, MalformedURLException { this.sourceUrl = url; outputFile = destFile; - outputStream = new FileOutputStream(outputFile); } public final InputStream getInputStream() throws IOException { @@ -100,9 +97,10 @@ public abstract class Downloader { public abstract boolean isCached(); - protected void downloadFromStream(int bufferSize) throws IOException, InterruptedException { + protected void downloadFromStream(int bufferSize, boolean resumable) throws IOException, InterruptedException { Utils.debugLog(TAG, "Downloading from stream"); InputStream input = null; + OutputStream outputStream = new FileOutputStream(outputFile, resumable); try { input = getInputStream(); @@ -110,7 +108,7 @@ public abstract class Downloader { // we were interrupted before proceeding to the download. throwExceptionIfInterrupted(); - copyInputToOutputStream(input, bufferSize); + copyInputToOutputStream(input, bufferSize, outputStream); } finally { Utils.closeQuietly(outputStream); Utils.closeQuietly(input); @@ -126,6 +124,7 @@ public abstract class Downloader { * interrupt occured during that blocking operation. The goal is to ensure we * don't move onto another slow, network operation if we have cancelled the * download. + * * @throws InterruptedException */ private void throwExceptionIfInterrupted() throws InterruptedException { @@ -150,7 +149,7 @@ public abstract class Downloader { * keeping track of the number of bytes that have flowed through for the * progress counter. */ - private void copyInputToOutputStream(InputStream input, int bufferSize) throws IOException, InterruptedException { + private void copyInputToOutputStream(InputStream input, int bufferSize, OutputStream output) throws IOException, InterruptedException { bytesRead = 0; totalBytes = totalDownloadSize(); byte[] buffer = new byte[bufferSize]; @@ -178,14 +177,13 @@ public abstract class Downloader { Utils.debugLog(TAG, "Finished downloading from stream"); break; } - bytesRead += count; - outputStream.write(buffer, 0, count); + output.write(buffer, 0, count); } timer.cancel(); timer.purge(); - outputStream.flush(); - outputStream.close(); + output.flush(); + output.close(); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java index 20dc237f2..f6a41c4fd 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java @@ -2,6 +2,7 @@ package org.fdroid.fdroid.net; import com.nostra13.universalimageloader.core.download.BaseImageDownloader; +import org.apache.commons.io.FileUtils; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Utils; import org.spongycastle.util.encoders.Base64; @@ -65,7 +66,7 @@ public class HttpDownloader extends Downloader { */ @Override protected InputStream getDownloadersInputStream() throws IOException { - setupConnection(); + setupConnection(false); return new BufferedInputStream(connection.getInputStream()); } @@ -79,8 +80,25 @@ public class HttpDownloader extends Downloader { */ @Override public void download() throws IOException, InterruptedException { - setupConnection(); - doDownload(); + boolean resumable = false; + long fileLength = outputFile.length(); + + // get the file size from the server + HttpURLConnection tmpConn = getConnection(); + int contentLength = -1; + if (tmpConn.getResponseCode() == 200) { + contentLength = tmpConn.getContentLength(); + } + tmpConn.disconnect(); + if (fileLength > contentLength) { + FileUtils.deleteQuietly(outputFile); + } else if (fileLength == contentLength && outputFile.isFile()) { + return; // already have it! + } else if (fileLength > 0) { + resumable = true; + } + setupConnection(resumable); + doDownload(resumable); } private boolean isSwapUrl() { @@ -90,10 +108,8 @@ public class HttpDownloader extends Downloader { && FDroidApp.subnetInfo.isInRange(host); // on the same subnet as we are } - protected void setupConnection() throws IOException { - if (connection != null) { - return; - } + private HttpURLConnection getConnection() throws IOException { + HttpURLConnection connection; if (isSwapUrl()) { // swap never works with a proxy, its unrouted IP on the same subnet connection = (HttpURLConnection) sourceUrl.openConnection(); @@ -113,9 +129,25 @@ public class HttpDownloader extends Downloader { String authString = username + ":" + password; connection.setRequestProperty("Authorization", "Basic " + Base64.toBase64String(authString.getBytes())); } + return connection; } - protected void doDownload() throws IOException, InterruptedException { + /** + * @return Whether the connection is resumable or not + */ + protected void setupConnection(boolean resumable) throws IOException { + if (connection != null) { + return; + } + connection = getConnection(); + + if (resumable) { + // partial file exists, resume the download + connection.setRequestProperty("Range", "bytes=" + outputFile.length() + "-"); + } + } + + protected void doDownload(boolean resumable) throws IOException, InterruptedException { if (wantToCheckCache()) { setupCacheCheck(); Utils.debugLog(TAG, "Checking cached status of " + sourceUrl); @@ -125,8 +157,8 @@ public class HttpDownloader extends Downloader { if (isCached()) { Utils.debugLog(TAG, sourceUrl + " is cached, so not downloading (HTTP " + statusCode + ")"); } else { - Utils.debugLog(TAG, "Downloading from " + sourceUrl); - downloadFromStream(4096); + Utils.debugLog(TAG, "doDownload for " + sourceUrl + " " + resumable); + downloadFromStream(8192, resumable); updateCacheCheck(); } } @@ -166,6 +198,8 @@ public class HttpDownloader extends Downloader { @Override public void close() { - connection.disconnect(); + if (connection != null) { + connection.disconnect(); + } } } diff --git a/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java index 7e249040e..2207a689f 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java @@ -43,7 +43,7 @@ public class LocalFileDownloader extends Downloader { @Override public void download() throws IOException, InterruptedException { - downloadFromStream(1024 * 50); + downloadFromStream(1024 * 50, false); } @Override From 49635c224d1c2e721e0cda6ad85701a84843ee3b Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 5 Apr 2016 21:15:22 +0200 Subject: [PATCH 10/15] show download errors in a Toast There are many possible cryptic errors that can cause downloads to fail. So just show the localized message from the Exception in an extended Toast. These might not show up until AppDetails works better with the lifecycle of downloads, but they are being sent :) closes #496 https://gitlab.com/fdroid/fdroidclient/issues/496 closes #594 https://gitlab.com/fdroid/fdroidclient/issues/594 closes #599 https://gitlab.com/fdroid/fdroidclient/issues/599 --- .../java/org/fdroid/fdroid/AppDetails.java | 9 ++++++++- .../java/org/fdroid/fdroid/net/Downloader.java | 1 + .../fdroid/fdroid/net/DownloaderService.java | 14 +++++++++----- .../fdroid/fdroid/views/swap/SwapAppsView.java | 18 +++++++++++++++++- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 6434eaf7c..1725011b7 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -515,7 +515,14 @@ public class AppDetails extends AppCompatActivity { private final BroadcastReceiver interruptedReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - Toast.makeText(context, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); + if (intent.hasExtra(Downloader.EXTRA_ERROR_MESSAGE)) { + String msg = intent.getStringExtra(Downloader.EXTRA_ERROR_MESSAGE) + + " " + intent.getDataString(); + Toast.makeText(context, R.string.download_error, Toast.LENGTH_SHORT).show(); + Toast.makeText(context, msg, Toast.LENGTH_LONG).show(); + } else { // user canceled + Toast.makeText(context, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); + } cleanUpFinishedDownload(); } }; diff --git a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java index ab5474ffb..86e7f2685 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java @@ -25,6 +25,7 @@ public abstract class Downloader { public static final String EXTRA_DOWNLOAD_PATH = "org.fdroid.fdroid.net.Downloader.extra.DOWNLOAD_PATH"; public static final String EXTRA_BYTES_READ = "org.fdroid.fdroid.net.Downloader.extra.BYTES_READ"; public static final String EXTRA_TOTAL_BYTES = "org.fdroid.fdroid.net.Downloader.extra.TOTAL_BYTES"; + public static final String EXTRA_ERROR_MESSAGE = "org.fdroid.fdroid.net.Downloader.extra.ERROR_MESSAGE"; private volatile boolean cancelled = false; private volatile int bytesRead; diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index aaa6ad423..532a0ffbb 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -180,7 +180,6 @@ public class DownloaderService extends Service { */ protected void handleIntent(Intent intent) { final Uri uri = intent.getData(); - Log.i(TAG, "handleIntent " + uri); File downloadDir = new File(Utils.getApkCacheDir(this), uri.getHost() + "-" + uri.getPort()); downloadDir.mkdirs(); final SanitizedFile localFile = new SanitizedFile(downloadDir, uri.getLastPathSegment()); @@ -190,7 +189,6 @@ public class DownloaderService extends Service { downloader.setListener(new Downloader.DownloaderProgressListener() { @Override public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) { - //Log.i(TAG, "sendProgress " + sourceUrl + " " + bytesRead + " / " + totalBytes); Intent intent = new Intent(Downloader.ACTION_PROGRESS); intent.setData(uri); intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); @@ -203,22 +201,28 @@ public class DownloaderService extends Service { } catch (InterruptedException e) { sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); } catch (IOException e) { - sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); e.printStackTrace(); + sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile, + e.getLocalizedMessage()); } finally { if (downloader != null) { downloader.close(); } } downloader = null; - Log.i(TAG, "handleIntent DONE " + uri); } private void sendBroadcast(Uri uri, String action, File file) { - Log.i(TAG, "sendBroadcast " + uri + " " + action + " " + file); + sendBroadcast(uri, action, file, null); + } + + private void sendBroadcast(Uri uri, String action, File file, String errorMessage) { Intent intent = new Intent(action); intent.setData(uri); intent.putExtra(Downloader.EXTRA_DOWNLOAD_PATH, file.getAbsolutePath()); + if (!TextUtils.isEmpty(errorMessage)) { + intent.putExtra(Downloader.EXTRA_ERROR_MESSAGE, errorMessage); + } localBroadcastManager.sendBroadcast(intent); } diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java index 190cd747e..ef4080b87 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -36,6 +36,7 @@ import android.widget.ImageView; import android.widget.ListView; import android.widget.ProgressBar; import android.widget.TextView; +import android.widget.Toast; import com.nostra13.universalimageloader.core.DisplayImageOptions; import com.nostra13.universalimageloader.core.ImageLoader; @@ -268,6 +269,21 @@ public class SwapAppsView extends ListView implements } }; + private final BroadcastReceiver interruptedReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + if (intent.hasExtra(Downloader.EXTRA_ERROR_MESSAGE)) { + String msg = intent.getStringExtra(Downloader.EXTRA_ERROR_MESSAGE) + + " " + intent.getDataString(); + Toast.makeText(context, R.string.download_error, Toast.LENGTH_SHORT).show(); + Toast.makeText(context, msg, Toast.LENGTH_LONG).show(); + } else { // user canceled + Toast.makeText(context, R.string.details_notinstalled, Toast.LENGTH_LONG).show(); + } + resetView(); + } + }; + private final ContentObserver appObserver = new ContentObserver(new Handler()) { @Override public void onChange(boolean selfChange) { @@ -293,7 +309,7 @@ public class SwapAppsView extends ListView implements DownloaderService.getIntentFilter(url, Downloader.ACTION_PROGRESS)); localBroadcastManager.registerReceiver(appListViewResetReceiver, DownloaderService.getIntentFilter(url, Downloader.ACTION_COMPLETE)); - localBroadcastManager.registerReceiver(appListViewResetReceiver, + localBroadcastManager.registerReceiver(interruptedReceiver, DownloaderService.getIntentFilter(url, Downloader.ACTION_INTERRUPTED)); } From 721d4a300a6f0df2b135e419f4f4e6110fcf7a92 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 5 Apr 2016 21:58:45 +0200 Subject: [PATCH 11/15] add method to check if a URL is being handled by DownloaderService This also saves the activeDownloadUrlString per packageName. Both are necessary so that AppDetails can accurately display the current state of a background download. Saving this per-packageName allows there to be multiple active downloads in the background, then when people move around AppDetails, it'll restore the progress meter and button state when coming back to the app that they clicked install on. By definition, there is just one DownloaderService enforced by Android with a single active Downloader instance enforced by the DownloaderService. That means using a static variable maps directly to those conditions and provides a really simple, implementation, especially compared to what would have to happen to do it via messages from the thread and any Activities. If this ends up blocking testing or something, it can always be changed when someone implements those tests. --- .../java/org/fdroid/fdroid/AppDetails.java | 17 ++++++++++- .../fdroid/fdroid/net/DownloaderService.java | 29 ++++++++++++++----- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 1725011b7..9312efa85 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -79,6 +79,7 @@ import com.nostra13.universalimageloader.core.assist.ImageScaleType; import org.fdroid.fdroid.Utils.CommaSeparatedList; import org.fdroid.fdroid.compat.PackageManagerCompat; +import org.fdroid.fdroid.compat.PreferencesCompat; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; @@ -428,7 +429,9 @@ public class AppDetails extends AppCompatActivity { refreshApkList(); refreshHeader(); supportInvalidateOptionsMenu(); - registerDownloaderReceivers(); + if (DownloaderService.isQueuedOrActive(activeDownloadUrlString)) { + registerDownloaderReceivers(); + } } /** @@ -448,6 +451,10 @@ public class AppDetails extends AppCompatActivity { @Override protected void onPause() { super.onPause(); + // save the active URL for this app in case we come back + PreferencesCompat.apply(getPreferences(MODE_PRIVATE) + .edit() + .putString(getPackageNameFromIntent(getIntent()), activeDownloadUrlString)); if (app != null && (app.ignoreAllUpdates != startingIgnoreAll || app.ignoreThisUpdate != startingIgnoreThis)) { Utils.debugLog(TAG, "Updating 'ignore updates', as it has changed since we started the activity..."); @@ -569,6 +576,14 @@ public class AppDetails extends AppCompatActivity { Utils.debugLog(TAG, "Getting application details for " + packageName); App newApp = null; + String urlString = getPreferences(MODE_PRIVATE).getString(packageName, null); + if (DownloaderService.isQueuedOrActive(urlString)) { + activeDownloadUrlString = urlString; + } else { + // this URL is no longer active, remove it + PreferencesCompat.apply(getPreferences(MODE_PRIVATE).edit().remove(packageName)); + } + if (!TextUtils.isEmpty(packageName)) { newApp = AppProvider.Helper.findByPackageName(getContentResolver(), packageName); } diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 532a0ffbb..ab4573992 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -73,11 +73,11 @@ public class DownloaderService extends Service { private static final String ACTION_CANCEL = "org.fdroid.fdroid.net.DownloaderService.action.CANCEL"; private volatile Looper serviceLooper; - private volatile ServiceHandler serviceHandler; - private volatile Downloader downloader; + private static volatile ServiceHandler serviceHandler; + private static volatile Downloader downloader; private LocalBroadcastManager localBroadcastManager; - private final HashMap queueWhats = new HashMap(); + private static final HashMap QUEUE_WHATS = new HashMap(); private int what; private final class ServiceHandler extends Handler { @@ -117,7 +117,7 @@ public class DownloaderService extends Service { } if (ACTION_CANCEL.equals(intent.getAction())) { Log.i(TAG, "Removed " + intent); - Integer what = queueWhats.remove(uriString); + Integer what = QUEUE_WHATS.remove(uriString); if (what != null && serviceHandler.hasMessages(what)) { // the URL is in the queue, remove it serviceHandler.removeMessages(what); @@ -134,8 +134,8 @@ public class DownloaderService extends Service { msg.obj = intent; msg.what = what++; serviceHandler.sendMessage(msg); - Log.i(TAG, "queueWhats.put(" + uriString + ", " + msg.what); - queueWhats.put(uriString, msg.what); + Log.i(TAG, "QUEUE_WHATS.put(" + uriString + ", " + msg.what); + QUEUE_WHATS.put(uriString, msg.what); } else { Log.e(TAG, "Received Intent with unknown action: " + intent); } @@ -170,7 +170,7 @@ public class DownloaderService extends Service { * the same DownloaderService, but it will not hold up anything else. * When all requests have been handled, the DownloaderService stops itself, * so you should not ever call {@link #stopSelf}. - *

+ *

* Downloads are put into subdirectories based on hostname/port of each repo * to prevent files with the same names from conflicting. Each repo enforces * unique APK file names on the server side. @@ -260,6 +260,21 @@ public class DownloaderService extends Service { context.startService(intent); } + /** + * Check if a URL is waiting in the queue for downloading or if actively + * being downloaded. This is useful for checking whether to re-register + * {@link android.content.BroadcastReceiver}s in + * {@link android.app.Activity#onResume()} + */ + public static boolean isQueuedOrActive(String urlString) { + if (TextUtils.isEmpty(urlString)) { + return false; + } + Integer what = QUEUE_WHATS.get(urlString); + return (what != null && serviceHandler.hasMessages(what)) + || (downloader != null && TextUtils.equals(urlString, downloader.sourceUrl.toString())); + } + /** * Get a prepared {@link IntentFilter} for use for matching this service's action events. * From 77e041d6401d6289caa3d15b619c4a90c8f81adb Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 6 Apr 2016 12:07:57 +0200 Subject: [PATCH 12/15] put stray preference handling into Preferences This moves a few stray preference handling instances into Preferences, and move the non-preference "lastUpdateCheck" state to local only to UpdateService. This will still work with existing installs since the String constant value is the same. --- .../main/java/org/fdroid/fdroid/Preferences.java | 9 ++++++++- .../java/org/fdroid/fdroid/UpdateService.java | 15 +++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/Preferences.java b/app/src/main/java/org/fdroid/fdroid/Preferences.java index 99670579f..36451ce78 100644 --- a/app/src/main/java/org/fdroid/fdroid/Preferences.java +++ b/app/src/main/java/org/fdroid/fdroid/Preferences.java @@ -54,7 +54,6 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh public static final String PREF_CACHE_APK = "cacheDownloaded"; public static final String PREF_UNSTABLE_UPDATES = "unstableUpdates"; public static final String PREF_EXPERT = "expert"; - public static final String PREF_UPD_LAST = "lastUpdateCheck"; public static final String PREF_PRIVILEGED_INSTALLER = "privilegedInstaller"; public static final String PREF_UNINSTALL_PRIVILEGED_APP = "uninstallPrivilegedApp"; public static final String PREF_LOCAL_REPO_NAME = "localRepoName"; @@ -179,6 +178,14 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh return preferences.getString(PREF_LOCAL_REPO_NAME, getDefaultLocalRepoName()); } + public boolean isUpdateNotificationEnabled() { + return preferences.getBoolean(PREF_UPD_NOTIFY, true); + } + + public boolean isUpdateOnlyOnWifi() { + return preferences.getBoolean(PREF_UPD_WIFI_ONLY, false); + } + /** * This preference's default is set dynamically based on whether Orbot is * installed. If Orbot is installed, default to using Tor, the user can still override diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index cfdd18012..c423e4ca1 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -73,6 +73,8 @@ public class UpdateService extends IntentService implements ProgressListener { public static final int STATUS_ERROR_LOCAL_SMALL = 4; public static final int STATUS_INFO = 5; + private static final String STATE_LAST_UPDATED = "lastUpdateCheck"; + private LocalBroadcastManager localBroadcastManager; private static final int NOTIFY_ID_UPDATING = 0; @@ -295,7 +297,7 @@ public class UpdateService extends IntentService implements ProgressListener { Log.i(TAG, "Skipping update - disabled"); return false; } - long lastUpdate = prefs.getLong(Preferences.PREF_UPD_LAST, 0); + long lastUpdate = prefs.getLong(STATE_LAST_UPDATED, 0); long elapsed = System.currentTimeMillis() - lastUpdate; if (elapsed < interval * 60 * 60 * 1000) { Log.i(TAG, "Skipping update - done " + elapsed @@ -318,9 +320,7 @@ public class UpdateService extends IntentService implements ProgressListener { return false; } - SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); - if (activeNetwork.getType() != ConnectivityManager.TYPE_WIFI - && prefs.getBoolean(Preferences.PREF_UPD_WIFI_ONLY, false)) { + if (activeNetwork.getType() != ConnectivityManager.TYPE_WIFI && Preferences.get().isUpdateOnlyOnWifi()) { Log.i(TAG, "Skipping update - wifi not available"); return false; } @@ -343,8 +343,6 @@ public class UpdateService extends IntentService implements ProgressListener { return; } - SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getBaseContext()); - // Grab some preliminary information, then we can release the // database while we do all the downloading, etc... List repos = RepoProvider.Helper.all(this); @@ -395,13 +393,14 @@ public class UpdateService extends IntentService implements ProgressListener { } else { notifyContentProviders(); - if (prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, true)) { + if (Preferences.get().isUpdateNotificationEnabled()) { performUpdateNotification(); } } + SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getBaseContext()); SharedPreferences.Editor e = prefs.edit(); - e.putLong(Preferences.PREF_UPD_LAST, System.currentTimeMillis()); + e.putLong(STATE_LAST_UPDATED, System.currentTimeMillis()); PreferencesCompat.apply(e); if (errorRepos == 0) { From adcdc417abf1b97abfda8078a26f1dc0c255aa6b Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 6 Apr 2016 09:23:06 +0200 Subject: [PATCH 13/15] add basic in progress Notification to DownloaderService This is super basic, really just a placeholder to have something there. It should be replaced with something much better as part of the UX overhaul. #601 https://gitlab.com/fdroid/fdroidclient/issues/601 --- .../fdroid/fdroid/net/DownloaderService.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index ab4573992..f83874d49 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -29,10 +29,13 @@ import android.os.Looper; import android.os.Message; import android.os.PatternMatcher; import android.os.Process; +import android.support.v4.app.NotificationCompat; import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; import android.util.Log; +import org.fdroid.fdroid.Preferences; +import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.SanitizedFile; @@ -72,6 +75,8 @@ public class DownloaderService extends Service { private static final String ACTION_QUEUE = "org.fdroid.fdroid.net.DownloaderService.action.QUEUE"; private static final String ACTION_CANCEL = "org.fdroid.fdroid.net.DownloaderService.action.CANCEL"; + private static final int NOTIFY_DOWNLOADING = 0x2344; + private volatile Looper serviceLooper; private static volatile ServiceHandler serviceHandler; private static volatile Downloader downloader; @@ -87,7 +92,6 @@ public class DownloaderService extends Service { @Override public void handleMessage(Message msg) { - Log.i(TAG, "handleMessage " + msg); handleIntent((Intent) msg.obj); stopSelf(msg.arg1); } @@ -128,6 +132,9 @@ public class DownloaderService extends Service { Log.e(TAG, "CANCEL called on something not queued or running: " + startId + " " + intent); } } else if (ACTION_QUEUE.equals(intent.getAction())) { + if (Preferences.get().isUpdateNotificationEnabled()) { + createNotification(intent.getDataString()); + } Log.i(TAG, "Queued " + intent); Message msg = serviceHandler.obtainMessage(); msg.arg1 = startId; @@ -141,6 +148,16 @@ public class DownloaderService extends Service { } } + private void createNotification(String urlString) { + NotificationCompat.Builder builder = + new NotificationCompat.Builder(this) + .setAutoCancel(true) + .setContentTitle(getString(R.string.downloading)) + .setSmallIcon(android.R.drawable.stat_sys_download) + .setContentText(urlString); + startForeground(NOTIFY_DOWNLOADING, builder.build()); + } + @Override public int onStartCommand(Intent intent, int flags, int startId) { onStart(intent, startId); @@ -151,6 +168,7 @@ public class DownloaderService extends Service { @Override public void onDestroy() { Log.i(TAG, "onDestroy"); + stopForeground(true); serviceLooper.quit(); //NOPMD - this is copied from IntentService, no super call needed } From d114f428e7c800f61b142db2082e0a50af7704ef Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 6 Apr 2016 13:45:29 +0200 Subject: [PATCH 14/15] add "Auto Download" pref to enable auto-downloading of updates Now that we have a nice background service, let's put it to work! This makes update APKs be automatically downloaded after the index is updated. Then when the user goes to install the updates, they will not have to wait for the download part. #601 https://gitlab.com/fdroid/fdroidclient/issues/601 --- .../java/org/fdroid/fdroid/Preferences.java | 5 ++++ .../java/org/fdroid/fdroid/UpdateService.java | 25 +++++++++++++++++++ app/src/main/res/values/strings.xml | 2 ++ app/src/main/res/xml/preferences.xml | 4 +++ 4 files changed, 36 insertions(+) diff --git a/app/src/main/java/org/fdroid/fdroid/Preferences.java b/app/src/main/java/org/fdroid/fdroid/Preferences.java index 36451ce78..df67a16f6 100644 --- a/app/src/main/java/org/fdroid/fdroid/Preferences.java +++ b/app/src/main/java/org/fdroid/fdroid/Preferences.java @@ -45,6 +45,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh public static final String PREF_UPD_INTERVAL = "updateInterval"; public static final String PREF_UPD_WIFI_ONLY = "updateOnWifiOnly"; + public static final String PREF_UPD_AUTO_DOWNLOAD = "updateAutoDownload"; public static final String PREF_UPD_NOTIFY = "updateNotify"; public static final String PREF_UPD_HISTORY = "updateHistoryDays"; public static final String PREF_ROOTED = "rooted"; @@ -182,6 +183,10 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh return preferences.getBoolean(PREF_UPD_NOTIFY, true); } + public boolean isAutoDownloadEnabled() { + return preferences.getBoolean(PREF_UPD_AUTO_DOWNLOAD, false); + } + public boolean isUpdateOnlyOnWifi() { return preferences.getBoolean(PREF_UPD_WIFI_ONLY, false); } diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index c423e4ca1..613436fa1 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -42,6 +42,7 @@ import android.util.Log; import android.widget.Toast; import org.fdroid.fdroid.compat.PreferencesCompat; +import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; @@ -386,6 +387,11 @@ public class UpdateService extends IntentService implements ProgressListener { Log.e(TAG, "Error updating repository " + repo.address, e); } localBroadcastManager.unregisterReceiver(downloadProgressReceiver); + + // now that downloading the index is done, start downloading updates + if (changes && Preferences.get().isAutoDownloadEnabled()) { + autoDownloadUpdates(repo.address); + } } if (!changes) { @@ -476,6 +482,25 @@ public class UpdateService extends IntentService implements ProgressListener { return inboxStyle; } + private void autoDownloadUpdates(String repoAddress) { + Cursor cursor = getContentResolver().query( + AppProvider.getCanUpdateUri(), + new String[]{ + AppProvider.DataColumns.PACKAGE_NAME, + AppProvider.DataColumns.SUGGESTED_VERSION_CODE, + }, null, null, null); + cursor.moveToFirst(); + for (int i = 0; i < cursor.getCount(); i++) { + App app = new App(cursor); + Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVercode, new String[]{ + ApkProvider.DataColumns.NAME, + }); + String urlString = Utils.getApkUrl(repoAddress, apk); + DownloaderService.queue(this, urlString); + cursor.moveToNext(); + } + } + private void showAppUpdatesNotification(Cursor hasUpdates) { Utils.debugLog(TAG, "Notifying " + hasUpdates.getCount() + " updates."); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ab11e85e8..d99f819e6 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -21,6 +21,8 @@ No automatic app list updates Only on Wi-Fi Update app lists automatically only on Wi-Fi + Automatically download updates + Download the update files in the background Update notifications Show a notification when updates are available Update history diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index 43cb8b184..4b2c7d14f 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -9,6 +9,10 @@ + From 74d1c9521d75e6157fc9f60e5deffda777755975 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 11 Apr 2016 14:17:50 -0400 Subject: [PATCH 15/15] put up a notification for each completed download This makes background installs and updates a lot easier --- .../fdroid/net/DownloaderServiceTest.java | 2 +- app/src/main/AndroidManifest.xml | 3 + .../java/org/fdroid/fdroid/AppDetails.java | 4 +- .../java/org/fdroid/fdroid/UpdateService.java | 2 +- .../main/java/org/fdroid/fdroid/Utils.java | 12 ++- .../fdroid/fdroid/installer/Installer.java | 7 +- .../fdroid/net/DownloadCompleteService.java | 88 +++++++++++++++++++ .../fdroid/fdroid/net/DownloaderService.java | 14 ++- .../views/swap/SwapWorkflowActivity.java | 8 +- app/src/main/res/values/strings.xml | 2 + 10 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java diff --git a/app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java b/app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java index 7803ad070..d859d7c0d 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/net/DownloaderServiceTest.java @@ -34,7 +34,7 @@ public class DownloaderServiceTest extends ServiceTestCase { } }, new IntentFilter(Downloader.ACTION_PROGRESS)); for (String url : urls) { - DownloaderService.queue(getContext(), url); + DownloaderService.queue(getContext(), null, url); } Thread.sleep(30000); } diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 3610320ea..0f2e2e03e 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -442,6 +442,9 @@ + diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 9312efa85..a9e57447b 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -511,7 +511,7 @@ public class AppDetails extends AppCompatActivity { public void onReceive(Context context, Intent intent) { File localFile = new File(intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH)); try { - installer.installPackage(localFile, app.packageName); + installer.installPackage(localFile, app.packageName, intent.getDataString()); } catch (InstallFailedException e) { Log.e(TAG, "Android not compatible with this Installer!", e); } @@ -869,7 +869,7 @@ public class AppDetails extends AppCompatActivity { activeDownloadUrlString = urlString; registerDownloaderReceivers(); headerFragment.startProgress(); - DownloaderService.queue(this, activeDownloadUrlString); + DownloaderService.queue(this, apk.packageName, activeDownloadUrlString); } public void removeApk(String packageName) { diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index 613436fa1..f27dcb3aa 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -496,7 +496,7 @@ public class UpdateService extends IntentService implements ProgressListener { ApkProvider.DataColumns.NAME, }); String urlString = Utils.getApkUrl(repoAddress, apk); - DownloaderService.queue(this, urlString); + DownloaderService.queue(this, app.packageName, urlString); cursor.moveToNext(); } } diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index 94ae4ed64..3b58681dc 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -66,6 +66,7 @@ import java.util.Formatter; import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.zip.Adler32; public final class Utils { @@ -321,7 +322,7 @@ public final class Utils { * This location is only for caching, do not install directly from this location * because if the file is on the External Storage, any other app could swap out * the APK while the install was in process, allowing malware to install things. - * Using {@link org.fdroid.fdroid.installer.Installer#installPackage(File, String)} + * 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) { @@ -412,6 +413,15 @@ public final class Utils { return repoAddress + "/" + apk.apkName.replace(" ", "%20"); } + /** + * This generates a unique, reproducible ID for notifications related to {@code urlString} + */ + public static int getApkUrlNotificationId(String urlString) { + Adler32 checksum = new Adler32(); + checksum.update(urlString.getBytes()); + return (int) checksum.getValue(); + } + public static final class CommaSeparatedList implements Iterable { private final String value; 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 092f62e41..bd85b71f0 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -20,6 +20,7 @@ package org.fdroid.fdroid.installer; import android.app.Activity; +import android.app.NotificationManager; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; @@ -170,7 +171,8 @@ public abstract class Installer { /** * This is the safe, single point of entry for submitting an APK file to be installed. */ - public void installPackage(File apkFile, String packageName) throws InstallFailedException { + public void installPackage(File apkFile, String packageName, String urlString) + throws InstallFailedException { SanitizedFile apkToInstall = null; try { Map attributes = AndroidXMLDecompress.getManifestHeaderAttributes(apkFile.getAbsolutePath()); @@ -228,6 +230,9 @@ public abstract class Installer { FileCompat.setReadable(apkToInstall, true, false); installPackageInternal(apkToInstall); + NotificationManager nm = (NotificationManager) + mContext.getSystemService(Context.NOTIFICATION_SERVICE); + nm.cancel(Utils.getApkUrlNotificationId(urlString)); } catch (NumberFormatException | NoSuchAlgorithmException | IOException e) { throw new InstallFailedException(e); } catch (ClassCastException e) { diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java new file mode 100644 index 000000000..6574a76fd --- /dev/null +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloadCompleteService.java @@ -0,0 +1,88 @@ +package org.fdroid.fdroid.net; + +import android.app.IntentService; +import android.app.NotificationManager; +import android.app.PendingIntent; +import android.content.Context; +import android.content.Intent; +import android.content.pm.PackageManager; +import android.net.Uri; +import android.os.Process; +import android.support.v4.app.NotificationCompat; +import android.support.v4.app.TaskStackBuilder; +import android.text.TextUtils; + +import org.fdroid.fdroid.AppDetails; +import org.fdroid.fdroid.R; +import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.data.App; +import org.fdroid.fdroid.data.AppProvider; + +public class DownloadCompleteService extends IntentService { + private static final String TAG = "DownloadCompleteService"; + + private static final String ACTION_NOTIFY = "org.fdroid.fdroid.net.action.NOTIFY"; + private static final String EXTRA_PACKAGE_NAME = "org.fdroid.fdroid.net.extra.PACKAGE_NAME"; + + public DownloadCompleteService() { + super("DownloadCompleteService"); + } + + public static void notify(Context context, String packageName, String urlString) { + Intent intent = new Intent(context, DownloadCompleteService.class); + intent.setAction(ACTION_NOTIFY); + intent.setData(Uri.parse(urlString)); + intent.putExtra(EXTRA_PACKAGE_NAME, packageName); + context.startService(intent); + } + + @Override + protected void onHandleIntent(Intent intent) { + Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST); + if (intent != null) { + final String action = intent.getAction(); + if (!ACTION_NOTIFY.equals(action)) { + Utils.debugLog(TAG, "intent action is not ACTION_NOTIFY"); + return; + } + String packageName = intent.getStringExtra(EXTRA_PACKAGE_NAME); + if (TextUtils.isEmpty(packageName)) { + Utils.debugLog(TAG, "intent is missing EXTRA_PACKAGE_NAME"); + return; + } + + String title; + try { + PackageManager pm = getPackageManager(); + title = String.format(getString(R.string.tap_to_update_format), + pm.getApplicationLabel(pm.getApplicationInfo(packageName, 0))); + } catch (PackageManager.NameNotFoundException e) { + App app = AppProvider.Helper.findByPackageName(getContentResolver(), packageName, + new String[]{ + AppProvider.DataColumns.NAME, + }); + title = String.format(getString(R.string.tap_to_install_format), app.name); + } + + Intent notifyIntent = new Intent(this, AppDetails.class); + notifyIntent.putExtra(AppDetails.EXTRA_APPID, packageName); + TaskStackBuilder stackBuilder = TaskStackBuilder + .create(this) + .addParentStack(AppDetails.class) + .addNextIntent(notifyIntent); + int requestCode = Utils.getApkUrlNotificationId(intent.getDataString()); + PendingIntent pendingIntent = stackBuilder.getPendingIntent(requestCode, + PendingIntent.FLAG_UPDATE_CURRENT); + + NotificationCompat.Builder builder = + new NotificationCompat.Builder(this) + .setAutoCancel(true) + .setContentTitle(title) + .setSmallIcon(android.R.drawable.stat_sys_download_done) + .setContentIntent(pendingIntent) + .setContentText(getString(R.string.tap_to_install)); + NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); + nm.notify(Utils.getApkUrlNotificationId(intent.getDataString()), builder.build()); + } + } +} diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index f83874d49..25d6aaed8 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -72,6 +72,8 @@ import java.util.HashMap; public class DownloaderService extends Service { public static final String TAG = "DownloaderService"; + static final String EXTRA_PACKAGE_NAME = "org.fdroid.fdroid.net.DownloaderService.extra.PACKAGE_NAME"; + private static final String ACTION_QUEUE = "org.fdroid.fdroid.net.DownloaderService.action.QUEUE"; private static final String ACTION_CANCEL = "org.fdroid.fdroid.net.DownloaderService.action.CANCEL"; @@ -216,6 +218,8 @@ public class DownloaderService extends Service { }); downloader.download(); sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile); + DownloadCompleteService.notify(this, intent.getStringExtra(EXTRA_PACKAGE_NAME), + intent.getDataString()); } catch (InterruptedException e) { sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile); } catch (IOException e) { @@ -250,14 +254,18 @@ public class DownloaderService extends Service { * All notifications are sent as an {@link Intent} via local broadcasts to be received by * * @param context - * @param urlString The URL to add to the download queue + * @param packageName The packageName of the app being downloaded + * @param urlString The URL to add to the download queue * @see #cancel(Context, String) */ - public static void queue(Context context, String urlString) { + public static void queue(Context context, String packageName, String urlString) { Log.i(TAG, "queue " + urlString); Intent intent = new Intent(context, DownloaderService.class); intent.setAction(ACTION_QUEUE); intent.setData(Uri.parse(urlString)); + if (!TextUtils.isEmpty(EXTRA_PACKAGE_NAME)) { + intent.putExtra(EXTRA_PACKAGE_NAME, packageName); + } context.startService(intent); } @@ -268,7 +276,7 @@ public class DownloaderService extends Service { * * @param context * @param urlString The URL to remove from the download queue - * @see #queue(Context, String) + * @see #queue(Context, String, String) */ public static void cancel(Context context, String urlString) { Log.i(TAG, "cancel " + urlString); 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 219f4c0b4..2586224be 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 @@ -788,15 +788,15 @@ public class SwapWorkflowActivity extends AppCompatActivity { @Override public void onReceive(Context context, Intent intent) { String path = intent.getStringExtra(Downloader.EXTRA_DOWNLOAD_PATH); - handleDownloadComplete(new File(path), app.packageName); + handleDownloadComplete(new File(path), app.packageName, intent.getDataString()); } }; localBroadcastManager.registerReceiver(downloadCompleteReceiver, DownloaderService.getIntentFilter(urlString, Downloader.ACTION_COMPLETE)); - DownloaderService.queue(this, urlString); + DownloaderService.queue(this, app.packageName, urlString); } - private void handleDownloadComplete(File apkFile, String packageName) { + private void handleDownloadComplete(File apkFile, String packageName, String urlString) { try { Installer.getActivityInstaller(this, new Installer.InstallerCallback() { @@ -811,7 +811,7 @@ public class SwapWorkflowActivity extends AppCompatActivity { public void onError(int operation, int errorCode) { // TODO: Boo! } - }).installPackage(apkFile, packageName); + }).installPackage(apkFile, packageName, urlString); localBroadcastManager.unregisterReceiver(downloadCompleteReceiver); } catch (Installer.InstallFailedException e) { // TODO: Handle exception properly diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d99f819e6..074fd531d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -341,6 +341,8 @@ Swapping not enabled Before swapping, your device must be made visible. + Tap to install %s + Tap to update %s Do you want to install this application? It will get access to: Do you want to install this application?