proper null checks and closing of cursors everywhere

6 files changed, 189 insertions, 148 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/FileHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/FileHelper.java
index f22fcd4b8..d7d73cf3d 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/FileHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/FileHelper.java
@@ -112,16 +112,18 @@ public class FileHelper {
 
         if ("content".equalsIgnoreCase(uri.getScheme())) {
             String[] projection = {"_data"};
-            Cursor cursor = null;
-
+            Cursor cursor = context.getContentResolver().query(uri, projection, null, null, null);
             try {
-                cursor = context.getContentResolver().query(uri, projection, null, null, null);
-                int columnIndex = cursor.getColumnIndexOrThrow("_data");
-                if (cursor.moveToFirst()) {
+                if (cursor != null && cursor.moveToFirst()) {
+                    int columnIndex = cursor.getColumnIndexOrThrow("_data");
                     return cursor.getString(columnIndex);
                 }
             } catch (Exception e) {
                 // Eat it
+            } finally {
+                if (cursor != null) {
+                    cursor.close();
+                }
             }
         } else if ("file".equalsIgnoreCase(uri.getScheme())) {
             return uri.getPath();
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java
index 061f91176..68726d3e0 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java
@@ -255,53 +255,60 @@ public class KeychainDatabase extends SQLiteOpenHelper {
             }
         }.getReadableDatabase();
 
-        Cursor c = null;
+        Cursor cursor = null;
         try {
             // we insert in two steps: first, all public keys that have secret keys
-            c = db.rawQuery("SELECT key_ring_data FROM key_rings WHERE type = 1 OR EXISTS ("
+            cursor = db.rawQuery("SELECT key_ring_data FROM key_rings WHERE type = 1 OR EXISTS ("
                     + " SELECT 1 FROM key_rings d2 WHERE key_rings.master_key_id = d2.master_key_id"
                     + " AND d2.type = 1) ORDER BY type ASC", null);
-            Log.d(Constants.TAG, "Importing " + c.getCount() + " secret keyrings from apg.db...");
-            for (int i = 0; i < c.getCount(); i++) {
-                c.moveToPosition(i);
-                byte[] data = c.getBlob(0);
-                PGPKeyRing ring = PgpConversionHelper.BytesToPGPKeyRing(data);
-                ProviderHelper providerHelper = new ProviderHelper(context);
-                if (ring instanceof PGPPublicKeyRing)
-                    providerHelper.saveKeyRing((PGPPublicKeyRing) ring);
-                else if (ring instanceof PGPSecretKeyRing)
-                    providerHelper.saveKeyRing((PGPSecretKeyRing) ring);
-                else {
-                    Log.e(Constants.TAG, "Unknown blob data type!");
+            Log.d(Constants.TAG, "Importing " + cursor.getCount() + " secret keyrings from apg.db...");
+            if (cursor != null) {
+                for (int i = 0; i < cursor.getCount(); i++) {
+                    cursor.moveToPosition(i);
+                    byte[] data = cursor.getBlob(0);
+                    PGPKeyRing ring = PgpConversionHelper.BytesToPGPKeyRing(data);
+                    ProviderHelper providerHelper = new ProviderHelper(context);
+                    if (ring instanceof PGPPublicKeyRing)
+                        providerHelper.saveKeyRing((PGPPublicKeyRing) ring);
+                    else if (ring instanceof PGPSecretKeyRing)
+                        providerHelper.saveKeyRing((PGPSecretKeyRing) ring);
+                    else {
+                        Log.e(Constants.TAG, "Unknown blob data type!");
+                    }
                 }
             }
+            if (cursor != null) {
+                cursor.close();
+            }
 
             // afterwards, insert all keys, starting with public keys that have secret keys, then
             // secret keys, then all others. this order is necessary to ensure all certifications
             // are recognized properly.
-            c = db.rawQuery("SELECT key_ring_data FROM key_rings ORDER BY (type = 0 AND EXISTS ("
+            cursor = db.rawQuery("SELECT key_ring_data FROM key_rings ORDER BY (type = 0 AND EXISTS ("
                     + " SELECT 1 FROM key_rings d2 WHERE key_rings.master_key_id = d2.master_key_id AND"
                     + " d2.type = 1)) DESC, type DESC", null);
             // import from old database
-            Log.d(Constants.TAG, "Importing " + c.getCount() + " keyrings from apg.db...");
-            for (int i = 0; i < c.getCount(); i++) {
-                c.moveToPosition(i);
-                byte[] data = c.getBlob(0);
-                PGPKeyRing ring = PgpConversionHelper.BytesToPGPKeyRing(data);
-                ProviderHelper providerHelper = new ProviderHelper(context);
-                if (ring instanceof PGPPublicKeyRing) {
-                    providerHelper.saveKeyRing((PGPPublicKeyRing) ring);
-                } else if (ring instanceof PGPSecretKeyRing) {
-                    providerHelper.saveKeyRing((PGPSecretKeyRing) ring);
-                } else {
-                    Log.e(Constants.TAG, "Unknown blob data type!");
+            Log.d(Constants.TAG, "Importing " + cursor.getCount() + " keyrings from apg.db...");
+            if (cursor != null) {
+                for (int i = 0; i < cursor.getCount(); i++) {
+                    cursor.moveToPosition(i);
+                    byte[] data = cursor.getBlob(0);
+                    PGPKeyRing ring = PgpConversionHelper.BytesToPGPKeyRing(data);
+                    ProviderHelper providerHelper = new ProviderHelper(context);
+                    if (ring instanceof PGPPublicKeyRing) {
+                        providerHelper.saveKeyRing((PGPPublicKeyRing) ring);
+                    } else if (ring instanceof PGPSecretKeyRing) {
+                        providerHelper.saveKeyRing((PGPSecretKeyRing) ring);
+                    } else {
+                        Log.e(Constants.TAG, "Unknown blob data type!");
+                    }
                 }
             }
         } catch (IOException e) {
             Log.e(Constants.TAG, "Error importing apg.db!", e);
         } finally {
-            if (c != null) {
-                c.close();
+            if (cursor != null) {
+                cursor.close();
             }
             if (db != null) {
                 db.close();
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java
index 1dc822ac2..ec7bf58d9 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java
@@ -541,20 +541,21 @@ public class KeychainProvider extends ContentProvider {
         }
 
         SQLiteDatabase db = getDb().getReadableDatabase();
-        Cursor c = qb.query(db, projection, selection, selectionArgs, groupBy, having, orderBy);
-
-        // Tell the cursor what uri to watch, so it knows when its source data changes
-        c.setNotificationUri(getContext().getContentResolver(), uri);
+        Cursor cursor = qb.query(db, projection, selection, selectionArgs, groupBy, having, orderBy);
+        if (cursor != null) {
+            // Tell the cursor what uri to watch, so it knows when its source data changes
+            cursor.setNotificationUri(getContext().getContentResolver(), uri);
+        }
 
         if (Constants.DEBUG) {
             Log.d(Constants.TAG,
                     "Query: "
                             + qb.buildQuery(projection, selection, selectionArgs, null, null,
                             orderBy, null));
-            Log.d(Constants.TAG, "Cursor: " + DatabaseUtils.dumpCursorToString(c));
+            Log.d(Constants.TAG, "Cursor: " + DatabaseUtils.dumpCursorToString(cursor));
         }
 
-        return c;
+        return cursor;
     }
 
     /**
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
index 7ef186f00..ab00db13a 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
@@ -100,36 +100,38 @@ public class ProviderHelper {
             throws NotFoundException {
         Cursor cursor = mContentResolver.query(uri, proj, null, null, null);
 
-        HashMap<String, Object> result = new HashMap<String, Object>(proj.length);
-        if (cursor != null && cursor.moveToFirst()) {
-            int pos = 0;
-            for (String p : proj) {
-                switch (types[pos]) {
-                    case FIELD_TYPE_NULL:
-                        result.put(p, cursor.isNull(pos));
-                        break;
-                    case FIELD_TYPE_INTEGER:
-                        result.put(p, cursor.getLong(pos));
-                        break;
-                    case FIELD_TYPE_FLOAT:
-                        result.put(p, cursor.getFloat(pos));
-                        break;
-                    case FIELD_TYPE_STRING:
-                        result.put(p, cursor.getString(pos));
-                        break;
-                    case FIELD_TYPE_BLOB:
-                        result.put(p, cursor.getBlob(pos));
-                        break;
+        try {
+            HashMap<String, Object> result = new HashMap<String, Object>(proj.length);
+            if (cursor != null && cursor.moveToFirst()) {
+                int pos = 0;
+                for (String p : proj) {
+                    switch (types[pos]) {
+                        case FIELD_TYPE_NULL:
+                            result.put(p, cursor.isNull(pos));
+                            break;
+                        case FIELD_TYPE_INTEGER:
+                            result.put(p, cursor.getLong(pos));
+                            break;
+                        case FIELD_TYPE_FLOAT:
+                            result.put(p, cursor.getFloat(pos));
+                            break;
+                        case FIELD_TYPE_STRING:
+                            result.put(p, cursor.getString(pos));
+                            break;
+                        case FIELD_TYPE_BLOB:
+                            result.put(p, cursor.getBlob(pos));
+                            break;
+                    }
+                    pos += 1;
                 }
-                pos += 1;
             }
-        }
 
-        if (cursor != null) {
-            cursor.close();
+            return result;
+        } finally {
+            if (cursor != null) {
+                cursor.close();
+            }
         }
-
-        return result;
     }
 
     public Object getUnifiedData(long masterKeyId, String column, int type)
@@ -576,27 +578,29 @@ public class ProviderHelper {
             }, inMasterKeyList, null, null);
         }
 
-        if (cursor != null) {
-            int masterIdCol = cursor.getColumnIndex(KeyRingData.MASTER_KEY_ID);
-            int dataCol = cursor.getColumnIndex(KeyRingData.KEY_RING_DATA);
-            if (cursor.moveToFirst()) {
-                do {
-                    Log.d(Constants.TAG, "masterKeyId: " + cursor.getLong(masterIdCol));
-
-                    byte[] data = cursor.getBlob(dataCol);
-
-                    // get actual keyring data blob and write it to ByteArrayOutputStream
-                    try {
-                        output.add(getKeyRingAsArmoredString(data));
-                    } catch (IOException e) {
-                        Log.e(Constants.TAG, "IOException", e);
-                    }
-                } while (cursor.moveToNext());
+        try {
+            if (cursor != null) {
+                int masterIdCol = cursor.getColumnIndex(KeyRingData.MASTER_KEY_ID);
+                int dataCol = cursor.getColumnIndex(KeyRingData.KEY_RING_DATA);
+                if (cursor.moveToFirst()) {
+                    do {
+                        Log.d(Constants.TAG, "masterKeyId: " + cursor.getLong(masterIdCol));
+
+                        byte[] data = cursor.getBlob(dataCol);
+
+                        // get actual keyring data blob and write it to ByteArrayOutputStream
+                        try {
+                            output.add(getKeyRingAsArmoredString(data));
+                        } catch (IOException e) {
+                            Log.e(Constants.TAG, "IOException", e);
+                        }
+                    } while (cursor.moveToNext());
+                }
+            }
+        } finally {
+            if (cursor != null) {
+                cursor.close();
             }
-        }
-
-        if (cursor != null) {
-            cursor.close();
         }
 
         if (output.size() > 0) {
@@ -610,17 +614,19 @@ public class ProviderHelper {
         Cursor cursor = mContentResolver.query(ApiApps.CONTENT_URI, null, null, null, null);
 
         ArrayList<String> packageNames = new ArrayList<String>();
-        if (cursor != null) {
-            int packageNameCol = cursor.getColumnIndex(ApiApps.PACKAGE_NAME);
-            if (cursor.moveToFirst()) {
-                do {
-                    packageNames.add(cursor.getString(packageNameCol));
-                } while (cursor.moveToNext());
+        try {
+            if (cursor != null) {
+                int packageNameCol = cursor.getColumnIndex(ApiApps.PACKAGE_NAME);
+                if (cursor.moveToFirst()) {
+                    do {
+                        packageNames.add(cursor.getString(packageNameCol));
+                    } while (cursor.moveToNext());
+                }
+            }
+        } finally {
+            if (cursor != null) {
+                cursor.close();
             }
-        }
-
-        if (cursor != null) {
-            cursor.close();
         }
 
         return packageNames;
@@ -668,13 +674,19 @@ public class ProviderHelper {
     public AppSettings getApiAppSettings(Uri uri) {
         AppSettings settings = null;
 
-        Cursor cur = mContentResolver.query(uri, null, null, null, null);
-        if (cur != null && cur.moveToFirst()) {
-            settings = new AppSettings();
-            settings.setPackageName(cur.getString(
-                    cur.getColumnIndex(KeychainContract.ApiApps.PACKAGE_NAME)));
-            settings.setPackageSignature(cur.getBlob(
-                    cur.getColumnIndex(KeychainContract.ApiApps.PACKAGE_SIGNATURE)));
+        Cursor cursor = mContentResolver.query(uri, null, null, null, null);
+        try {
+            if (cursor != null && cursor.moveToFirst()) {
+                settings = new AppSettings();
+                settings.setPackageName(cursor.getString(
+                        cursor.getColumnIndex(KeychainContract.ApiApps.PACKAGE_NAME)));
+                settings.setPackageSignature(cursor.getBlob(
+                        cursor.getColumnIndex(KeychainContract.ApiApps.PACKAGE_SIGNATURE)));
+            }
+        } finally {
+            if (cursor != null) {
+                cursor.close();
+            }
         }
 
         return settings;
@@ -683,20 +695,26 @@ public class ProviderHelper {
     public AccountSettings getApiAccountSettings(Uri accountUri) {
         AccountSettings settings = null;
 
-        Cursor cur = mContentResolver.query(accountUri, null, null, null, null);
-        if (cur != null && cur.moveToFirst()) {
-            settings = new AccountSettings();
-
-            settings.setAccountName(cur.getString(
-                    cur.getColumnIndex(KeychainContract.ApiAccounts.ACCOUNT_NAME)));
-            settings.setKeyId(cur.getLong(
-                    cur.getColumnIndex(KeychainContract.ApiAccounts.KEY_ID)));
-            settings.setCompression(cur.getInt(
-                    cur.getColumnIndexOrThrow(KeychainContract.ApiAccounts.COMPRESSION)));
-            settings.setHashAlgorithm(cur.getInt(
-                    cur.getColumnIndexOrThrow(KeychainContract.ApiAccounts.HASH_ALORITHM)));
-            settings.setEncryptionAlgorithm(cur.getInt(
-                    cur.getColumnIndexOrThrow(KeychainContract.ApiAccounts.ENCRYPTION_ALGORITHM)));
+        Cursor cursor = mContentResolver.query(accountUri, null, null, null, null);
+        try {
+            if (cursor != null && cursor.moveToFirst()) {
+                settings = new AccountSettings();
+
+                settings.setAccountName(cursor.getString(
+                        cursor.getColumnIndex(KeychainContract.ApiAccounts.ACCOUNT_NAME)));
+                settings.setKeyId(cursor.getLong(
+                        cursor.getColumnIndex(KeychainContract.ApiAccounts.KEY_ID)));
+                settings.setCompression(cursor.getInt(
+                        cursor.getColumnIndexOrThrow(KeychainContract.ApiAccounts.COMPRESSION)));
+                settings.setHashAlgorithm(cursor.getInt(
+                        cursor.getColumnIndexOrThrow(KeychainContract.ApiAccounts.HASH_ALORITHM)));
+                settings.setEncryptionAlgorithm(cursor.getInt(
+                        cursor.getColumnIndexOrThrow(KeychainContract.ApiAccounts.ENCRYPTION_ALGORITHM)));
+            }
+        } finally {
+            if (cursor != null) {
+                cursor.close();
+            }
         }
 
         return settings;
@@ -706,10 +724,16 @@ public class ProviderHelper {
         Set<Long> keyIds = new HashSet<Long>();
 
         Cursor cursor = mContentResolver.query(uri, null, null, null, null);
-        if (cursor != null) {
-            int keyIdColumn = cursor.getColumnIndex(KeychainContract.ApiAccounts.KEY_ID);
-            while (cursor.moveToNext()) {
-                keyIds.add(cursor.getLong(keyIdColumn));
+        try {
+            if (cursor != null) {
+                int keyIdColumn = cursor.getColumnIndex(KeychainContract.ApiAccounts.KEY_ID);
+                while (cursor.moveToNext()) {
+                    keyIds.add(cursor.getLong(keyIdColumn));
+                }
+            }
+        } finally {
+            if (cursor != null) {
+                cursor.close();
             }
         }
 
@@ -722,18 +746,18 @@ public class ProviderHelper {
         String[] projection = new String[]{ApiApps.PACKAGE_SIGNATURE};
 
         Cursor cursor = mContentResolver.query(queryUri, projection, null, null, null);
+        try {
+            byte[] signature = null;
+            if (cursor != null && cursor.moveToFirst()) {
+                int signatureCol = 0;
 
-        byte[] signature = null;
-        if (cursor != null && cursor.moveToFirst()) {
-            int signatureCol = 0;
-
-            signature = cursor.getBlob(signatureCol);
-        }
-
-        if (cursor != null) {
-            cursor.close();
+                signature = cursor.getBlob(signatureCol);
+            }
+            return signature;
+        } finally {
+            if (cursor != null) {
+                cursor.close();
+            }
         }
-
-        return signature;
     }
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
index 5691d2fe2..db2db668d 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
@@ -70,19 +70,25 @@ public class OpenPgpService extends RemoteService {
 
         for (String email : encryptionUserIds) {
             Uri uri = KeyRings.buildUnifiedKeyRingsFindByEmailUri(email);
-            Cursor cur = getContentResolver().query(uri, null, null, null, null);
-            if (cur.moveToFirst()) {
-                long id = cur.getLong(cur.getColumnIndex(KeyRings.MASTER_KEY_ID));
-                keyIds.add(id);
-            } else {
-                missingUserIdsCheck = true;
-                missingUserIds.add(email);
-                Log.d(Constants.TAG, "user id missing");
-            }
-            if (cur.moveToNext()) {
-                duplicateUserIdsCheck = true;
-                duplicateUserIds.add(email);
-                Log.d(Constants.TAG, "more than one user id with the same email");
+            Cursor cursor = getContentResolver().query(uri, null, null, null, null);
+            try {
+                if (cursor != null && cursor.moveToFirst()) {
+                    long id = cursor.getLong(cursor.getColumnIndex(KeyRings.MASTER_KEY_ID));
+                    keyIds.add(id);
+                } else {
+                    missingUserIdsCheck = true;
+                    missingUserIds.add(email);
+                    Log.d(Constants.TAG, "user id missing");
+                }
+                if (cursor != null && cursor.moveToNext()) {
+                    duplicateUserIdsCheck = true;
+                    duplicateUserIds.add(email);
+                    Log.d(Constants.TAG, "more than one user id with the same email");
+                }
+            } finally {
+                if (cursor != null) {
+                    cursor.close();
+                }
             }
         }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
index d363a4119..f1e30c560 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
@@ -690,8 +690,7 @@ public class KeychainIntentService extends IntentService
                         new String[]{KeyRings.MASTER_KEY_ID, KeyRings.HAS_ANY_SECRET},
                         selection, null, null);
                 try {
-                    cursor.moveToFirst();
-                    do {
+                    if (cursor != null && cursor.moveToFirst()) do {
                         // export public either way
                         publicMasterKeyIds.add(cursor.getLong(0));
                         // add secret if available (and requested)
@@ -699,7 +698,9 @@ public class KeychainIntentService extends IntentService
                             secretMasterKeyIds.add(cursor.getLong(0));
                     } while (cursor.moveToNext());
                 } finally {
-                    cursor.close();
+                    if (cursor != null) {
+                        cursor.close();
+                    }
                 }
 
                 PgpImportExport pgpImportExport = new PgpImportExport(this, this, this);