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

Encapsulate functions which can produce invalid SQL.

Browse Source 
Now the other content provider functions which can result in broken SQL
due to the number of arguments is private, and can only be accessed
from a public helper method which ensures that limit is never hit.
This commit is contained in:
Peter Serwylo 2015-04-21 22:27:58 +10:00
parent 898f331bfd
commit 424839c793
3 changed files with 50 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
F-Droid/src/org/fdroid/fdroid/UpdateService.java
View File

@ -604,23 +604,6 @@ public class UpdateService extends IntentService implements ProgressListener {
return knownIds;
}
/**
* If you call this with too many apks, then it will likely hit limit of
* parameters allowed for sqlite3 query. Rather, you should use
* {@link org.fdroid.fdroid.UpdateService#getKnownApks(java.util.List)}
* instead, which will only call this with the right number of apks at
* a time.
* @see org.fdroid.fdroid.UpdateService#getKnownAppIds(java.util.List)
*/
private List<Apk> getKnownApksFromProvider(List<Apk> apks) {
final String[] fields = {
ApkProvider.DataColumns.APK_ID,
ApkProvider.DataColumns.VERSION,
ApkProvider.DataColumns.VERSION_CODE
};
return ApkProvider.Helper.knownApks(this, apks, fields);
}
private void updateOrInsertApps(List<App> appsToUpdate, int totalUpdateCount, int currentCount) {
List<ContentProviderOperation> operations = new ArrayList<>();
@ -662,17 +645,12 @@ public class UpdateService extends IntentService implements ProgressListener {
* Return list of apps from the "apks" argument which are already in the database.
*/
private List<Apk> getKnownApks(List<Apk> apks) {
List<Apk> knownApks = new ArrayList<>();
if (apks.size() > ApkProvider.MAX_APKS_TO_QUERY) {
int middle = apks.size() / 2;
List<Apk> apks1 = apks.subList(0, middle);
List<Apk> apks2 = apks.subList(middle, apks.size());
knownApks.addAll(getKnownApks(apks1));
knownApks.addAll(getKnownApks(apks2));
} else {
knownApks.addAll(getKnownApksFromProvider(apks));
}
return knownApks;
final String[] fields = {
ApkProvider.DataColumns.APK_ID,
ApkProvider.DataColumns.VERSION,
ApkProvider.DataColumns.VERSION_CODE
};
return ApkProvider.Helper.knownApks(this, apks, fields);
}
private void updateOrInsertApks(List<Apk> apksToUpdate, int totalApksAppsCount, int currentCount) {

29
F-Droid/src/org/fdroid/fdroid/data/ApkProvider.java
View File

@ -24,7 +24,7 @@ public class ApkProvider extends FDroidProvider {
* we may want to add additional constraints, so we give our self some
* room by saying only 450 apks can be queried at once.
*/
public static final int MAX_APKS_TO_QUERY = 450;
protected static final int MAX_APKS_TO_QUERY = 450;
public static final class Helper {
@ -118,11 +118,26 @@ public class ApkProvider extends FDroidProvider {
* Returns apks in the database, which have the same id and version as
* one of the apks in the "apks" argument.
*/
public static List<Apk> knownApks(Context context,
List<Apk> apks, String[] fields) {
public static List<Apk> knownApks(Context context, List<Apk> apks, String[] fields) {
if (apks.size() == 0) {
return new ArrayList<>();
}
List<Apk> knownApks = new ArrayList<>();
if (apks.size() > ApkProvider.MAX_APKS_TO_QUERY) {
int middle = apks.size() / 2;
List<Apk> apks1 = apks.subList(0, middle);
List<Apk> apks2 = apks.subList(middle, apks.size());
knownApks.addAll(knownApks(context, apks1, fields));
knownApks.addAll(knownApks(context,apks2, fields));
} else {
knownApks.addAll(knownApksSafe(context,apks, fields));
}
return knownApks;
}
private static List<Apk> knownApksSafe(final Context context, final List<Apk> apks, final String[] fields) {
ContentResolver resolver = context.getContentResolver();
Uri uri = getContentUri(apks);
Cursor cursor = resolver.query(uri, fields, null, null, null);
@ -250,7 +265,13 @@ public class ApkProvider extends FDroidProvider {
.build();
}
public static Uri getContentUri(List<Apk> apks) {
/**
* Intentionally left protected because it will break if apks is larger than
* {@link org.fdroid.fdroid.data.ApkProvider#MAX_APKS_TO_QUERY}. Instead of using
* this directly, think about using
* {@link org.fdroid.fdroid.data.ApkProvider.Helper#knownApks(android.content.Context, java.util.List, String[])}
*/
protected static Uri getContentUri(List<Apk> apks) {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < apks.size(); i++) {
if (i != 0) {

25
F-Droid/test/src/org/fdroid/fdroid/ApkProviderTest.java
View File

@ -16,6 +16,19 @@ import java.util.List;
public class ApkProviderTest extends BaseApkProviderTest {
/**
* I want to test the protected {@link org.fdroid.fdroid.data.ApkProvider#getContentUri(java.util.List)}
* method, but don't want to make it public. This exposes it.
*/
private static class PublicApkProvider extends ApkProvider {
public static final int MAX_APKS_TO_QUERY = ApkProvider.MAX_APKS_TO_QUERY;
public static Uri getContentUri(List<Apk> apks) {
return ApkProvider.getContentUri(apks);
}
}
public void testUris() {
assertInvalidUri(ApkProvider.getAuthority());
assertInvalidUri(RepoProvider.getContentUri());
@ -29,15 +42,15 @@ public class ApkProviderTest extends BaseApkProviderTest {
assertValidUri(ApkProvider.getAppUri("org.fdroid.fdroid"));
assertValidUri(ApkProvider.getContentUri(new MockApk("org.fdroid.fdroid", 100)));
assertValidUri(ApkProvider.getContentUri());
assertValidUri(ApkProvider.getContentUri(apks));
assertValidUri(PublicApkProvider.getContentUri(apks));
assertValidUri(ApkProvider.getContentUri("org.fdroid.fdroid", 100));
assertValidUri(ApkProvider.getRepoUri(1000));
List<Apk> manyApks = new ArrayList<Apk>(ApkProvider.MAX_APKS_TO_QUERY - 5);
for (int i = 0; i < ApkProvider.MAX_APKS_TO_QUERY - 1; i ++) {
List<Apk> manyApks = new ArrayList<Apk>(PublicApkProvider.MAX_APKS_TO_QUERY - 5);
for (int i = 0; i < PublicApkProvider.MAX_APKS_TO_QUERY - 1; i ++) {
manyApks.add(new MockApk("com.example." + i, i));
}
assertValidUri(ApkProvider.getContentUri(manyApks));
assertValidUri(PublicApkProvider.getContentUri(manyApks));
manyApks.add(new MockApk("org.fdroid.fdroid.1", 1));
manyApks.add(new MockApk("org.fdroid.fdroid.2", 2));
@ -46,7 +59,7 @@ public class ApkProviderTest extends BaseApkProviderTest {
// throw an UnsupportedOperationException. However it
// is still not okay (we run out of bindable parameters
// in the sqlite query.
assertValidUri(ApkProvider.getContentUri(manyApks));
assertValidUri(PublicApkProvider.getContentUri(manyApks));
fail();
} catch (IllegalArgumentException e) {
// This is the expected error behaviour.
@ -93,7 +106,7 @@ public class ApkProviderTest extends BaseApkProviderTest {
assertCantUpdate(ApkProvider.getContentUri());
assertCantUpdate(ApkProvider.getAppUri("org.fdroid.fdroid"));
assertCantUpdate(ApkProvider.getRepoUri(1));
assertCantUpdate(ApkProvider.getContentUri(apks));
assertCantUpdate(PublicApkProvider.getContentUri(apks));
assertCantUpdate(Uri.withAppendedPath(ApkProvider.getContentUri(), "some-random-path"));
// The only valid ones are: