store: Fix conflict detection.

  - Detect conflicts when no key is present in the database.

  - Add comments.

2 files changed, 44 insertions, 17 deletions

diff --git a/store/src/backend.rs b/store/src/backend.rs
index 94553fc5..91cbc79f 100644
--- a/store/src/backend.rs
+++ b/store/src/backend.rs
@@ -276,39 +276,48 @@ impl node::binding::Server for BindingServer {
         bind_results!(results);
         let force = pry!(params.get()).get_force();
 
+        // This is the key to import.
         let mut new = sry!(TPK::from_bytes(&pry!(pry!(params.get()).get_key())));
 
+        // Check in the database for the current key.
         let key_id = sry!(self.key_id());
         let (fingerprint, key): (String, Option<Vec<u8>>)
             = sry!(self.c.borrow().query_row(
                 "SELECT fingerprint, key FROM keys WHERE id = ?1",
                 &[&key_id],
                 |row| (row.get(0), row.get_checked(1).ok())));
-        if let Some(current) = key {
-            let current = sry!(TPK::from_bytes(&current));
 
+        // If we found one, convert it to TPK.
+        let current = if let Some(current) = key {
+            let current = sry!(TPK::from_bytes(&current));
             if current.fingerprint().to_hex() != fingerprint {
                 // Inconsistent database.
                 fail!(node::Error::SystemError);
             }
-
-            if current.fingerprint() != new.fingerprint() {
-                if force || new.is_signed_by(&current) {
-                    // Update binding, and retry.
-                    let key_id =
-                        sry!(get_key_id(&self.c.borrow(), new.fingerprint().to_hex().as_ref()));
-                    sry!(self.c.borrow()
-                         .execute("UPDATE bindings SET key = ?1 WHERE id = ?2",
-                                  &[&key_id, &self.id]));
-                    return self.import(params, results);
-                } else {
-                    fail!(node::Error::Conflict);
-                }
+            Some(current)
+        } else {
+            None
+        };
+
+        // Check for conflicts.
+        if new.fingerprint().to_hex() != fingerprint {
+            if force || (current.is_some() && new.is_signed_by(&current.unwrap())) {
+                // Update binding, and retry.
+                let key_id =
+                    sry!(get_key_id(&self.c.borrow(), new.fingerprint().to_hex().as_ref()));
+                sry!(self.c.borrow()
+                     .execute("UPDATE bindings SET key = ?1 WHERE id = ?2",
+                              &[&key_id, &self.id]));
+                return self.import(params, results);
             } else {
-                new = sry!(current.merge(new));
+                fail!(node::Error::Conflict);
             }
         }
 
+        if current.is_some() {
+            new = sry!(current.unwrap().merge(new));
+        }
+
         // Write key back to the database.
         let mut blob = vec![];
         sry!(new.serialize(&mut blob));
diff --git a/store/src/lib.rs b/store/src/lib.rs
index 38893d68..85fe97b6 100644
--- a/store/src/lib.rs
+++ b/store/src/lib.rs
@@ -704,7 +704,11 @@ impl From<rusqlite::Error> for Error {
 
 #[cfg(test)]
 mod store_test {
-    use super::{core, Store, Error};
+    use super::{core, Store, Error, TPK, Fingerprint};
+
+    macro_rules! bytes {
+        ( $x:expr ) => { include_bytes!(concat!("../../openpgp/tests/data/keys/", $x)) };
+    }
 
     #[test]
     fn store_network_policy_mismatch() {
@@ -722,5 +726,19 @@ mod store_test {
         let store = Store::open(&ctx2, "default");
         assert_match!(Err(Error::CoreError(core::Error::NetworkPolicyViolation(_))) = store);
     }
+
+    #[test]
+    fn add_then_import_wrong_key() {
+        let ctx = core::Context::configure("org.sequoia-pgp.tests")
+            .ephemeral()
+            .network_policy(core::NetworkPolicy::Offline)
+            .build().unwrap();
+        let mut store = Store::open(&ctx, "default").unwrap();
+        let tpk = TPK::from_bytes(bytes!("testy.pgp")).unwrap();
+        let fp = Fingerprint::from_bytes(b"bbbbbbbbbbbbbbbbbbbb");
+        let binding = store.add("Mister B.", &fp).unwrap();
+        let r = binding.import(&tpk);
+        assert_match!(Err(Error::Conflict) = r);
+    }
 }