Merge branch 'mirror-tweaks' into 'master'

mirror tweaks

Closes #1702

See merge request fdroid/fdroidclient!789

app/src/androidTest/assets/extendedPerms.xml

@@ -68,7 +68,7 @@
             </permissions>
             <uses-permission name="android.permission.GET_ACCOUNTS" maxSdkVersion="22" />
             <uses-permission name="android.permission.READ_EXTERNAL_STORAGE" maxSdkVersion="18" />
-            <uses-permission name="android.permission.WRITE_EXTERNAL_STORAGE" maxSdkVersion="18" />
+            <uses-permission name="android.permission.WRITE_EXTERNAL_STORAGE" />
             <uses-permission name="android.permission.AUTHENTICATE_ACCOUNTS" maxSdkVersion="22" />
             <uses-permission name="android.permission.MANAGE_ACCOUNTS" maxSdkVersion="22" />
         </package>
@@ -85,7 +85,7 @@
             <targetSdkVersion>23</targetSdkVersion>
             <added>2016-06-26</added>
             <permissions>
-                org.dmfs.permission.READ_TASKS,READ_EXTERNAL_STORAGE,WRITE_CONTACTS,GET_ACCOUNTS,AUTHENTICATE_ACCOUNTS,WRITE_EXTERNAL_STORAGE,READ_CALENDAR,ACCESS_WIFI_STATE,org.dmfs.permission.WRITE_TASKS,ACCESS_NETWORK_STATE,WRITE_CALENDAR,READ_CONTACTS,READ_SYNC_SETTINGS,INTERNET,MANAGE_ACCOUNTS,WRITE_SYNC_SETTINGS
+                org.dmfs.permission.READ_TASKS,WRITE_CONTACTS,GET_ACCOUNTS,AUTHENTICATE_ACCOUNTS,WRITE_EXTERNAL_STORAGE,READ_CALENDAR,ACCESS_WIFI_STATE,org.dmfs.permission.WRITE_TASKS,ACCESS_NETWORK_STATE,WRITE_CALENDAR,READ_CONTACTS,READ_SYNC_SETTINGS,INTERNET,MANAGE_ACCOUNTS,WRITE_SYNC_SETTINGS
             </permissions>
         </package>
         <package>

app/src/androidTest/java/org/fdroid/fdroid/installer/ApkVerifierTest.java

@@ -26,13 +26,12 @@ import android.support.annotation.NonNull;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.runner.AndroidJUnit4;
 import android.util.Log;
-
 import org.fdroid.fdroid.AssetUtils;
-import org.fdroid.fdroid.data.RepoXMLHandler;
 import org.fdroid.fdroid.Utils;
 import org.fdroid.fdroid.compat.FileCompatTest;
 import org.fdroid.fdroid.data.Apk;
 import org.fdroid.fdroid.data.Repo;
+import org.fdroid.fdroid.data.RepoXMLHandler;
 import org.fdroid.fdroid.mock.RepoDetails;
 import org.junit.Before;
 import org.junit.Test;
@@ -45,6 +44,7 @@ import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.TreeSet;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -103,7 +103,7 @@ public class ApkVerifierTest {
     public void testNulls() {
         assertTrue(ApkVerifier.requestedPermissionsEqual(null, null));
 
-        String[] perms = new String[] {"Blah"};
+        String[] perms = new String[]{"Blah"};
         assertFalse(ApkVerifier.requestedPermissionsEqual(perms, null));
         assertFalse(ApkVerifier.requestedPermissionsEqual(null, perms));
     }
@@ -290,7 +290,7 @@ public class ApkVerifierTest {
     public void testExtendedPerms() throws IOException,
             ApkVerifier.ApkPermissionUnequalException, ApkVerifier.ApkVerificationException {
         RepoDetails actualDetails = getFromFile(extendedPermsXml);
-        HashSet<String> expectedSet = new HashSet<>(Arrays.asList(new String[]{
+        HashSet<String> expectedSet = new HashSet<>(Arrays.asList(
                 "android.permission.ACCESS_NETWORK_STATE",
                 "android.permission.ACCESS_WIFI_STATE",
                 "android.permission.INTERNET",
@@ -301,8 +301,8 @@ public class ApkVerifierTest {
                 "android.permission.READ_CONTACTS",
                 "android.permission.WRITE_CONTACTS",
                 "android.permission.READ_CALENDAR",
-                "android.permission.WRITE_CALENDAR",
+                "android.permission.WRITE_CALENDAR"
-        }));
+        ));
         if (Build.VERSION.SDK_INT <= 18) {
             expectedSet.add("android.permission.READ_EXTERNAL_STORAGE");
             expectedSet.add("android.permission.WRITE_EXTERNAL_STORAGE");
@@ -345,6 +345,87 @@ public class ApkVerifierTest {
         apkVerifier.verifyApk();
     }
 
+    @Test
+    public void testImpliedPerms() throws IOException {
+        RepoDetails actualDetails = getFromFile(extendedPermsXml);
+        TreeSet<String> expectedSet = new TreeSet<>(Arrays.asList(
+                "android.permission.ACCESS_NETWORK_STATE",
+                "android.permission.ACCESS_WIFI_STATE",
+                "android.permission.INTERNET",
+                "android.permission.READ_CALENDAR",
+                "android.permission.READ_CONTACTS",
+                "android.permission.READ_EXTERNAL_STORAGE",
+                "android.permission.READ_SYNC_SETTINGS",
+                "android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS",
+                "android.permission.WRITE_CALENDAR",
+                "android.permission.WRITE_CONTACTS",
+                "android.permission.WRITE_EXTERNAL_STORAGE",
+                "android.permission.WRITE_SYNC_SETTINGS",
+                "org.dmfs.permission.READ_TASKS",
+                "org.dmfs.permission.WRITE_TASKS"
+        ));
+        if (Build.VERSION.SDK_INT <= 22) { // maxSdkVersion="22"
+            expectedSet.addAll(Arrays.asList(
+                    "android.permission.AUTHENTICATE_ACCOUNTS",
+                    "android.permission.GET_ACCOUNTS",
+                    "android.permission.MANAGE_ACCOUNTS"
+            ));
+        }
+        Apk apk = actualDetails.apks.get(1);
+        Log.i(TAG, "APK: " + apk.apkName);
+        HashSet<String> actualSet = new HashSet<>(Arrays.asList(apk.requestedPermissions));
+        for (String permission : expectedSet) {
+            if (!actualSet.contains(permission)) {
+                Log.i(TAG, permission + " in expected but not actual! (android-"
+                        + Build.VERSION.SDK_INT + ")");
+            }
+        }
+        for (String permission : actualSet) {
+            if (!expectedSet.contains(permission)) {
+                Log.i(TAG, permission + " in actual but not expected! (android-"
+                        + Build.VERSION.SDK_INT + ")");
+            }
+        }
+        String[] expectedPermissions = expectedSet.toArray(new String[expectedSet.size()]);
+        assertTrue(ApkVerifier.requestedPermissionsEqual(expectedPermissions, apk.requestedPermissions));
+
+        expectedSet = new TreeSet<>(Arrays.asList(
+                "android.permission.ACCESS_NETWORK_STATE",
+                "android.permission.ACCESS_WIFI_STATE",
+                "android.permission.AUTHENTICATE_ACCOUNTS",
+                "android.permission.GET_ACCOUNTS",
+                "android.permission.INTERNET",
+                "android.permission.MANAGE_ACCOUNTS",
+                "android.permission.READ_CALENDAR",
+                "android.permission.READ_CONTACTS",
+                "android.permission.READ_EXTERNAL_STORAGE",
+                "android.permission.READ_SYNC_SETTINGS",
+                "android.permission.WRITE_CALENDAR",
+                "android.permission.WRITE_CONTACTS",
+                "android.permission.WRITE_EXTERNAL_STORAGE",
+                "android.permission.WRITE_SYNC_SETTINGS",
+                "org.dmfs.permission.READ_TASKS",
+                "org.dmfs.permission.WRITE_TASKS"
+        ));
+        expectedPermissions = expectedSet.toArray(new String[expectedSet.size()]);
+        apk = actualDetails.apks.get(2);
+        Log.i(TAG, "APK: " + apk.apkName);
+        actualSet = new HashSet<>(Arrays.asList(apk.requestedPermissions));
+        for (String permission : expectedSet) {
+            if (!actualSet.contains(permission)) {
+                Log.i(TAG, permission + " in expected but not actual! (android-"
+                        + Build.VERSION.SDK_INT + ")");
+            }
+        }
+        for (String permission : actualSet) {
+            if (!expectedSet.contains(permission)) {
+                Log.i(TAG, permission + " in actual but not expected! (android-"
+                        + Build.VERSION.SDK_INT + ")");
+            }
+        }
+        assertTrue(ApkVerifier.requestedPermissionsEqual(expectedPermissions, apk.requestedPermissions));
+    }
+
     @NonNull
     private RepoDetails getFromFile(File indexFile) throws IOException {
         InputStream inputStream = null;

app/src/main/java/org/fdroid/fdroid/FDroidApp.java

@@ -71,6 +71,7 @@ import org.fdroid.fdroid.installer.ApkFileProvider;
 import org.fdroid.fdroid.installer.InstallHistoryService;
 import org.fdroid.fdroid.localrepo.SDCardScannerService;
 import org.fdroid.fdroid.net.ConnectivityMonitorService;
+import org.fdroid.fdroid.net.Downloader;
 import org.fdroid.fdroid.net.HttpDownloader;
 import org.fdroid.fdroid.net.ImageLoaderForUIL;
 import org.fdroid.fdroid.net.WifiStateChangeService;
@@ -126,7 +127,7 @@ public class FDroidApp extends Application {
 
     private static volatile LongSparseArray<String> lastWorkingMirrorArray = new LongSparseArray<>(1);
     private static volatile int numTries = Integer.MAX_VALUE;
-    private static volatile int timeout = 10000;
+    private static volatile int timeout = Downloader.DEFAULT_TIMEOUT;
 
     // Leaving the fully qualified class name here to help clarify the difference between spongy/bouncy castle.
     private static final org.bouncycastle.jce.provider.BouncyCastleProvider BOUNCYCASTLE_PROVIDER;
@@ -244,10 +245,26 @@ public class FDroidApp extends Application {
         repo = new Repo();
     }
 
+    /**
+     * @see #getMirror(String, Repo)
+     */
     public static String getMirror(String urlString, long repoId) throws IOException {
         return getMirror(urlString, RepoProvider.Helper.findById(getInstance(), repoId));
     }
 
+    /**
+     * Each time this is called, it will return a mirror from the pool of
+     * mirrors.  If it reaches the end of the list of mirrors, it will start
+     * again from the stop, while setting the timeout to
+     * {@link Downloader#SECOND_TIMEOUT}.  If it reaches the end of the list
+     * again, it will do one last pass through the list with the timeout set to
+     * {@link Downloader#LONGEST_TIMEOUT}.  After that, this gives up with a
+     * {@link IOException}.
+     *
+     * @see #resetMirrorVars()
+     * @see #getTimeout()
+     * @see Repo#getMirror(String)
+     */
     public static String getMirror(String urlString, Repo repo2) throws IOException {
         if (repo2.hasMirrors()) {
             String lastWorkingMirror = lastWorkingMirrorArray.get(repo2.getId());
@@ -255,11 +272,11 @@ public class FDroidApp extends Application {
                 lastWorkingMirror = repo2.address;
             }
             if (numTries <= 0) {
-                if (timeout == 10000) {
+                if (timeout == Downloader.DEFAULT_TIMEOUT) {
-                    timeout = 30000;
+                    timeout = Downloader.SECOND_TIMEOUT;
                     numTries = Integer.MAX_VALUE;
-                } else if (timeout == 30000) {
+                } else if (timeout == Downloader.SECOND_TIMEOUT) {
-                    timeout = 60000;
+                    timeout = Downloader.LONGEST_TIMEOUT;
                     numTries = Integer.MAX_VALUE;
                 } else {
                     Utils.debugLog(TAG, "Mirrors: Giving up");
@@ -291,7 +308,7 @@ public class FDroidApp extends Application {
             lastWorkingMirrorArray.removeAt(i);
         }
         numTries = Integer.MAX_VALUE;
-        timeout = 10000;
+        timeout = Downloader.DEFAULT_TIMEOUT;
     }
 
     @Override
@@ -437,6 +454,7 @@ public class FDroidApp extends Application {
         ImageLoader.getInstance().init(config);
 
         if (preferences.isIndexNeverUpdated()) {
+            preferences.setDefaultForDataOnlyConnection(this);
             // force this check to ensure it starts fetching the index on initial runs
             networkState = ConnectivityMonitorService.getNetworkState(this);
         }

app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java

@@ -65,9 +65,9 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeSet;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
@@ -294,11 +294,10 @@ public class IndexV1Updater extends IndexUpdater {
         repo.icon = getStringRepoValue(repoMap, "icon");
         repo.description = getStringRepoValue(repoMap, "description");
 
-        // ensure the canonical URL is included in the "mirrors" list
+        // ensure the canonical URL is included in the "mirrors" list as the first entry
-        List<String> mirrorsList = getStringListRepoValue(repoMap, "mirrors");
+        TreeSet<String> mirrors = new TreeSet<>();
-        HashSet<String> mirrors = new HashSet<>(mirrorsList.size() + 1);
-        mirrors.addAll(mirrorsList);
         mirrors.add(repo.address);
+        mirrors.addAll(getStringListRepoValue(repoMap, "mirrors"));
         repo.mirrors = mirrors.toArray(new String[mirrors.size()]);
 
         // below are optional, can be default value

app/src/main/java/org/fdroid/fdroid/Preferences.java

@@ -25,6 +25,8 @@ package org.fdroid.fdroid;
 import android.annotation.SuppressLint;
 import android.content.Context;
 import android.content.SharedPreferences;
+import android.net.ConnectivityManager;
+import android.net.NetworkInfo;
 import android.os.Build;
 import android.support.v7.preference.PreferenceManager;
 import android.text.format.DateUtils;
@@ -443,6 +445,26 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh
         return preferences.getInt(PREF_OVER_DATA, IGNORED_I);
     }
 
+    /**
+     * Some users never use WiFi, this lets us check for that state on first run.
+     */
+    public void setDefaultForDataOnlyConnection(Context context) {
+        ConnectivityManager cm = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
+        if (cm == null) {
+            return;
+        }
+        NetworkInfo activeNetwork = cm.getActiveNetworkInfo();
+        if (activeNetwork == null || !activeNetwork.isConnectedOrConnecting()) {
+            return;
+        }
+        if (activeNetwork.getType() == ConnectivityManager.TYPE_MOBILE) {
+            NetworkInfo wifiNetwork = cm.getNetworkInfo(ConnectivityManager.TYPE_WIFI);
+            if (!wifiNetwork.isConnectedOrConnecting()) {
+                preferences.edit().putInt(PREF_OVER_DATA, OVER_NETWORK_ALWAYS).apply();
+            }
+        }
+    }
+
     /**
      * 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

app/src/main/java/org/fdroid/fdroid/data/Apk.java

@@ -1,5 +1,6 @@
 package org.fdroid.fdroid.data;
 
+import android.Manifest;
 import android.annotation.TargetApi;
 import android.content.ContentValues;
 import android.content.Context;
@@ -486,6 +487,16 @@ public class Apk extends ValueObject implements Comparable<Apk>, Parcelable {
         setRequestedPermissions(permissions, 23);
     }
 
+    /**
+     * Generate the set of requested permissions for the current Android version.
+     * <p>
+     * There are also a bunch of crazy rules where having one permission will imply
+     * another permission, for example, {@link Manifest.permission#WRITE_EXTERNAL_STORAGE}
+     * implies {@code Manifest.permission#READ_EXTERNAL_STORAGE}.  Many of these rules
+     * are for quite old Android versions, so they are not included here.
+     *
+     * @see Manifest.permission#READ_EXTERNAL_STORAGE
+     */
     private void setRequestedPermissions(Object[][] permissions, int minSdk) {
         HashSet<String> set = new HashSet<>();
         if (requestedPermissions != null) {
@@ -500,6 +511,9 @@ public class Apk extends ValueObject implements Comparable<Apk>, Parcelable {
                 set.add((String) versions[0]);
             }
         }
+        if (Build.VERSION.SDK_INT >= 16 && set.contains(Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+            set.add(Manifest.permission.READ_EXTERNAL_STORAGE);
+        }
         requestedPermissions = set.toArray(new String[set.size()]);
     }
 

app/src/main/java/org/fdroid/fdroid/data/Repo.java

@@ -389,6 +389,10 @@ public class Repo extends ValueObject {
      * URL in the mirrors list so the mirror logic works on the first index
      * update.  That makes it possible to do the first index update via SD Card
      * or USB OTG drive.
+     *
+     * @see FDroidApp#resetMirrorVars()
+     * @see FDroidApp#getMirror(String, Repo)
+     * @see FDroidApp#getTimeout()
      */
     public String getMirror(String lastWorkingMirror) {
         if (TextUtils.isEmpty(lastWorkingMirror)) {

app/src/main/java/org/fdroid/fdroid/data/RepoXMLHandler.java

@@ -19,6 +19,7 @@
 
 package org.fdroid.fdroid.data;
 
+import android.Manifest;
 import android.os.Build;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
@@ -98,6 +99,10 @@ public class RepoXMLHandler extends DefaultHandler {
         if ("application".equals(localName) && curapp != null) {
             onApplicationParsed();
         } else if ("package".equals(localName) && curapk != null && curapp != null) {
+            if (Build.VERSION.SDK_INT >= 16 &&
+                    requestedPermissionsSet.contains(Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+                requestedPermissionsSet.add(Manifest.permission.READ_EXTERNAL_STORAGE);
+            }
             int size = requestedPermissionsSet.size();
             curapk.requestedPermissions = requestedPermissionsSet.toArray(new String[size]);
             requestedPermissionsSet.clear();

app/src/main/java/org/fdroid/fdroid/net/Downloader.java

@@ -2,7 +2,7 @@ package org.fdroid.fdroid.net;
 
 import android.net.Uri;
 import android.support.annotation.NonNull;
-
+import android.text.format.DateUtils;
 import org.fdroid.fdroid.ProgressListener;
 import org.fdroid.fdroid.Utils;
 
@@ -33,6 +33,10 @@ public abstract class Downloader {
     public static final String EXTRA_CANONICAL_URL = "org.fdroid.fdroid.net.Downloader.extra.ERROR_CANONICAL_URL";
     public static final String EXTRA_MIRROR_URL = "org.fdroid.fdroid.net.Downloader.extra.ERROR_MIRROR_URL";
 
+    public static final int DEFAULT_TIMEOUT = 10000;
+    public static final int SECOND_TIMEOUT = (int) DateUtils.MINUTE_IN_MILLIS;
+    public static final int LONGEST_TIMEOUT = 600000; // 10 minutes
+
     private volatile boolean cancelled = false;
     private volatile long bytesRead;
     private volatile long totalBytes;
@@ -43,7 +47,7 @@ public abstract class Downloader {
     String cacheTag;
     boolean notFound;
 
-    private volatile int timeout = 10000;
+    private volatile int timeout = DEFAULT_TIMEOUT;
 
     /**
      * For sending download progress, should only be called in {@link #progressTask}

app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java

@@ -176,6 +176,7 @@ public class HttpDownloader extends Downloader {
 
         connection.setRequestProperty("User-Agent", "F-Droid " + BuildConfig.VERSION_NAME);
         connection.setConnectTimeout(getTimeout());
+        connection.setReadTimeout(getTimeout());
 
         if (Build.VERSION.SDK_INT < 19) { // gzip encoding can be troublesome on old Androids
             connection.setRequestProperty("Accept-Encoding", "identity");

app/src/main/res/values/strings.xml

@@ -51,7 +51,7 @@
         them
     </string>
     <string name="update_auto_install">Automatically install updates</string>
-    <string name="update_auto_install_summary">Download and install update apps in the background</string>
+    <string name="update_auto_install_summary">Download and update apps in the background, showing a notification</string>
     <string name="notify">Show available updates</string>
     <string name="notify_on">Show a notification when updates are available</string>
     <string name="system_installer">Privileged Extension</string>

app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java

@@ -38,6 +38,8 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -53,6 +55,7 @@ import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
@@ -132,6 +135,14 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
         InstalledAppTestUtils.install(context, "com.waze", 1019841, "v3.9.5.4", "362488e7be5ea0689b4e97d989ae1404",
                 "cbbdb8c5dafeccd7dd7b642dde0477d3489e18ac366e3c8473d5c07e5f735a95");
         assertEquals(1, AppProvider.Helper.findInstalledAppsWithKnownVulns(context).size());
+
+        Apk apk = ApkProvider.Helper.findApkFromAnyRepo(context, "io.proto.player", 1110);
+        assertNotNull("We should find this APK", apk);
+        assertEquals("io.proto.player-1.apk", apk.apkName);
+        HashSet<String> requestedPermissions = new HashSet<>(Arrays.asList(apk.requestedPermissions));
+        assertTrue(requestedPermissions.contains(android.Manifest.permission.READ_EXTERNAL_STORAGE));
+        assertTrue(requestedPermissions.contains(android.Manifest.permission.WRITE_EXTERNAL_STORAGE));
+        assertFalse(requestedPermissions.contains(android.Manifest.permission.READ_CALENDAR));
     }
 
     @Test(expected = IndexUpdater.SigningException.class)