From fe4fa499565e28ca445e97a761aefdb9fefe4e8a Mon Sep 17 00:00:00 2001
From: Justus Winter <justus@sequoia-pgp.org>
Date: Thu, 16 Feb 2023 09:12:47 +0100
Subject: openpgp: Make KeyHandle::partial_cmp transitive.

  - Previously, KeyHandle::partial_cmp tried to sort aliasing handles
    together.  However, this made the function not transitive, which
    is required by implementations of PartialOrd.

  - Fix this by simply comparing the byte representations, and
    computing aliasing in KeyHandle::aliases.

  - Note: This makes PartialOrd (and PartialEq) total, but we still
    don't implement Ord (and Eq) to prevent naive comparisons.
---
 openpgp/src/keyhandle.rs | 48 ++++++++++++++++++------------------------------
 1 file changed, 18 insertions(+), 30 deletions(-)

diff --git a/openpgp/src/keyhandle.rs b/openpgp/src/keyhandle.rs
index 5e6dc3d8..cf15bbaa 100644
--- a/openpgp/src/keyhandle.rs
+++ b/openpgp/src/keyhandle.rs
@@ -1,5 +1,4 @@
 use std::convert::TryFrom;
-use std::cmp;
 use std::cmp::Ordering;
 use std::borrow::Borrow;
 
@@ -174,26 +173,7 @@ impl PartialOrd for KeyHandle {
     fn partial_cmp(&self, other: &KeyHandle) -> Option<Ordering> {
         let a = self.as_bytes();
         let b = other.as_bytes();
-
-        let l = cmp::min(a.len(), b.len());
-
-        // Do a little endian comparison so that for v4 keys (where
-        // the KeyID is a suffix of the Fingerprint) equivalent KeyIDs
-        // and Fingerprints sort next to each other.
-        for (a, b) in a[a.len()-l..].iter().zip(b[b.len()-l..].iter()) {
-            let cmp = a.cmp(b);
-            if cmp != Ordering::Equal {
-                return Some(cmp);
-            }
-        }
-
-        if a.len() == b.len() {
-            Some(Ordering::Equal)
-        } else {
-            // One (a KeyID) is the suffix of the other (a
-            // Fingerprint).
-            None
-        }
+        Some(a.cmp(b))
     }
 }
 
@@ -252,10 +232,6 @@ impl KeyHandle {
     /// fpr1 == keyid and fpr2 == keyid, but fpr1 != fpr2.
     /// ```
     ///
-    /// In these cases (and only these cases) `KeyHandle`'s
-    /// `PartialOrd` implementation returns `None` to correctly
-    /// indicate that a comparison is not possible.
-    ///
     /// This definition of equality makes searching for a given
     /// `KeyHandle` using `PartialEq` awkward.  This function fills
     /// that gap.  It answers the question: given two `KeyHandles`,
@@ -290,11 +266,23 @@ impl KeyHandle {
     pub fn aliases<H>(&self, other: H) -> bool
         where H: Borrow<KeyHandle>
     {
-        // This works, because the PartialOrd implementation only
-        // returns None if one value is a fingerprint and the other is
-        // a key id that matches the fingerprint's key id.
-        self.partial_cmp(other.borrow()).unwrap_or(Ordering::Equal)
-            == Ordering::Equal
+        let other = other.borrow();
+        if self.partial_cmp(other) == Some(Ordering::Equal) {
+            true
+        } else {
+            match (self, other) {
+                (KeyHandle::Fingerprint(Fingerprint::V4(f)),
+                 KeyHandle::KeyID(KeyID::V4(i)))
+                    | (KeyHandle::KeyID(KeyID::V4(i)),
+                       KeyHandle::Fingerprint(Fingerprint::V4(f))) =>
+                {
+                    // A v4 key ID are the 8 right-most octets of a v4
+                    // fingerprint.
+                    &f[12..] == i
+                },
+                _ => false,
+            }
+        }
     }
 
     /// Returns whether the KeyHandle is invalid.
-- 
cgit v1.2.3