Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ensure swapping doesn't get confused by apks in different repos.

Browse Source 
While investigating #1086 which was about swap being busted, I
discovered that we recently introduced a worse bug when working with
multi sig stuff. The swap process, when installing an app (or even when
listening for if a user started installing - before they even did
anything), would ask for an apk from any repo. This is wrong, because we
should only ask for the apks from the swap repo when presented with a
swap dialog.

By fixing this so that it asks for a specific apk, this may also
fix the issue in #1086, because that was about us not asking for enough
info from the database for each Apk which was returned. Now we just
return all columns, because the performance overhead should be minimal,
but it prevents this class of bugs, where we didn't fully populate
our value object. However, I'm not confident that it is fixed, because I
was unable to reproduce it due to the other crash fixed in this change.

Relevant crash:

```
java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String org.fdroid.fdroid.data.Apk.getUrl()' on a null object reference
  at org.fdroid.fdroid.views.swap.SwapAppsView$AppListAdapter$ViewHolder.setApp(SwapAppsView.java:311)
  at org.fdroid.fdroid.views.swap.SwapAppsView$AppListAdapter.bindView(SwapAppsView.java:422)
  at org.fdroid.fdroid.views.swap.SwapAppsView$AppListAdapter.newView(SwapAppsView.java:414)
  at android.support.v4.widget.CursorAdapter.getView(CursorAdapter.java:269)
  at android.widget.AbsListView.obtainView(AbsListView.java:2349)
  at android.widget.ListView.makeAndAddView(ListView.java:1864)
  at android.widget.ListView.fillDown(ListView.java:698)
...
```
This commit is contained in:
Peter Serwylo 2017-07-14 11:18:09 +10:00
parent a71eb243fa
commit 2d377453d9
7 changed files with 102 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java
View File

@ -132,13 +132,15 @@ public class ApkProvider extends FDroidProvider {
return cursorToList(cursor);
}
public static Apk get(Context context, Uri uri) {
return get(context, uri, Cols.ALL);
@NonNull
public static List<Apk> findAppVersionsByRepo(Context context, App app, Repo repo) {
ContentResolver resolver = context.getContentResolver();
final Uri uri = getRepoUri(repo.getId(), app.packageName);
Cursor cursor = resolver.query(uri, Cols.ALL, null, null, null);
return cursorToList(cursor);
}
public static Apk get(Context context, Uri uri, String[] fields) {
ContentResolver resolver = context.getContentResolver();
Cursor cursor = resolver.query(uri, fields, null, null, null);
private static Apk cursorToApk(Cursor cursor) {
Apk apk = null;
if (cursor != null) {
if (cursor.getCount() > 0) {
@ -150,6 +152,17 @@ public class ApkProvider extends FDroidProvider {
return apk;
}
public static Apk get(Context context, Uri uri) {
return get(context, uri, Cols.ALL);
}
@Nullable
public static Apk get(Context context, Uri uri, String[] fields) {
ContentResolver resolver = context.getContentResolver();
Cursor cursor = resolver.query(uri, fields, null, null, null);
return cursorToApk(cursor);
}
@NonNull
public static List<Apk> findApksByHash(Context context, String apkHash) {
ContentResolver resolver = context.getContentResolver();
@ -167,10 +180,12 @@ public class ApkProvider extends FDroidProvider {
private static final int CODE_APK_ROW_ID = CODE_APKS + 1;
static final int CODE_APK_FROM_ANY_REPO = CODE_APK_ROW_ID + 1;
static final int CODE_APK_FROM_REPO = CODE_APK_FROM_ANY_REPO + 1;
private static final int CODE_REPO_APP = CODE_APK_FROM_REPO + 1;
private static final String PROVIDER_NAME = "ApkProvider";
protected static final String PATH_APK_FROM_ANY_REPO = "apk-any-repo";
protected static final String PATH_APK_FROM_REPO = "apk-from-repo";
protected static final String PATH_REPO_APP = "repo-app";
private static final String PATH_APKS = "apks";
private static final String PATH_APP = "app";
private static final String PATH_REPO = "repo";
@ -187,6 +202,7 @@ public class ApkProvider extends FDroidProvider {
PACKAGE_FIELDS.put(Cols.Package.PACKAGE_NAME, PackageTable.Cols.PACKAGE_NAME);
MATCHER.addURI(getAuthority(), PATH_REPO + "/#", CODE_REPO);
MATCHER.addURI(getAuthority(), PATH_REPO_APP + "/#/*", CODE_REPO_APP);
MATCHER.addURI(getAuthority(), PATH_APK_FROM_ANY_REPO + "/#/*/*", CODE_APK_FROM_ANY_REPO);
MATCHER.addURI(getAuthority(), PATH_APK_FROM_ANY_REPO + "/#/*", CODE_APK_FROM_ANY_REPO);
MATCHER.addURI(getAuthority(), PATH_APK_FROM_REPO + "/#/#", CODE_APK_FROM_REPO);
@ -227,6 +243,15 @@ public class ApkProvider extends FDroidProvider {
.build();
}
public static Uri getRepoUri(long repoId, String packageName) {
return getContentUri()
.buildUpon()
.appendPath(PATH_REPO_APP)
.appendPath(Long.toString(repoId))
.appendPath(packageName)
.build();
}
public static Uri getApkFromAnyRepoUri(Apk apk) {
return getApkFromAnyRepoUri(apk.packageName, apk.versionCode, null);
}
@ -427,6 +452,13 @@ public class ApkProvider extends FDroidProvider {
QuerySelection query = new QuerySelection(selection, selectionArgs);
switch (MATCHER.match(uri)) {
case CODE_REPO_APP:
List<String> uriSegments = uri.getPathSegments();
Long repoId = Long.parseLong(uriSegments.get(1));
String packageName = uriSegments.get(2);
query = query.add(queryRepo(repoId)).add(queryPackage(packageName));
break;
case CODE_LIST:
break;

29
app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java
View File

@ -53,6 +53,7 @@ import org.fdroid.fdroid.localrepo.SwapService;
import org.fdroid.fdroid.net.Downloader;
import org.fdroid.fdroid.net.DownloaderService;
import java.util.List;
import java.util.Timer;
import java.util.TimerTask;
@ -233,8 +234,13 @@ public class SwapAppsView extends ListView implements
private class ViewHolder {
private final LocalBroadcastManager localBroadcastManager;
@Nullable
private App app;
@Nullable
private Apk apk;
ProgressBar progressView;
TextView nameView;
ImageView iconView;
@ -305,14 +311,19 @@ public class SwapAppsView extends ListView implements
if (this.app == null || !this.app.packageName.equals(app.packageName)) {
this.app = app;
Context context = getContext();
Apk apk = ApkProvider.Helper.findApkFromAnyRepo(context,
app.packageName, app.suggestedVersionCode);
String urlString = apk.getUrl();
List<Apk> availableApks = ApkProvider.Helper.findAppVersionsByRepo(getActivity(), app, repo);
if (availableApks.size() > 0) {
// Swap repos only add one version of an app, so we will just ask for the first apk.
this.apk = availableApks.get(0);
}
// TODO unregister receivers? or will they just die with this instance
localBroadcastManager.registerReceiver(downloadReceiver,
DownloaderService.getIntentFilter(urlString));
if (apk != null) {
String urlString = apk.getUrl();
// TODO unregister receivers? or will they just die with this instance
IntentFilter downloadFilter = DownloaderService.getIntentFilter(urlString);
localBroadcastManager.registerReceiver(downloadReceiver, downloadFilter);
}
// NOTE: Instead of continually unregistering and re-registering the observer
// (with a different URI), this could equally be done by only having one
@ -364,8 +375,8 @@ public class SwapAppsView extends ListView implements
OnClickListener installListener = new OnClickListener() {
@Override
public void onClick(View v) {
if (app.hasUpdates() || app.compatible) {
getActivity().install(app);
if (apk != null && (app.hasUpdates() || app.compatible)) {
getActivity().install(app, apk);
showProgress();
}
}

4
app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java
View File

@ -39,7 +39,6 @@ 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.ApkProvider;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.NewRepoConfig;
import org.fdroid.fdroid.installer.InstallManagerService;
@ -766,8 +765,7 @@ public class SwapWorkflowActivity extends AppCompatActivity {
}
}
public void install(@NonNull final App app) {
final Apk apk = ApkProvider.Helper.findApkFromAnyRepo(this, app.packageName, app.suggestedVersionCode);
public void install(@NonNull final App app, @NonNull final Apk apk) {
Uri downloadUri = Uri.parse(apk.getUrl());
localBroadcastManager.registerReceiver(installReceiver,
Installer.getInstallIntentFilter(downloadUri));

7
app/src/test/java/org/fdroid/fdroid/Assert.java
View File

@ -9,6 +9,7 @@ import org.fdroid.fdroid.data.ApkProvider;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.AppProvider;
import org.fdroid.fdroid.data.InstalledAppProvider;
import org.fdroid.fdroid.data.Repo;
import org.fdroid.fdroid.data.Schema.ApkTable;
import org.fdroid.fdroid.data.Schema.AppMetadataTable;
import org.fdroid.fdroid.data.Schema.InstalledAppTable;
@ -181,6 +182,12 @@ public class Assert {
return insertApp(context, packageName, name, new ContentValues());
}
public static App insertApp(Context context, String packageName, String name, Repo repo) {
ContentValues values = new ContentValues();
values.put(AppMetadataTable.Cols.REPO_ID, repo.getId());
return insertApp(context, packageName, name, values);
}
public static App insertApp(Context context, String packageName, String name, ContentValues additionalValues) {
ContentValues values = new ContentValues();

11
app/src/test/java/org/fdroid/fdroid/TestUtils.java
View File

@ -6,7 +6,10 @@ import android.content.ContentValues;
import android.content.Context;
import android.content.ContextWrapper;
import android.content.pm.ProviderInfo;
import android.net.Uri;
import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.ApkProvider;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.AppProvider;
import org.fdroid.fdroid.data.Repo;
@ -81,10 +84,14 @@ public class TestUtils {
assertEquals(message, formatSigForDebugging(expected), formatSigForDebugging(actual));
}
public static void insertApk(Context context, App app, int versionCode, String signature) {
public static Apk insertApk(Context context, App app, int versionCode, String signature) {
ContentValues values = new ContentValues();
values.put(Schema.ApkTable.Cols.SIGNATURE, signature);
Assert.insertApk(context, app, versionCode, values);
long repoId = app.repoId > 0 ? app.repoId : 1;
values.put(Schema.ApkTable.Cols.REPO_ID, repoId);
Uri uri = Assert.insertApk(context, app, versionCode, values);
return ApkProvider.Helper.findByUri(context, uri, Schema.ApkTable.Cols.ALL);
}
public static App insertApp(Context context, String packageName, String appName, int upstreamVersionCode,

22
app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java
View File

@ -6,6 +6,7 @@ import android.database.Cursor;
import android.net.Uri;
import org.fdroid.fdroid.Assert;
import org.fdroid.fdroid.BuildConfig;
import org.fdroid.fdroid.TestUtils;
import org.fdroid.fdroid.data.Schema.ApkTable.Cols;
import org.fdroid.fdroid.data.Schema.RepoTable;
import org.fdroid.fdroid.mock.MockApk;
@ -271,6 +272,27 @@ public class ApkProviderTest extends FDroidProviderTest {
assertBelongsToApp(thingoApks, "com.apk.thingo");
}
@Test
public void findApksForAppInSpecificRepo() {
Repo fdroidRepo = RepoProvider.Helper.findByAddress(context, "https://f-droid.org/repo");
Repo swapRepo = RepoProviderTest.insertRepo(context, "http://192.168.1.3/fdroid/repo", "", "22", "", true);
App officialFDroid = insertApp(context, "org.fdroid.fdroid", "F-Droid (Official)", fdroidRepo);
TestUtils.insertApk(context, officialFDroid, 4, TestUtils.FDROID_SIG);
TestUtils.insertApk(context, officialFDroid, 5, TestUtils.FDROID_SIG);
App debugSwapFDroid = insertApp(context, "org.fdroid.fdroid", "F-Droid (Debug)", swapRepo);
TestUtils.insertApk(context, debugSwapFDroid, 6, TestUtils.THIRD_PARTY_SIG);
List<Apk> foundOfficialApks = ApkProvider.Helper.findAppVersionsByRepo(context, officialFDroid, fdroidRepo);
assertEquals(2, foundOfficialApks.size());
List<Apk> debugSwapApks = ApkProvider.Helper.findAppVersionsByRepo(context, officialFDroid, swapRepo);
assertEquals(1, debugSwapApks.size());
assertEquals(debugSwapFDroid.getId(), debugSwapApks.get(0).appId);
assertEquals(6, debugSwapApks.get(0).versionCode);
}
@Test
public void testUpdate() {

6
app/src/test/java/org/fdroid/fdroid/data/RepoProviderTest.java
View File

@ -256,11 +256,17 @@ public class RepoProviderTest extends FDroidProviderTest {
public static Repo insertRepo(Context context, String address, String description,
String fingerprint, @Nullable String name) {
return insertRepo(context, address, description, fingerprint, name, false);
}
public static Repo insertRepo(Context context, String address, String description,
String fingerprint, @Nullable String name, boolean isSwap) {
ContentValues values = new ContentValues();
values.put(RepoTable.Cols.ADDRESS, address);
values.put(RepoTable.Cols.DESCRIPTION, description);
values.put(RepoTable.Cols.FINGERPRINT, fingerprint);
values.put(RepoTable.Cols.NAME, name);
values.put(RepoTable.Cols.IS_SWAP, isSwap);
RepoProvider.Helper.insert(context, values);
return RepoProvider.Helper.findByAddress(context, address);