test and fix: adding an empty user id should fail

4 files changed, 20 insertions, 1 deletions

diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java
index 5c6072c25..e423d790d 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java
@@ -107,7 +107,7 @@ public class PgpKeyOperationTest {
     }
 
     @Test
-    public void testAlgorithmChoice() {
+    public void createSecretKeyRingTests() {
 
         OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
 
@@ -629,6 +629,15 @@ public class PgpKeyOperationTest {
     @Test
     public void testUserIdAdd() throws Exception {
 
+        {
+            parcel.mAddUserIds.add("");
+            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
+            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
+            Assert.assertNull("adding an empty user id should fail", modified);
+        }
+
+        parcel.reset();
         parcel.mAddUserIds.add("rainbow");
 
         UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
@@ -689,6 +698,9 @@ public class PgpKeyOperationTest {
             parcel.reset();
             //noinspection SpellCheckingInspection
             parcel.mChangePrimaryUserId = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+            if (parcel.mChangePrimaryUserId.equals(passphrase)) {
+                parcel.mChangePrimaryUserId += "A";
+            }
 
             WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
             OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
index 00f73a5b0..69244ca14 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -313,6 +313,11 @@ public class PgpKeyOperation {
             for (String userId : saveParcel.mAddUserIds) {
                 log.add(LogLevel.INFO, LogType.MSG_MF_UID_ADD, indent);
 
+                if (userId.equals("")) {
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_UID_ERROR_EMPTY, indent+1);
+                    return null;
+                }
+
                 // this operation supersedes all previous binding and revocation certificates,
                 // so remove those to retain assertions from canonicalization for later operations
                 @SuppressWarnings("unchecked")
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 89d534df6..e55ec5568 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -279,6 +279,7 @@ public class OperationResultParcel implements Parcelable {
         MSG_MF_UID_ADD (R.string.msg_mf_uid_add),
         MSG_MF_UID_PRIMARY (R.string.msg_mf_uid_primary),
         MSG_MF_UID_REVOKE (R.string.msg_mf_uid_revoke),
+        MSG_MF_UID_ERROR_EMPTY (R.string.msg_mf_uid_error_empty),
         MSG_MF_UNLOCK_ERROR (R.string.msg_mf_unlock_error),
         MSG_MF_UNLOCK (R.string.msg_mf_unlock),
         ;
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 55ecf3ae0..d1a20813b 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -674,6 +674,7 @@
     <string name="msg_mf_uid_add">Adding user id %s</string>
     <string name="msg_mf_uid_primary">Changing primary uid to %s</string>
     <string name="msg_mf_uid_revoke">Revoking user id %s</string>
+    <string name="msg_mf_uid_error_empty">User ID must not be empty!</string>
     <string name="msg_mf_unlock_error">Error unlocking keyring!</string>
     <string name="msg_mf_unlock">Unlocking keyring</string>