From cbc3988628d09ed8a4fe967e1f21786f46cb038b Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 8 May 2014 15:56:32 +0200 Subject: proper null checks and closing of cursors everywhere --- .../keychain/helper/FileHelper.java | 12 +- .../keychain/provider/KeychainDatabase.java | 65 ++++--- .../keychain/provider/KeychainProvider.java | 13 +- .../keychain/provider/ProviderHelper.java | 208 ++++++++++++--------- .../keychain/remote/OpenPgpService.java | 32 ++-- .../keychain/service/KeychainIntentService.java | 7 +- 6 files changed, 189 insertions(+), 148 deletions(-) (limited to 'OpenKeychain') 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 result = new HashMap(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 result = new HashMap(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 packageNames = new ArrayList(); - 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 keyIds = new HashSet(); 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); -- cgit v1.2.3