From 9d1743af33275f41bcc537e8779c8d68d8f1ba85 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 23 Mar 2016 15:23:32 +0100 Subject: [PATCH 1/7] purge disabled Android DownloadManager code Unfortunately, this approach did not really work out. It would have been really nice to rely on the provided DownloadManager stuff, but it has too many issues, like not working with Tor or other proxies, and being difficult to tightly integrate. --- app/src/main/AndroidManifest.xml | 8 - .../java/org/fdroid/fdroid/AppDetails.java | 13 - .../net/AsyncDownloaderFromAndroid.java | 396 ------------------ .../fdroid/fdroid/net/DownloaderFactory.java | 58 +-- .../receiver/DownloadManagerReceiver.java | 85 ---- 5 files changed, 1 insertion(+), 559 deletions(-) delete mode 100644 app/src/main/java/org/fdroid/fdroid/net/AsyncDownloaderFromAndroid.java delete mode 100644 app/src/main/java/org/fdroid/fdroid/receiver/DownloadManagerReceiver.java diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index df55f5589..61ed38012 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -437,14 +437,6 @@ - - - - - - - - diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 9617cdf2c..6785e52ea 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -92,7 +92,6 @@ import org.fdroid.fdroid.installer.Installer; 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.AsyncDownloaderFromAndroid; import org.fdroid.fdroid.net.Downloader; import java.io.File; @@ -434,18 +433,6 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A } localBroadcastManager = LocalBroadcastManager.getInstance(this); - - // Check if a download is running for this app - if (AsyncDownloaderFromAndroid.isDownloading(this, app.packageName) >= 0) { - // call install() to re-setup the listeners and downloaders - // the AsyncDownloader will not restart the download since the download is running, - // and thus the version we pass to install() is not important - refreshHeader(); - refreshApkList(); - final Apk apkToInstall = ApkProvider.Helper.find(this, app.packageName, app.suggestedVercode); - install(apkToInstall); - } - } @Override diff --git a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloaderFromAndroid.java b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloaderFromAndroid.java deleted file mode 100644 index 409e56e6f..000000000 --- a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloaderFromAndroid.java +++ /dev/null @@ -1,396 +0,0 @@ -package org.fdroid.fdroid.net; - -import android.annotation.TargetApi; -import android.app.DownloadManager; -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; -import android.content.IntentFilter; -import android.database.Cursor; -import android.net.Uri; -import android.os.Build; -import android.os.ParcelFileDescriptor; -import android.support.v4.content.LocalBroadcastManager; -import android.text.TextUtils; - -import org.fdroid.fdroid.R; -import org.fdroid.fdroid.Utils; - -import java.io.File; -import java.io.FileDescriptor; -import java.io.FileInputStream; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; - -/** - * A downloader that uses Android's DownloadManager to perform a download. - */ -@TargetApi(Build.VERSION_CODES.GINGERBREAD) -public class AsyncDownloaderFromAndroid implements AsyncDownloader { - private final Context context; - private final DownloadManager dm; - private final LocalBroadcastManager localBroadcastManager; - private final File localFile; - private final String remoteAddress; - private final String downloadTitle; - private final String uniqueDownloadId; - private final Listener listener; - private boolean isCancelled; - - private long downloadManagerId = -1; - - /** - * 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. - */ - public AsyncDownloaderFromAndroid(Context context, Listener listener, String downloadTitle, String downloadId, String remoteAddress, File localFile) { - this.context = context; - this.uniqueDownloadId = downloadId; - this.remoteAddress = remoteAddress; - this.listener = listener; - this.localFile = localFile; - - if (TextUtils.isEmpty(downloadTitle)) { - this.downloadTitle = remoteAddress; - } else { - this.downloadTitle = downloadTitle; - } - - dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - localBroadcastManager = LocalBroadcastManager.getInstance(context); - } - - @Override - public void download() { - isCancelled = false; - - // check if download failed - if (downloadManagerId >= 0) { - int status = validDownload(context, downloadManagerId); - if (status > 0) { - // error downloading - dm.remove(downloadManagerId); - if (listener != null) { - listener.onErrorDownloading(context.getString(R.string.download_error)); - } - return; - } - } - - // Check if the download is complete - downloadManagerId = isDownloadComplete(context, uniqueDownloadId); - if (downloadManagerId > 0) { - // clear the download - dm.remove(downloadManagerId); - - try { - // write the downloaded file to the expected location - ParcelFileDescriptor fd = dm.openDownloadedFile(downloadManagerId); - copyFile(fd.getFileDescriptor(), localFile); - listener.onDownloadComplete(); - } catch (IOException e) { - listener.onErrorDownloading(e.getLocalizedMessage()); - } - return; - } - - // Check if the download is still in progress - if (downloadManagerId < 0) { - downloadManagerId = isDownloading(context, uniqueDownloadId); - } - - // Start a new download - if (downloadManagerId < 0) { - // set up download request - DownloadManager.Request request = new DownloadManager.Request(Uri.parse(remoteAddress)); - request.setTitle(downloadTitle); - request.setDescription(uniqueDownloadId); // we will retrieve this later from the description field - this.downloadManagerId = dm.enqueue(request); - } - - context.registerReceiver(receiver, - new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE)); - - Thread progressThread = new Thread() { - @Override - public void run() { - while (!isCancelled && isDownloading(context, uniqueDownloadId) >= 0) { - try { - Thread.sleep(1000); - } catch (Exception e) { - // ignore - } - sendProgress(getBytesRead(), getTotalBytes()); - } - } - }; - progressThread.start(); - } - - /** - * Copy input file to output file - * @throws IOException - */ - private void copyFile(FileDescriptor inputFile, File outputFile) throws IOException { - InputStream input = null; - OutputStream output = null; - try { - input = new FileInputStream(inputFile); - output = new FileOutputStream(outputFile); - Utils.copy(input, output); - } finally { - Utils.closeQuietly(output); - Utils.closeQuietly(input); - } - } - - @Override - public int getBytesRead() { - if (downloadManagerId < 0) return 0; - - DownloadManager.Query query = new DownloadManager.Query(); - query.setFilterById(downloadManagerId); - Cursor c = dm.query(query); - - try { - if (c.moveToFirst()) { - // we use the description column to store the unique id of this download - int columnIndex = c.getColumnIndex(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR); - return c.getInt(columnIndex); - } - } finally { - c.close(); - } - - return 0; - } - - @Override - public int getTotalBytes() { - if (downloadManagerId < 0) return 0; - - DownloadManager.Query query = new DownloadManager.Query(); - query.setFilterById(downloadManagerId); - Cursor c = dm.query(query); - - try { - if (c.moveToFirst()) { - // we use the description column to store the unique id for this download - int columnIndex = c.getColumnIndex(DownloadManager.COLUMN_TOTAL_SIZE_BYTES); - return c.getInt(columnIndex); - } - } finally { - c.close(); - } - - return 0; - } - - private void sendProgress(int bytesRead, int totalBytes) { - Intent intent = new Intent(Downloader.LOCAL_ACTION_PROGRESS); - intent.putExtra(Downloader.EXTRA_ADDRESS, remoteAddress); - intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); - intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); - localBroadcastManager.sendBroadcast(intent); - } - - @Override - public void attemptCancel(boolean userRequested) { - isCancelled = true; - try { - context.unregisterReceiver(receiver); - } catch (Exception e) { - // ignore if receiver already unregistered - } - - if (userRequested && downloadManagerId >= 0) { - dm.remove(downloadManagerId); - } - } - - /** - * Extract the uniqueDownloadId from a given download id. - * @return - uniqueDownloadId or null if not found - */ - public static String getDownloadId(Context context, long downloadId) { - DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - DownloadManager.Query query = new DownloadManager.Query(); - query.setFilterById(downloadId); - Cursor c = dm.query(query); - - try { - if (c.moveToFirst()) { - // we use the description column to store the unique id for this download - int columnIndex = c.getColumnIndex(DownloadManager.COLUMN_DESCRIPTION); - return c.getString(columnIndex); - } - } finally { - c.close(); - } - - return null; - } - - /** - * Extract the download title from a given download id. - * @return - title or null if not found - */ - public static String getDownloadTitle(Context context, long downloadId) { - DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - DownloadManager.Query query = new DownloadManager.Query(); - query.setFilterById(downloadId); - Cursor c = dm.query(query); - - try { - if (c.moveToFirst()) { - int columnIndex = c.getColumnIndex(DownloadManager.COLUMN_TITLE); - return c.getString(columnIndex); - } - } finally { - c.close(); - } - - return null; - } - - /** - * Get the downloadManagerId from an Intent sent by the DownloadManagerReceiver - */ - @TargetApi(Build.VERSION_CODES.HONEYCOMB) - public static long getDownloadId(Intent intent) { - if (intent != null) { - if (intent.hasExtra(DownloadManager.EXTRA_DOWNLOAD_ID)) { - // we have been passed a DownloadManager download id, so get the unique id for that download - return intent.getLongExtra(DownloadManager.EXTRA_DOWNLOAD_ID, -1); - } - - if (intent.hasExtra(DownloadManager.EXTRA_NOTIFICATION_CLICK_DOWNLOAD_IDS)) { - // we have been passed multiple download id's - just return the first one - long[] downloadIds = intent.getLongArrayExtra(DownloadManager.EXTRA_NOTIFICATION_CLICK_DOWNLOAD_IDS); - if (downloadIds != null && downloadIds.length > 0) { - return downloadIds[0]; - } - } - } - - return -1; - } - - /** - * Check if a download is running for the specified id - * @return -1 if not downloading, else the id from the Android download manager - */ - public static long isDownloading(Context context, String uniqueDownloadId) { - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.GINGERBREAD) { - // TODO: remove. This is necessary because AppDetails calls this - // static method directly, without using the whole pipe through - // DownloaderFactory. This shouldn't be called at all on android-8 - // devices, since AppDetails is really using the old downloader, - // not this one. - return -1; - } - DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - DownloadManager.Query query = new DownloadManager.Query(); - Cursor c = dm.query(query); - if (c == null) { - // TODO: same as above. - return -1; - } - int columnUniqueDownloadId = c.getColumnIndex(DownloadManager.COLUMN_DESCRIPTION); - int columnId = c.getColumnIndex(DownloadManager.COLUMN_ID); - - try { - while (c.moveToNext()) { - if (uniqueDownloadId.equals(c.getString(columnUniqueDownloadId))) { - return c.getLong(columnId); - } - } - } finally { - c.close(); - } - - return -1; - } - - /** - * Check if a specific download is complete. - * @return -1 if download is not complete, otherwise the download id - */ - private static long isDownloadComplete(Context context, String uniqueDownloadId) { - DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - DownloadManager.Query query = new DownloadManager.Query(); - query.setFilterByStatus(DownloadManager.STATUS_SUCCESSFUL); - Cursor c = dm.query(query); - int columnUniqueDownloadId = c.getColumnIndex(DownloadManager.COLUMN_DESCRIPTION); - int columnId = c.getColumnIndex(DownloadManager.COLUMN_ID); - - try { - while (c.moveToNext()) { - if (uniqueDownloadId.equals(c.getString(columnUniqueDownloadId))) { - return c.getLong(columnId); - } - } - } finally { - c.close(); - } - - return -1; - } - - /** - * Check if download was valid, see issue - * http://code.google.com/p/android/issues/detail?id=18462 - * From http://stackoverflow.com/questions/8937817/downloadmanager-action-download-complete-broadcast-receiver-receiving-same-downl - * @return 0 if successful, -1 if download doesn't exist, else the DownloadManager.ERROR_... code - */ - public static int validDownload(Context context, long downloadId) { - //Verify if download is a success - DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - Cursor c = dm.query(new DownloadManager.Query().setFilterById(downloadId)); - - try { - if (c.moveToFirst()) { - int status = c.getInt(c.getColumnIndex(DownloadManager.COLUMN_STATUS)); - - if (status == DownloadManager.STATUS_SUCCESSFUL) { - return 0; // Download is valid, celebrate - } - return c.getInt(c.getColumnIndex(DownloadManager.COLUMN_REASON)); - } - } finally { - c.close(); - } - - return -1; // download doesn't exist - } - - /** - * Broadcast receiver to listen for ACTION_DOWNLOAD_COMPLETE broadcasts - */ - private final BroadcastReceiver receiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - if (DownloadManager.ACTION_DOWNLOAD_COMPLETE.equals(intent.getAction())) { - long dId = getDownloadId(intent); - String downloadId = getDownloadId(context, dId); - if (listener != null && dId == AsyncDownloaderFromAndroid.this.downloadManagerId && downloadId != null) { - // our current download has just completed, so let's throw up install dialog - // immediately - try { - context.unregisterReceiver(receiver); - } catch (Exception e) { - // ignore if receiver already unregistered - } - - // call download() to copy the file and start the installer - download(); - } - } - } - }; -} 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 2ac93be4d..b65001cb8 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java @@ -1,14 +1,9 @@ package org.fdroid.fdroid.net; -import android.annotation.TargetApi; -import android.app.DownloadManager; import android.content.Context; import android.content.Intent; -import android.database.Cursor; -import android.os.Build; import android.support.v4.content.LocalBroadcastManager; -import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Credentials; import java.io.File; @@ -90,62 +85,11 @@ public class DownloaderFactory { } public static AsyncDownloader createAsync(Context context, String urlString, File destFile, String title, String id, Credentials credentials, AsyncDownloader.Listener listener) throws IOException { - return createAsync(context, new URL(urlString), destFile, title, id, credentials, listener); - } - - public static AsyncDownloader createAsync(Context context, URL url, File destFile, String title, String id, Credentials credentials, AsyncDownloader.Listener listener) - throws IOException { - // To re-enable, fix the following: - // * https://gitlab.com/fdroid/fdroidclient/issues/445 - // * https://gitlab.com/fdroid/fdroidclient/issues/459 - if (false && canUseDownloadManager(context, url)) { - Utils.debugLog(TAG, "Using AsyncDownloaderFromAndroid"); - return new AsyncDownloaderFromAndroid(context, listener, title, id, url.toString(), destFile); - } - Utils.debugLog(TAG, "Using AsyncDownloadWrapper"); + URL url = new URL(urlString); return new AsyncDownloadWrapper(create(context, url, destFile, credentials), listener); } private static boolean isOnionAddress(URL url) { return url.getHost().endsWith(".onion"); } - - @TargetApi(Build.VERSION_CODES.GINGERBREAD) - private static boolean hasDownloadManager(Context context) { - DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - if (dm == null) { - // Service was not found - return false; - } - DownloadManager.Query query = new DownloadManager.Query(); - Cursor c = dm.query(query); - if (c == null) { - // Download Manager was disabled - return false; - } - c.close(); - return true; - } - - /** - * Tests to see if we can use Android's DownloadManager to download the APK, instead of - * a downloader returned from DownloadFactory. - */ - private static boolean canUseDownloadManager(Context context, URL url) { - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) { - // No HTTPS support on 2.3, no DownloadManager on 2.2. Don't have - // 3.0 devices to test on, so require 4.0. - return false; - } - if (isOnionAddress(url)) { - // We support onion addresses through our own downloader. - return false; - } - if (isBluetoothAddress(url)) { - // Completely differnet protocol not understood by the download manager. - return false; - } - return hasDownloadManager(context); - } - } diff --git a/app/src/main/java/org/fdroid/fdroid/receiver/DownloadManagerReceiver.java b/app/src/main/java/org/fdroid/fdroid/receiver/DownloadManagerReceiver.java deleted file mode 100644 index 1da9af76a..000000000 --- a/app/src/main/java/org/fdroid/fdroid/receiver/DownloadManagerReceiver.java +++ /dev/null @@ -1,85 +0,0 @@ -package org.fdroid.fdroid.receiver; - -import android.annotation.TargetApi; -import android.app.DownloadManager; -import android.app.Notification; -import android.app.NotificationManager; -import android.app.PendingIntent; -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; -import android.support.annotation.StringRes; -import android.support.v4.app.NotificationCompat; - -import org.fdroid.fdroid.AppDetails; -import org.fdroid.fdroid.R; -import org.fdroid.fdroid.net.AsyncDownloaderFromAndroid; - -/** - * Receive notifications from the Android DownloadManager and pass them onto the - * AppDetails activity - */ -@TargetApi(9) -public class DownloadManagerReceiver extends BroadcastReceiver { - @Override - public void onReceive(Context context, Intent intent) { - // work out the package name to send to the AppDetails Screen - long downloadId = AsyncDownloaderFromAndroid.getDownloadId(intent); - String packageName = AsyncDownloaderFromAndroid.getDownloadId(context, downloadId); - - if (packageName == null) { - // bogus broadcast (e.g. download cancelled, but system sent a DOWNLOAD_COMPLETE) - return; - } - - if (DownloadManager.ACTION_DOWNLOAD_COMPLETE.equals(intent.getAction())) { - int status = AsyncDownloaderFromAndroid.validDownload(context, downloadId); - if (status == 0) { - // successful download - showNotification(context, packageName, intent, downloadId, R.string.tap_to_install); - } else { - // download failed! - showNotification(context, packageName, intent, downloadId, R.string.download_error); - - // clear the download to allow user to download again - DownloadManager dm = (DownloadManager) context.getSystemService(Context.DOWNLOAD_SERVICE); - dm.remove(downloadId); - } - } else if (DownloadManager.ACTION_NOTIFICATION_CLICKED.equals(intent.getAction())) { - // pass the notification click onto the AppDetails screen and let it handle it - Intent appDetails = new Intent(context, AppDetails.class); - appDetails.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_CLEAR_TOP); - appDetails.setAction(intent.getAction()); - appDetails.putExtras(intent.getExtras()); - appDetails.putExtra(AppDetails.EXTRA_APPID, packageName); - context.startActivity(appDetails); - } - } - - private void showNotification(Context context, String packageName, Intent intent, long downloadId, - @StringRes int messageResId) { - // show a notification the user can click to install the app - Intent appDetails = new Intent(context, AppDetails.class); - appDetails.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_CLEAR_TOP); - appDetails.setAction(intent.getAction()); - appDetails.putExtra(DownloadManager.EXTRA_DOWNLOAD_ID, downloadId); - appDetails.putExtra(AppDetails.EXTRA_APPID, packageName); - - // set separate pending intents per download id - PendingIntent pi = PendingIntent.getActivity( - context, (int) downloadId, appDetails, PendingIntent.FLAG_ONE_SHOT); - - // build & show notification - String downloadTitle = AsyncDownloaderFromAndroid.getDownloadTitle(context, downloadId); - Notification notif = new NotificationCompat.Builder(context) - .setContentTitle(downloadTitle) - .setContentText(context.getString(messageResId)) - .setSmallIcon(R.drawable.ic_stat_notify) - .setContentIntent(pi) - .setAutoCancel(true) - .build(); - - NotificationManager nm = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); - nm.notify((int) downloadId, notif); - } -} From 2578e6bdffe176f27bf831fa430abca5978301a8 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 23 Mar 2016 14:03:29 +0100 Subject: [PATCH 2/7] remove unused portions of DownloaderFactory Since this is internal code and not a library for use with other projects, it should only include the methods that are actually in use. The other copies are just dead code, which means more stuff to read in order to figure out. --- .../org/fdroid/fdroid/net/ApkDownloader.java | 2 +- .../fdroid/fdroid/net/DownloaderFactory.java | 25 +++---------------- 2 files changed, 4 insertions(+), 23 deletions(-) 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 7698c2ccd..8e54f1d87 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -197,7 +197,7 @@ public class ApkDownloader implements AsyncDownloader.Listener { Utils.debugLog(TAG, "Downloading apk from " + remoteAddress + " to " + localFile); try { - dlWrapper = DownloaderFactory.createAsync(context, remoteAddress, localFile, app.name + " " + curApk.version, curApk.packageName, credentials, this); + dlWrapper = DownloaderFactory.createAsync(context, remoteAddress, localFile, credentials, this); dlWrapper.download(); return true; } catch (IOException e) { 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 b65001cb8..d50896a43 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderFactory.java @@ -22,29 +22,9 @@ public class DownloaderFactory { */ public static Downloader create(Context context, String urlString) throws IOException { - return create(context, new URL(urlString)); - } - - /** - * Downloads to a temporary file, which *you must delete yourself when - * you are done. It is stored in {@link Context#getCacheDir()} and starts - * with the prefix {@code dl-}. - */ - public static Downloader create(Context context, URL url) - throws IOException { File destFile = File.createTempFile("dl-", "", context.getCacheDir()); destFile.deleteOnExit(); // this probably does nothing, but maybe... - return create(context, url, destFile); - } - - public static Downloader create(Context context, String urlString, File destFile) - throws IOException { - return create(context, new URL(urlString), destFile); - } - - public static Downloader create(Context context, URL url, File destFile) - throws IOException { - return create(context, url, destFile, null); + return create(context, new URL(urlString), destFile, null); } public static Downloader create(Context context, URL url, File destFile, Credentials credentials) @@ -84,7 +64,8 @@ public class DownloaderFactory { return "file".equalsIgnoreCase(url.getProtocol()); } - public static AsyncDownloader createAsync(Context context, String urlString, File destFile, String title, String id, Credentials credentials, AsyncDownloader.Listener listener) throws IOException { + public static AsyncDownloader createAsync(Context context, String urlString, File destFile, Credentials credentials, AsyncDownloader.Listener listener) + throws IOException { URL url = new URL(urlString); return new AsyncDownloadWrapper(create(context, url, destFile, credentials), listener); } From 88b5e284b5193aa556bff7024a7dc7a55caa78f2 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 28 Mar 2016 20:08:31 +0200 Subject: [PATCH 3/7] use commons-io via a gradle dependency This makes it so gradle provides all dependencies, rather than a mix of classes that are copied in versus imported via gradle. This library is already used by the tests, so its not really a new dependency, and proguard should remove all the unused stuff. --- app/build.gradle | 2 + .../commons/io/input/BoundedInputStream.java | 231 ------------------ 2 files changed, 2 insertions(+), 231 deletions(-) delete mode 100644 app/src/main/java/org/apache/commons/io/input/BoundedInputStream.java diff --git a/app/build.gradle b/app/build.gradle index 5fea53e9c..04108e0da 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -19,6 +19,7 @@ dependencies { compile 'eu.chainfire:libsuperuser:1.0.0.201602271131' compile 'cc.mvdan.accesspoint:library:0.1.3' compile 'info.guardianproject.netcipher:netcipher:1.2.1' + compile 'commons-io:commons-io:2.4' compile 'commons-net:commons-net:3.4' compile 'org.openhab.jmdns:jmdns:3.4.2' compile('ch.acra:acra:4.8.2') { @@ -72,6 +73,7 @@ if (!hasProperty('sourceDeps')) { 'com.google.zxing:core:b4d82452e7a6bf6ec2698904b332431717ed8f9a850224f295aec89de80f2259', 'eu.chainfire:libsuperuser:018344ff19ee94d252c14b4a503ee8b519184db473a5af83513f5837c413b128', 'cc.mvdan.accesspoint:library:dc89a085d6bc40381078b8dd7776b12bde0dbaf8ffbcddb17ec4ebc3edecc7ba', + 'commons-io:commons-io:cc6a41dc3eaacc9e440a6bd0d2890b20d36b4ee408fe2d67122f328bb6e01581', 'commons-net:commons-net:38cf2eca826b8bcdb236fc1f2e79e0c6dd8e7e0f5c44a3b8e839a1065b2fbe2e', 'info.guardianproject.netcipher:netcipher:611ec5bde9d799fd57e1efec5c375f9f460de2cdda98918541decc9a7d02f2ad', 'org.openhab.jmdns:jmdns:7a4b34b5606bbd2aff7fdfe629edcb0416fccd367fb59a099f210b9aba4f0bce', diff --git a/app/src/main/java/org/apache/commons/io/input/BoundedInputStream.java b/app/src/main/java/org/apache/commons/io/input/BoundedInputStream.java deleted file mode 100644 index 7eae22a52..000000000 --- a/app/src/main/java/org/apache/commons/io/input/BoundedInputStream.java +++ /dev/null @@ -1,231 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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.apache.commons.io.input; - -import java.io.IOException; -import java.io.InputStream; - -/** - * This is a stream that will only supply bytes up to a certain length - if its - * position goes above that, it will stop. - *

- * This is useful to wrap ServletInputStreams. The ServletInputStream will block - * if you try to read content from it that isn't there, because it doesn't know - * whether the content hasn't arrived yet or whether the content has finished. - * So, one of these, initialized with the Content-length sent in the - * ServletInputStream's header, will stop it blocking, providing it's been sent - * with a correct content length. - * - * @version $Id: BoundedInputStream.java 1307462 2012-03-30 15:13:11Z ggregory $ - * @since 2.0 - */ -public class BoundedInputStream extends InputStream { - - /** the wrapped input stream */ - private final InputStream in; - - /** the max length to provide */ - private final long max; - - /** the number of bytes already returned */ - private long pos; - - /** the marked position */ - private long mark = -1; - - /** flag if close shoud be propagated */ - private boolean propagateClose = true; - - /** - * Creates a new BoundedInputStream that wraps the given input - * stream and limits it to a certain size. - * - * @param in The wrapped input stream - * @param size The maximum number of bytes to return - */ - public BoundedInputStream(InputStream in, long size) { - // Some badly designed methods - eg the servlet API - overload length - // such that "-1" means stream finished - this.max = size; - this.in = in; - } - - /** - * Creates a new BoundedInputStream that wraps the given input - * stream and is unlimited. - * - * @param in The wrapped input stream - */ - public BoundedInputStream(InputStream in) { - this(in, -1); - } - - /** - * Invokes the delegate's read() method if - * the current position is less than the limit. - * @return the byte read or -1 if the end of stream or - * the limit has been reached. - * @throws IOException if an I/O error occurs - */ - @Override - public int read() throws IOException { - if (max >= 0 && pos >= max) { - return -1; - } - int result = in.read(); - pos++; - return result; - } - - /** - * Invokes the delegate's read(byte[]) method. - * @param b the buffer to read the bytes into - * @return the number of bytes read or -1 if the end of stream or - * the limit has been reached. - * @throws IOException if an I/O error occurs - */ - @Override - public int read(byte[] b) throws IOException { - return this.read(b, 0, b.length); - } - - /** - * Invokes the delegate's read(byte[], int, int) method. - * @param b the buffer to read the bytes into - * @param off The start offset - * @param len The number of bytes to read - * @return the number of bytes read or -1 if the end of stream or - * the limit has been reached. - * @throws IOException if an I/O error occurs - */ - @Override - public int read(byte[] b, int off, int len) throws IOException { - if (max >= 0 && pos >= max) { - return -1; - } - long maxRead = max >= 0 ? Math.min(len, max - pos) : len; - int bytesRead = in.read(b, off, (int) maxRead); - - if (bytesRead == -1) { - return -1; - } - - pos += bytesRead; - return bytesRead; - } - - /** - * Invokes the delegate's skip(long) method. - * @param n the number of bytes to skip - * @return the actual number of bytes skipped - * @throws IOException if an I/O error occurs - */ - @Override - public long skip(long n) throws IOException { - long toSkip = max >= 0 ? Math.min(n, max - pos) : n; - long skippedBytes = in.skip(toSkip); - pos += skippedBytes; - return skippedBytes; - } - - /** - * {@inheritDoc} - */ - @Override - public int available() throws IOException { - if (max >= 0 && pos >= max) { - return 0; - } - return in.available(); - } - - /** - * Invokes the delegate's toString() method. - * @return the delegate's toString() - */ - @Override - public String toString() { - return in.toString(); - } - - /** - * Invokes the delegate's close() method - * if {@link #isPropagateClose()} is {@code true}. - * @throws IOException if an I/O error occurs - */ - @Override - public void close() throws IOException { - if (propagateClose) { - in.close(); - } - } - - /** - * Invokes the delegate's reset() method. - * @throws IOException if an I/O error occurs - */ - @Override - public synchronized void reset() throws IOException { - in.reset(); - pos = mark; - } - - /** - * Invokes the delegate's mark(int) method. - * @param readlimit read ahead limit - */ - @Override - public synchronized void mark(int readlimit) { - in.mark(readlimit); - mark = pos; - } - - /** - * Invokes the delegate's markSupported() method. - * @return true if mark is supported, otherwise false - */ - @Override - public boolean markSupported() { - return in.markSupported(); - } - - /** - * Indicates whether the {@link #close()} method - * should propagate to the underling {@link InputStream}. - * - * @return {@code true} if calling {@link #close()} - * propagates to the close() method of the - * underlying stream or {@code false} if it does not. - */ - public boolean isPropagateClose() { - return propagateClose; - } - - /** - * Set whether the {@link #close()} method - * should propagate to the underling {@link InputStream}. - * - * @param propagateClose {@code true} if calling - * {@link #close()} propagates to the close() - * method of the underlying stream or - * {@code false} if it does not. - */ - public void setPropagateClose(boolean propagateClose) { - this.propagateClose = propagateClose; - } -} From 2019b7a7c38aee76f0907b5cb783d51839346538 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 28 Mar 2016 16:52:58 +0200 Subject: [PATCH 4/7] make all Downloader downloads cancelable Allowing all downloads, including updates, to be canceled simplifies the code and if the user wants to cancel an update, they should be able to. But canceling updates is not implemented in this commit. --- .../main/java/org/fdroid/fdroid/RepoUpdater.java | 5 ++++- .../java/org/fdroid/fdroid/net/Downloader.java | 14 -------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index e1fd27d9c..f38c4c3df 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -102,7 +102,7 @@ public class RepoUpdater { repo.getCredentials() ); downloader.setCacheTag(repo.lastetag); - downloader.downloadUninterrupted(); + downloader.download(); if (downloader.isCached()) { // The index is unchanged since we last read it. We just mark @@ -118,6 +118,9 @@ public class RepoUpdater { } throw new UpdateException(repo, "Error getting index file", e); + } catch (InterruptedException e) { + // ignored if canceled, the local database just won't be updated + e.printStackTrace(); } return downloader; } 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 b66dcc7ec..e66b793ad 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java @@ -91,20 +91,6 @@ public abstract class Downloader { protected abstract int totalDownloadSize(); - /** - * Helper function for synchronous downloads (i.e. those *not* using AsyncDownloadWrapper), - * which don't really want to bother dealing with an InterruptedException. - * The InterruptedException thrown from download() is there to enable cancelling asynchronous - * downloads, but regular synchronous downloads cannot be cancelled because download() will - * block until completed. - * @throws IOException - */ - public void downloadUninterrupted() throws IOException { - try { - download(); - } catch (InterruptedException ignored) { } - } - public abstract void download() throws IOException, InterruptedException; public abstract boolean isCached(); From 9c47f56f03e423f3a598b2e0db554513e794ac24 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 29 Mar 2016 11:23:34 +0200 Subject: [PATCH 5/7] remove unused event message: ApkDownloader.ERROR_DOWNLOAD_FAILED --- app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java | 2 -- 1 file changed, 2 deletions(-) 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 8e54f1d87..c179c0025 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -59,7 +59,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { public static final String EXTRA_URL = "apkDownloadUrl"; public static final int ERROR_HASH_MISMATCH = 101; - public static final int ERROR_DOWNLOAD_FAILED = 102; private static final String EVENT_SOURCE_ID = "sourceId"; private static long downloadIdCounter; @@ -236,7 +235,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { @Override public void onErrorDownloading(String localisedExceptionDetails) { Log.e(TAG, "Download failed: " + localisedExceptionDetails); - sendError(ERROR_DOWNLOAD_FAILED); delete(localFile); } From ab709e171a7fb1c6c77712c0a8c2bbebe3a12d6b Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 29 Mar 2016 11:56:17 +0200 Subject: [PATCH 6/7] remove ApkDownloader.EXTRA_TYPE, it is entirely unused This constant is wired up, but ultimately does nothing at all, since all of the cases in the switch do the exact same thing. --- .../java/org/fdroid/fdroid/net/ApkDownloader.java | 2 -- .../org/fdroid/fdroid/views/swap/SwapAppsView.java | 14 ++------------ 2 files changed, 2 insertions(+), 14 deletions(-) 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 c179c0025..42d9c0ef6 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -55,7 +55,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { public static final String EVENT_ERROR = "apkDownloadError"; public static final String ACTION_STATUS = "apkDownloadStatus"; - public static final String EXTRA_TYPE = "apkDownloadStatusType"; public static final String EXTRA_URL = "apkDownloadUrl"; public static final int ERROR_HASH_MISMATCH = 101; @@ -227,7 +226,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { Intent intent = new Intent(ACTION_STATUS); intent.putExtras(event.getData()); - intent.putExtra(EXTRA_TYPE, event.type); intent.putExtra(EXTRA_URL, Utils.getApkUrl(repoAddress, curApk)); LocalBroadcastManager.getInstance(context).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 2598c76a2..664884a20 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 @@ -279,18 +279,8 @@ public class SwapAppsView extends ListView implements // 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.getStringExtra(ApkDownloader.EXTRA_URL); - if (!TextUtils.equals(Utils.getApkUrl(apk.repoAddress, apk), broadcastUrl)) { - return; - } - - switch (intent.getStringExtra(ApkDownloader.EXTRA_TYPE)) { - // Fallthrough for each of these "downloader no longer going" events... - case ApkDownloader.EVENT_APK_DOWNLOAD_COMPLETE: - case ApkDownloader.EVENT_APK_DOWNLOAD_CANCELLED: - case ApkDownloader.EVENT_ERROR: - case ApkDownloader.EVENT_DATA_ERROR_TYPE: - resetView(); - break; + if (TextUtils.equals(Utils.getApkUrl(apk.repoAddress, apk), broadcastUrl)) { + resetView(); } } }; From d76d7aa367f58478d4fac3f722567a4097c3cc04 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 29 Mar 2016 12:21:56 +0200 Subject: [PATCH 7/7] remove unused ApkDownloader.EVENT_APK_DOWNLOAD_CANCELLED This also removes all the related stuff that resulted in EVENT_APK_DOWNLOAD_CANCELLED being sent. Since EVENT_APK_DOWNLOAD_CANCELLED ultimately does nothing, that whole bit of plumbing is unused. --- app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java | 6 ------ .../java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java | 6 +----- .../main/java/org/fdroid/fdroid/net/AsyncDownloader.java | 2 -- .../org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java | 2 -- 4 files changed, 1 insertion(+), 15 deletions(-) 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 42d9c0ef6..0fa6811b6 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/ApkDownloader.java @@ -51,7 +51,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { private static final String TAG = "ApkDownloader"; public static final String EVENT_APK_DOWNLOAD_COMPLETE = "apkDownloadComplete"; - public static final String EVENT_APK_DOWNLOAD_CANCELLED = "apkDownloadCancelled"; public static final String EVENT_ERROR = "apkDownloadError"; public static final String ACTION_STATUS = "apkDownloadStatus"; @@ -257,11 +256,6 @@ public class ApkDownloader implements AsyncDownloader.Listener { prepareApkFileAndSendCompleteMessage(); } - @Override - public void onDownloadCancelled() { - sendMessage(EVENT_APK_DOWNLOAD_CANCELLED); - } - @Override public void onProgress(Event event) { sendProgressEvent(event); diff --git a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java index 84618bb7f..ff559e201 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java +++ b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloadWrapper.java @@ -12,7 +12,6 @@ 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_DOWNLOAD_CANCELLED = 3; private static final int MSG_ERROR = 4; private static final String MSG_DATA = "data"; @@ -61,9 +60,6 @@ class AsyncDownloadWrapper extends Handler implements AsyncDownloader { case MSG_DOWNLOAD_COMPLETE: listener.onDownloadComplete(); break; - case MSG_DOWNLOAD_CANCELLED: - listener.onDownloadCancelled(); - break; case MSG_ERROR: listener.onErrorDownloading(message.getData().getString(MSG_DATA)); break; @@ -77,7 +73,7 @@ class AsyncDownloadWrapper extends Handler implements AsyncDownloader { downloader.download(); sendMessage(MSG_DOWNLOAD_COMPLETE); } catch (InterruptedException e) { - sendMessage(MSG_DOWNLOAD_CANCELLED); + // ignored } catch (IOException e) { Log.e(TAG, "I/O exception in download thread", e); Bundle data = new Bundle(1); diff --git a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java index d0cfc77a4..0c70895f9 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/AsyncDownloader.java @@ -8,8 +8,6 @@ public interface AsyncDownloader { void onErrorDownloading(String localisedExceptionDetails); void onDownloadComplete(); - - void onDownloadCancelled(); } int getBytesRead(); 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 b5f8ed5c0..a3b521f21 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 @@ -789,8 +789,6 @@ public class SwapWorkflowActivity extends AppCompatActivity { case ApkDownloader.EVENT_APK_DOWNLOAD_COMPLETE: handleDownloadComplete(downloader.localFile(), app.packageName); break; - case ApkDownloader.EVENT_ERROR: - break; } } });