diff --git a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java index 2b59e139d..f132c6c4d 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java @@ -9,6 +9,10 @@ import android.net.Uri; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.util.Log; + +import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.data.Schema.AntiFeatureTable; +import org.fdroid.fdroid.data.Schema.ApkAntiFeatureJoinTable; import org.fdroid.fdroid.data.Schema.ApkTable; import org.fdroid.fdroid.data.Schema.ApkTable.Cols; import org.fdroid.fdroid.data.Schema.AppMetadataTable; @@ -17,8 +21,10 @@ import org.fdroid.fdroid.data.Schema.RepoTable; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; @SuppressWarnings("LineLength") public class ApkProvider extends FDroidProvider { @@ -266,6 +272,10 @@ public class ApkProvider extends FDroidProvider { return ApkTable.NAME; } + protected String getApkAntiFeatureJoinTableName() { + return ApkAntiFeatureJoinTable.NAME; + } + protected String getAppTableName() { return AppMetadataTable.NAME; } @@ -283,6 +293,18 @@ public class ApkProvider extends FDroidProvider { private class Query extends QueryBuilder { private boolean repoTableRequired; + private boolean antiFeaturesRequested; + + /** + * If the query includes anti features, then we group by apk id. This is because joining onto the anti-features + * table will result in multiple result rows for each apk (potentially), so we will GROUP_CONCAT each of the + * anti features into a single comma separated list for each apk. If we are _not_ including anti features, then + * don't group by apk, because when doing a COUNT(*) this will result in the wrong result. + */ + @Override + protected String groupBy() { + return antiFeaturesRequested ? "apk." + Cols.ROW_ID : null; + } @Override protected String getRequiredTables() { @@ -301,6 +323,9 @@ public class ApkProvider extends FDroidProvider { addPackageField(PACKAGE_FIELDS.get(field), field); } else if (REPO_FIELDS.containsKey(field)) { addRepoField(REPO_FIELDS.get(field), field); + } else if (Cols.AntiFeatures.ANTI_FEATURES.equals(field)) { + antiFeaturesRequested = true; + addAntiFeatures(); } else if (field.equals(Cols._ID)) { appendField("rowid", "apk", "_id"); } else if (field.equals(Cols._COUNT)) { @@ -324,6 +349,18 @@ public class ApkProvider extends FDroidProvider { appendField(field, "repo", alias); } + private void addAntiFeatures() { + String apkAntiFeature = "apkAntiFeatureJoin"; + String antiFeature = "antiFeature"; + + leftJoin(getApkAntiFeatureJoinTableName(), apkAntiFeature, + "apk." + Cols.ROW_ID + " = " + apkAntiFeature + "." + ApkAntiFeatureJoinTable.Cols.APK_ID); + + leftJoin(AntiFeatureTable.NAME, antiFeature, + apkAntiFeature + "." + ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID + " = " + antiFeature + "." + AntiFeatureTable.Cols.ROW_ID); + + appendField("group_concat(" + antiFeature + "." + AntiFeatureTable.Cols.NAME + ") as " + Cols.AntiFeatures.ANTI_FEATURES); + } } private QuerySelection queryPackage(String packageName) { @@ -508,15 +545,73 @@ public class ApkProvider extends FDroidProvider { @Override public Uri insert(Uri uri, ContentValues values) { + boolean saveAntiFeatures = false; + String[] antiFeatures = null; + if (values.containsKey(Cols.AntiFeatures.ANTI_FEATURES)) { + saveAntiFeatures = true; + String antiFeaturesString = values.getAsString(Cols.AntiFeatures.ANTI_FEATURES); + antiFeatures = Utils.parseCommaSeparatedString(antiFeaturesString); + values.remove(Cols.AntiFeatures.ANTI_FEATURES); + } + removeFieldsFromOtherTables(values); validateFields(Cols.ALL, values); long newId = db().insertOrThrow(getTableName(), null, values); + + if (saveAntiFeatures) { + ensureAntiFeatures(antiFeatures, newId); + } + if (!isApplyingBatch()) { getContext().getContentResolver().notifyChange(uri, null); } return getApkUri(newId); } + protected void ensureAntiFeatures(String[] antiFeatures, long apkId) { + db().delete(getApkAntiFeatureJoinTableName(), ApkAntiFeatureJoinTable.Cols.APK_ID + " = ?", new String[] {Long.toString(apkId)}); + if (antiFeatures != null) { + Set antiFeatureSet = new HashSet<>(); + for (String antiFeatureName : antiFeatures) { + + // There is nothing stopping a server repeating a category name in the metadata of + // an app. In order to prevent unique constraint violations, only insert once into + // the join table. + if (antiFeatureSet.contains(antiFeatureName)) { + continue; + } + + antiFeatureSet.add(antiFeatureName); + + long antiFeatureId = ensureAntiFeature(antiFeatureName); + ContentValues categoryValues = new ContentValues(2); + categoryValues.put(ApkAntiFeatureJoinTable.Cols.APK_ID, apkId); + categoryValues.put(ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID, antiFeatureId); + db().insert(getApkAntiFeatureJoinTableName(), null, categoryValues); + } + } + } + + protected long ensureAntiFeature(String antiFeatureName) { + long antiFeatureId = 0; + Cursor cursor = db().query(AntiFeatureTable.NAME, new String[] {AntiFeatureTable.Cols.ROW_ID}, AntiFeatureTable.Cols.NAME + " = ?", new String[]{antiFeatureName}, null, null, null); + if (cursor != null) { + if (cursor.getCount() > 0) { + cursor.moveToFirst(); + antiFeatureId = cursor.getLong(0); + } + cursor.close(); + } + + if (antiFeatureId <= 0) { + ContentValues values = new ContentValues(1); + values.put(AntiFeatureTable.Cols.NAME, antiFeatureName); + antiFeatureId = db().insert(AntiFeatureTable.NAME, null, values); + } + + return antiFeatureId; + } + @Override public int delete(Uri uri, String where, String[] whereArgs) { @@ -549,6 +644,15 @@ public class ApkProvider extends FDroidProvider { throw new UnsupportedOperationException("Cannot update anything other than a single apk."); } + boolean saveAntiFeatures = false; + String[] antiFeatures = null; + if (values.containsKey(Cols.AntiFeatures.ANTI_FEATURES)) { + saveAntiFeatures = true; + String antiFeaturesString = values.getAsString(Cols.AntiFeatures.ANTI_FEATURES); + antiFeatures = Utils.parseCommaSeparatedString(antiFeaturesString); + values.remove(Cols.AntiFeatures.ANTI_FEATURES); + } + validateFields(Cols.ALL, values); removeFieldsFromOtherTables(values); @@ -556,6 +660,19 @@ public class ApkProvider extends FDroidProvider { query = query.add(querySingleWithAppId(uri)); int numRows = db().update(getTableName(), values, query.getSelection(), query.getArgs()); + + if (saveAntiFeatures) { + // Get the database ID of the row we just updated, so that we can join relevant anti features to it. + Cursor result = db().query(getTableName(), new String[]{Cols.ROW_ID}, + query.getSelection(), query.getArgs(), null, null, null); + if (result != null) { + result.moveToFirst(); + long apkId = result.getLong(0); + ensureAntiFeatures(antiFeatures, apkId); + result.close(); + } + } + if (!isApplyingBatch()) { getContext().getContentResolver().notifyChange(uri, null); } diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java index 3e487f594..139869e5c 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -200,7 +200,7 @@ class DBHelper extends SQLiteOpenHelper { + AntiFeatureTable.Cols.NAME + " TEXT NOT NULL " + " );"; - private static final String CREATE_TABLE_APK_ANTI_FEATURE_JOIN = "CREATE TABLE " + ApkAntiFeatureJoinTable.NAME + static final String CREATE_TABLE_APK_ANTI_FEATURE_JOIN = "CREATE TABLE " + ApkAntiFeatureJoinTable.NAME + " ( " + ApkAntiFeatureJoinTable.Cols.APK_ID + " INT NOT NULL, " + ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID + " INT NOT NULL, " diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java index 5a45c827b..3c00ea546 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java @@ -36,6 +36,11 @@ public class TempApkProvider extends ApkProvider { return TABLE_TEMP_APK; } + @Override + protected String getApkAntiFeatureJoinTableName() { + return TempAppProvider.TABLE_TEMP_APK_ANTI_FEATURE_JOIN; + } + @Override protected String getAppTableName() { return TempAppProvider.TABLE_TEMP_APP; @@ -93,11 +98,23 @@ public class TempApkProvider extends ApkProvider { final SQLiteDatabase db = db(); final String memoryDbName = TempAppProvider.DB; db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(ApkTable.NAME, memoryDbName + "." + getTableName())); + db.execSQL(DBHelper.CREATE_TABLE_APK_ANTI_FEATURE_JOIN.replaceFirst(Schema.ApkAntiFeatureJoinTable.NAME, memoryDbName + "." + getApkAntiFeatureJoinTableName())); String where = ApkTable.NAME + "." + Cols.REPO_ID + " != ?"; String[] whereArgs = new String[]{Long.toString(repoIdBeingUpdated)}; db.execSQL(TempAppProvider.copyData(Cols.ALL_COLS, ApkTable.NAME, memoryDbName + "." + getTableName(), where), whereArgs); + String antiFeaturesWhere = + Schema.ApkAntiFeatureJoinTable.NAME + "." + Schema.ApkAntiFeatureJoinTable.Cols.APK_ID + " IN " + + "(SELECT innerApk." + Cols.ROW_ID + " FROM " + ApkTable.NAME + " AS innerApk " + + "WHERE innerApk." + Cols.REPO_ID + " != ?)"; + + db.execSQL(TempAppProvider.copyData( + Schema.ApkAntiFeatureJoinTable.Cols.ALL_COLS, + Schema.ApkAntiFeatureJoinTable.NAME, + memoryDbName + "." + getApkAntiFeatureJoinTableName(), + antiFeaturesWhere), whereArgs); + db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + Cols.APP_ID + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + Cols.IS_COMPATIBLE + ");"); } diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java index 1f5ebb3a4..7c0843be1 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -30,6 +30,7 @@ public class TempAppProvider extends AppProvider { private static final String PROVIDER_NAME = "TempAppProvider"; static final String TABLE_TEMP_APP = "temp_" + AppMetadataTable.NAME; + static final String TABLE_TEMP_APK_ANTI_FEATURE_JOIN = "temp_" + Schema.ApkAntiFeatureJoinTable.NAME; static final String TABLE_TEMP_CAT_JOIN = "temp_" + CatJoinTable.NAME; private static final String PATH_INIT = "init"; @@ -218,6 +219,7 @@ public class TempAppProvider extends AppProvider { final String tempApp = DB + "." + TABLE_TEMP_APP; final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK; final String tempCatJoin = DB + "." + TABLE_TEMP_CAT_JOIN; + final String tempAntiFeatureJoin = DB + "." + TABLE_TEMP_APK_ANTI_FEATURE_JOIN; final String[] repoArgs = new String[]{Long.toString(repoIdToCommit)}; @@ -230,6 +232,16 @@ public class TempAppProvider extends AppProvider { db.execSQL("DELETE FROM " + CatJoinTable.NAME + " WHERE " + getCatRepoWhere(CatJoinTable.NAME), repoArgs); db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, tempCatJoin, CatJoinTable.NAME, getCatRepoWhere(tempCatJoin)), repoArgs); + db.execSQL( + "DELETE FROM " + Schema.ApkAntiFeatureJoinTable.NAME + " " + + "WHERE " + getAntiFeatureRepoWhere(Schema.ApkAntiFeatureJoinTable.NAME), repoArgs); + + db.execSQL(copyData( + Schema.ApkAntiFeatureJoinTable.Cols.ALL_COLS, + tempAntiFeatureJoin, + Schema.ApkAntiFeatureJoinTable.NAME, + getAntiFeatureRepoWhere(tempAntiFeatureJoin))); + db.setTransactionSuccessful(); getContext().getContentResolver().notifyChange(AppProvider.getContentUri(), null); @@ -250,4 +262,13 @@ public class TempAppProvider extends AppProvider { return CatJoinTable.Cols.ROW_ID + " IN (" + catRepoSubquery + ")"; } + + private String getAntiFeatureRepoWhere(String antiFeatureTable) { + String subquery = + "SELECT innerApk." + ApkTable.Cols.ROW_ID + " " + + "FROM " + ApkTable.NAME + " AS innerApk " + + "WHERE innerApk." + ApkTable.Cols.REPO_ID + " != ?"; + + return antiFeatureTable + "." + Schema.ApkAntiFeatureJoinTable.Cols.APK_ID + " IN (" + subquery + ")"; + } } diff --git a/app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java b/app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java new file mode 100644 index 000000000..a751c03c6 --- /dev/null +++ b/app/src/test/java/org/fdroid/fdroid/AntiFeaturesTest.java @@ -0,0 +1,67 @@ +package org.fdroid.fdroid; + +import android.app.Application; +import android.content.ContentValues; + +import org.fdroid.fdroid.data.Apk; +import org.fdroid.fdroid.data.ApkProvider; +import org.fdroid.fdroid.data.App; +import org.fdroid.fdroid.data.FDroidProviderTest; +import org.fdroid.fdroid.data.Schema; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import java.io.IOException; +import java.util.List; + +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; + +@Config(constants = BuildConfig.class, application = Application.class, sdk = 24) +@RunWith(RobolectricTestRunner.class) +public class AntiFeaturesTest extends FDroidProviderTest { + + @Test + public void testPerApkAntiFeatures() throws IOException, RepoUpdater.UpdateException { + ContentValues vulnValues = new ContentValues(1); + vulnValues.put(Schema.ApkTable.Cols.AntiFeatures.ANTI_FEATURES, "KnownVuln,ContainsGreenButtons"); + + App vulnAtV2 = Assert.insertApp(context, "com.vuln", "Fixed it"); + Assert.insertApk(context, vulnAtV2, 1); + Assert.insertApk(context, vulnAtV2, 2, vulnValues); + Assert.insertApk(context, vulnAtV2, 3); + + App notVuln = Assert.insertApp(context, "com.not-vuln", "It's Fine"); + Assert.insertApk(context, notVuln, 5); + Assert.insertApk(context, notVuln, 10); + Assert.insertApk(context, notVuln, 15); + + App allVuln = Assert.insertApp(context, "com.all-vuln", "Oops"); + Assert.insertApk(context, allVuln, 100, vulnValues); + Assert.insertApk(context, allVuln, 101, vulnValues); + Assert.insertApk(context, allVuln, 105, vulnValues); + + List notVulnApks = ApkProvider.Helper.findByPackageName(context, notVuln.packageName); + assertEquals(3, notVulnApks.size()); + + List allVulnApks = ApkProvider.Helper.findByPackageName(context, allVuln.packageName); + assertEquals(3, allVulnApks.size()); + for (Apk apk : allVulnApks) { + assertArrayEquals(new String[]{"KnownVuln", "ContainsGreenButtons"}, apk.antiFeatures); + } + + List vulnAtV2Apks = ApkProvider.Helper.findByPackageName(context, vulnAtV2.packageName); + assertEquals(3, vulnAtV2Apks.size()); + for (Apk apk : vulnAtV2Apks) { + if (apk.versionCode == 2) { + assertArrayEquals(new String[]{"KnownVuln", "ContainsGreenButtons"}, apk.antiFeatures); + } else { + assertNull(apk.antiFeatures); + } + } + } + +} diff --git a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java index 3e1057e58..bd0f2b86e 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -22,6 +22,7 @@ import java.util.List; import static org.fdroid.fdroid.Assert.assertCantDelete; import static org.fdroid.fdroid.Assert.assertResultCount; import static org.fdroid.fdroid.Assert.insertApp; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -309,10 +310,12 @@ public class ApkProviderTest extends FDroidProviderTest { assertEquals("com.example", apk.packageName); assertEquals(10, apk.versionCode); + assertNull(apk.antiFeatures); assertNull(apk.features); assertNull(apk.added); assertNull(apk.hashType); + apk.antiFeatures = new String[] {"KnownVuln", "Other anti feature"}; apk.features = new String[] {"one", "two", "three" }; long dateTimestamp = System.currentTimeMillis(); apk.added = new Date(dateTimestamp); @@ -335,14 +338,8 @@ public class ApkProviderTest extends FDroidProviderTest { assertEquals("com.example", updatedApk.packageName); assertEquals(10, updatedApk.versionCode); - assertNotNull(updatedApk.features); - assertNotNull(updatedApk.added); - assertNotNull(updatedApk.hashType); - - assertEquals(3, updatedApk.features.length); - assertEquals("one", updatedApk.features[0]); - assertEquals("two", updatedApk.features[1]); - assertEquals("three", updatedApk.features[2]); + assertArrayEquals(new String[]{"KnownVuln", "Other anti feature"}, updatedApk.antiFeatures); + assertArrayEquals(new String[]{"one", "two", "three"}, updatedApk.features); assertEquals(new Date(dateTimestamp).getYear(), updatedApk.added.getYear()); assertEquals(new Date(dateTimestamp).getMonth(), updatedApk.added.getMonth()); assertEquals(new Date(dateTimestamp).getDay(), updatedApk.added.getDay()); diff --git a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java index c070fea20..100c4257c 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java @@ -91,7 +91,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest { updater.processIndexV1(indexInputStream, indexEntry, "fakeEtag"); IOUtils.closeQuietly(indexInputStream); List apps = AppProvider.Helper.all(context.getContentResolver()); - assertEquals("53 apps present", 53, apps.size()); + assertEquals("63 apps present", 63, apps.size()); String[] packages = { "fake.app.one", @@ -110,7 +110,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest { repos = RepoProvider.Helper.all(context); assertEquals("One repo", 1, repos.size()); Repo repoFromDb = repos.get(0); - assertEquals("repo.timestamp should be set", 1481222111, repoFromDb.timestamp); + assertEquals("repo.timestamp should be set", 1497639511, repoFromDb.timestamp); assertEquals("repo.address should be the same", repo.address, repoFromDb.address); assertEquals("repo.name should be set", "non-public test repo", repoFromDb.name); assertEquals("repo.maxage should be set", 0, repoFromDb.maxage); diff --git a/app/src/test/resources/testy.at.or.at_index-v1.jar b/app/src/test/resources/testy.at.or.at_index-v1.jar index 11cfd5f2c..a2813137e 100644 Binary files a/app/src/test/resources/testy.at.or.at_index-v1.jar and b/app/src/test/resources/testy.at.or.at_index-v1.jar differ