From f8d895dea4ba1a5133107474410813f4128176ef Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 18 Jun 2014 20:13:01 +0200 Subject: consolidate: almost sane logging --- .../keychain/pgp/UncachedKeyRing.java | 2 +- .../keychain/provider/ProviderHelper.java | 96 ++++++++++++---------- .../keychain/service/OperationResultParcel.java | 7 +- OpenKeychain/src/main/res/values/strings.xml | 7 +- 4 files changed, 63 insertions(+), 49 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java index 0f0d7cca3..a1c6b158b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -634,7 +634,7 @@ public class UncachedKeyRing { */ public UncachedKeyRing merge(UncachedKeyRing other, OperationLog log, int indent) { - log.add(LogLevel.START, isSecret() ? LogType.MSG_MG_SECRET : LogType.MSG_MG_PUBLIC, + log.add(LogLevel.DEBUG, isSecret() ? LogType.MSG_MG_SECRET : LogType.MSG_MG_PUBLIC, new String[]{ PgpKeyHelper.convertKeyIdToHex(getMasterKeyId()) }, indent); indent += 1; 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 36b4b4ac7..79bd5777c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -532,7 +532,7 @@ public class ProviderHelper { mIndent -= 1; return SaveKeyringResult.RESULT_ERROR; } catch (OperationApplicationException e) { - log(LogLevel.ERROR, LogType.MSG_IP_FAIL_OP_EX); + log(LogLevel.ERROR, LogType.MSG_IP_FAIL_OP_EXC); Log.e(Constants.TAG, "OperationApplicationException during import", e); mIndent -= 1; return SaveKeyringResult.RESULT_ERROR; @@ -603,7 +603,7 @@ public class ProviderHelper { } } catch (IOException e) { Log.e(Constants.TAG, "Failed to encode key!", e); - log(LogLevel.ERROR, LogType.MSG_IS_IO_EXCPTION); + log(LogLevel.ERROR, LogType.MSG_IS_FAIL_IO_EXC); return SaveKeyringResult.RESULT_ERROR; } @@ -681,16 +681,22 @@ public class ProviderHelper { long masterKeyId = publicRing.getMasterKeyId(); log(LogLevel.START, LogType.MSG_IP, new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) }); - mIndent += 1; - // IF there is a secret key, preserve it! + // If there is an old keyring, merge it try { UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncached(); // Merge data from new public ring into the old one publicRing = oldPublicRing.merge(publicRing, mLog, mIndent); + // If this is null, there is an error in the log so we can just return + if (publicRing == null) { + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); + } + + // Canonicalize this keyring, to assert a number of assumptions made about it. + publicRing = publicRing.canonicalize(mLog, mIndent); if (publicRing == null) { return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } @@ -698,37 +704,37 @@ public class ProviderHelper { // Early breakout if nothing changed if (Arrays.hashCode(publicRing.getEncoded()) == Arrays.hashCode(oldPublicRing.getEncoded())) { - log(LogLevel.OK, LogType.MSG_IP, - new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) }); + log(LogLevel.OK, LogType.MSG_IP_SUCCESS_IDENTICAL, null); return new SaveKeyringResult(SaveKeyringResult.RESULT_OK, mLog); } } catch (NotFoundException e) { - // not an issue, just means we are dealing with a new keyring - } + // Not an issue, just means we are dealing with a new keyring. + + // Canonicalize this keyring, to assert a number of assumptions made about it. + publicRing = publicRing.canonicalize(mLog, mIndent); + if (publicRing == null) { + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); + } - // Canonicalize this keyring, to assert a number of assumptions made about it. - publicRing = publicRing.canonicalize(mLog, mIndent); - if (publicRing == null) { - return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } - // IF there is a secret key, preserve it! + // If there is a secret key, merge new data (if any) and save the key for later UncachedKeyRing secretRing; try { secretRing = getWrappedSecretKeyRing(publicRing.getMasterKeyId()).getUncached(); - log(LogLevel.DEBUG, LogType.MSG_IP_PRESERVING_SECRET); - progress.setProgress(LogType.MSG_IP_PRESERVING_SECRET.getMsgId(), 10, 100); - mIndent += 1; // Merge data from new public ring into secret one secretRing = secretRing.merge(publicRing, mLog, mIndent); if (secretRing == null) { return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } - - mIndent -= 1; + secretRing = secretRing.canonicalize(mLog, mIndent); + if (secretRing == null) { + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); + } } catch (NotFoundException e) { + // No secret key available (this is what happens most of the time) secretRing = null; } @@ -736,19 +742,19 @@ public class ProviderHelper { // Save the saved keyring (if any) if (secretRing != null) { - log(LogLevel.DEBUG, LogType.MSG_IP_REINSERT_SECRET); progress.setProgress(LogType.MSG_IP_REINSERT_SECRET.getMsgId(), 90, 100); - mIndent += 1; - secretRing = secretRing.canonicalize(mLog, mIndent); - internalSaveSecretKeyRing(secretRing); - result |= SaveKeyringResult.SAVED_SECRET; - mIndent -= 1; + int secretResult = internalSaveSecretKeyRing(secretRing); + if ((secretResult & SaveKeyringResult.RESULT_ERROR) != SaveKeyringResult.RESULT_ERROR) { + result |= SaveKeyringResult.SAVED_SECRET; + } } + mIndent -= 1; return new SaveKeyringResult(result, mLog); } catch (IOException e) { - return null; + log(LogLevel.ERROR, LogType.MSG_IP_FAIL_IO_EXC); + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } } @@ -759,16 +765,22 @@ public class ProviderHelper { long masterKeyId = secretRing.getMasterKeyId(); log(LogLevel.START, LogType.MSG_IS, new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) }); - mIndent += 1; - // If there is a secret key, merge it. + // If there is an old secret key, merge it. try { UncachedKeyRing oldSecretRing = getWrappedSecretKeyRing(masterKeyId).getUncached(); - // Merge data from new public ring into the old one + // Merge data from new secret ring into old one secretRing = oldSecretRing.merge(secretRing, mLog, mIndent); + // If this is null, there is an error in the log so we can just return + if (secretRing == null) { + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); + } + + // Canonicalize this keyring, to assert a number of assumptions made about it. + secretRing = secretRing.canonicalize(mLog, mIndent); if (secretRing == null) { return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } @@ -776,26 +788,24 @@ public class ProviderHelper { // Early breakout if nothing changed if (Arrays.hashCode(secretRing.getEncoded()) == Arrays.hashCode(oldSecretRing.getEncoded())) { - log(LogLevel.OK, LogType.MSG_IS, + log(LogLevel.OK, LogType.MSG_IS_SUCCESS_IDENTICAL, new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) }); return new SaveKeyringResult(SaveKeyringResult.RESULT_OK, mLog); } } catch (NotFoundException e) { - // not an issue, just means we are dealing with a new keyring - } + // Not an issue, just means we are dealing with a new keyring + + // Canonicalize this keyring, to assert a number of assumptions made about it. + secretRing = secretRing.canonicalize(mLog, mIndent); + if (secretRing == null) { + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); + } - // Canonicalize this keyring, to assert a number of assumptions made about it. - secretRing = secretRing.canonicalize(mLog, mIndent); - if (secretRing == null) { - return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } // Merge new data into public keyring as well, if there is any try { UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncached(); - log(LogLevel.DEBUG, LogType.MSG_IP_PRESERVING_SECRET); - progress.setProgress(LogType.MSG_IP_PRESERVING_SECRET.getMsgId(), 10, 100); - mIndent += 1; // Merge data from new public ring into secret one UncachedKeyRing publicRing = oldPublicRing.merge(secretRing, mLog, mIndent); @@ -803,7 +813,7 @@ public class ProviderHelper { return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } - // Early breakout if nothing changed + // If anything changed, reinsert if (Arrays.hashCode(publicRing.getEncoded()) != Arrays.hashCode(oldPublicRing.getEncoded())) { @@ -816,11 +826,12 @@ public class ProviderHelper { } int result = internalSavePublicKeyRing(publicRing, progress, true); + if ((result & SaveKeyringResult.RESULT_ERROR) == SaveKeyringResult.RESULT_ERROR) { + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); + } } - mIndent -= 1; - } catch (NotFoundException e) { // TODO, this WILL error out later because secret rings cannot be inserted without // public ones @@ -831,7 +842,8 @@ public class ProviderHelper { return new SaveKeyringResult(result, mLog); } catch (IOException e) { - return null; + log(LogLevel.ERROR, LogType.MSG_IS_FAIL_IO_EXC, null); + return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index 4010ab108..48eb39a39 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -131,12 +131,11 @@ public class OperationResultParcel implements Parcelable { MSG_IP_DELETE_OLD_OK (R.string.msg_ip_delete_old_ok), MSG_IP_ENCODE_FAIL (R.string.msg_ip_encode_fail), MSG_IP_FAIL_IO_EXC (R.string.msg_ip_fail_io_exc), - MSG_IP_FAIL_OP_EX (R.string.msg_ip_fail_op_ex), + MSG_IP_FAIL_OP_EXC (R.string.msg_ip_fail_op_exc), MSG_IP_FAIL_REMOTE_EX (R.string.msg_ip_fail_remote_ex), MSG_IP_INSERT_KEYRING (R.string.msg_ip_insert_keyring), MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_keys), MSG_IP_PREPARE (R.string.msg_ip_prepare), - MSG_IP_PRESERVING_SECRET (R.string.msg_ip_preserving_secret), MSG_IP_REINSERT_SECRET (R.string.msg_ip_reinsert_secret), MSG_IP_MASTER (R.string.msg_ip_master), MSG_IP_MASTER_EXPIRED (R.string.msg_ip_master_expired), @@ -161,6 +160,7 @@ public class OperationResultParcel implements Parcelable { MSG_IP_SUBKEY_FLAGS_XXS (R.string.msg_ip_subkey_flags_xxs), MSG_IP_SUBKEY_FLAGS_XXX (R.string.msg_ip_subkey_flags_xxx), MSG_IP_SUCCESS (R.string.msg_ip_success), + MSG_IP_SUCCESS_IDENTICAL (R.string.msg_ip_success_identical), MSG_IP_UID_CERT_BAD (R.string.msg_ip_uid_cert_bad), MSG_IP_UID_CERT_ERROR (R.string.msg_ip_uid_cert_error), MSG_IP_UID_CERT_GOOD (R.string.msg_ip_uid_cert_good), @@ -175,11 +175,12 @@ public class OperationResultParcel implements Parcelable { MSG_IS_BAD_TYPE_PUBLIC (R.string.msg_is_bad_type_public), MSG_IS_DB_EXCEPTION (R.string.msg_is_db_exception), MSG_IS_IMPORTING_SUBKEYS (R.string.msg_is_importing_subkeys), - MSG_IS_IO_EXCPTION (R.string.msg_is_io_excption), + MSG_IS_FAIL_IO_EXC (R.string.msg_is_io_exc), MSG_IS_SUBKEY_NONEXISTENT (R.string.msg_is_subkey_nonexistent), MSG_IS_SUBKEY_OK (R.string.msg_is_subkey_ok), MSG_IS_SUBKEY_STRIPPED (R.string.msg_is_subkey_stripped), MSG_IS_SUCCESS (R.string.msg_is_success), + MSG_IS_SUCCESS_IDENTICAL (R.string.msg_is_success_identical), // keyring canonicalization MSG_KC_PUBLIC (R.string.msg_kc_public), diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index d9c033a96..ed9093194 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -507,13 +507,12 @@ Deleted old key from database Operation failed due to encoding error Operation failed due to i/o error - Operation failed due to database error + Operation failed due to database error Operation failed due to internal error Importing public keyring %s Encoding keyring data Parsing keys Preparing database operations - Preserving available secret key Processing master key %s Keyring expired on %s Keyring expires on %s @@ -537,6 +536,7 @@ Subkey flags: sign Subkey flags: none Successfully imported public keyring + Keyring contains no new data, nothing to do Re-inserting secret key Encountered bad certificate! Error processing certificate! @@ -552,11 +552,12 @@ Importing secret key %s Database error! Processing secret subkeys - Error encoding keyring + Error encoding keyring Subkey %s unavailable in public key Marked %s as available Marked %s as stripped Successfully imported secret keyring + Keyring contains no new data, nothing to do Canonicalizing public keyring %s -- cgit v1.2.3