refactor: Make Fingerprint not implement Display (#8177)

Currently, the Fingerprint type implements Display, but this doesn't get
you the canonical fingerprint representation, but something
human-readable. This is confusing, and back when I first used
`Fingerprint`, I immediately wrote a bug because of this. So, instead,
make a function `human_readable()` on Fingerprint.

This comes from the discussion at
https://github.com/chatmail/core/pull/8174#discussion_r3143130722.
This commit is contained in:
Hocuri
2026-04-27 11:22:21 +02:00
committed by GitHub
parent 0580056b62
commit b806efa096
7 changed files with 34 additions and 33 deletions

View File

@@ -238,7 +238,7 @@ impl From<Qr> for QrObject {
is_v3, is_v3,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::AskVerifyContact { QrObject::AskVerifyContact {
contact_id, contact_id,
fingerprint, fingerprint,
@@ -257,7 +257,7 @@ impl From<Qr> for QrObject {
is_v3, is_v3,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::AskVerifyGroup { QrObject::AskVerifyGroup {
grpname, grpname,
grpid, grpid,
@@ -278,7 +278,7 @@ impl From<Qr> for QrObject {
is_v3, is_v3,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::AskJoinBroadcast { QrObject::AskJoinBroadcast {
name, name,
grpid, grpid,
@@ -321,7 +321,7 @@ impl From<Qr> for QrObject {
authcode, authcode,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::WithdrawVerifyContact { QrObject::WithdrawVerifyContact {
contact_id, contact_id,
fingerprint, fingerprint,
@@ -338,7 +338,7 @@ impl From<Qr> for QrObject {
authcode, authcode,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::WithdrawVerifyGroup { QrObject::WithdrawVerifyGroup {
grpname, grpname,
grpid, grpid,
@@ -357,7 +357,7 @@ impl From<Qr> for QrObject {
authcode, authcode,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::WithdrawJoinBroadcast { QrObject::WithdrawJoinBroadcast {
name, name,
grpid, grpid,
@@ -374,7 +374,7 @@ impl From<Qr> for QrObject {
authcode, authcode,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::ReviveVerifyContact { QrObject::ReviveVerifyContact {
contact_id, contact_id,
fingerprint, fingerprint,
@@ -391,7 +391,7 @@ impl From<Qr> for QrObject {
authcode, authcode,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::ReviveVerifyGroup { QrObject::ReviveVerifyGroup {
grpname, grpname,
grpid, grpid,
@@ -410,7 +410,7 @@ impl From<Qr> for QrObject {
authcode, authcode,
} => { } => {
let contact_id = contact_id.to_u32(); let contact_id = contact_id.to_u32();
let fingerprint = fingerprint.to_string(); let fingerprint = fingerprint.human_readable();
QrObject::ReviveJoinBroadcast { QrObject::ReviveJoinBroadcast {
name, name,
grpid, grpid,

View File

@@ -1210,7 +1210,8 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=?
); );
let fingerprint = contact let fingerprint = contact
.fingerprint() .fingerprint()
.context("Contact does not have a fingerprint in encrypted chat")?; .context("Contact does not have a fingerprint in encrypted chat")?
.human_readable();
if let Some(public_key) = contact.public_key(context).await? { if let Some(public_key) = contact.public_key(context).await? {
if let Some(relay_addrs) = addresses_from_public_key(&public_key) { if let Some(relay_addrs) = addresses_from_public_key(&public_key) {
let relays = relay_addrs.join(","); let relays = relay_addrs.join(",");

View File

@@ -1396,7 +1396,7 @@ WHERE addr=?
let Some(fingerprint_other) = contact.fingerprint() else { let Some(fingerprint_other) = contact.fingerprint() else {
return Ok(stock_str::encr_none(context)); return Ok(stock_str::encr_none(context));
}; };
let fingerprint_other = fingerprint_other.to_string(); let fingerprint_other = fingerprint_other.human_readable();
let stock_message = if contact.public_key(context).await?.is_some() { let stock_message = if contact.public_key(context).await?.is_some() {
stock_str::messages_are_e2ee(context) stock_str::messages_are_e2ee(context)
@@ -1410,7 +1410,7 @@ WHERE addr=?
let fingerprint_self = load_self_public_key(context) let fingerprint_self = load_self_public_key(context)
.await? .await?
.dc_fingerprint() .dc_fingerprint()
.to_string(); .human_readable();
if addr < contact.addr { if addr < contact.addr {
cat_fingerprint( cat_fingerprint(
&mut ret, &mut ret,

View File

@@ -1,7 +1,7 @@
//! Cryptographic key module. //! Cryptographic key module.
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::fmt; use std::fmt::{self, Write as _};
use std::io::Cursor; use std::io::Cursor;
use anyhow::{Context as _, Result, bail, ensure}; use anyhow::{Context as _, Result, bail, ensure};
@@ -583,6 +583,21 @@ impl Fingerprint {
pub fn hex(&self) -> String { pub fn hex(&self) -> String {
hex::encode_upper(&self.0) hex::encode_upper(&self.0)
} }
/// Make a human-readable fingerprint.
pub fn human_readable(&self) -> String {
let mut f = String::new();
// Split key into chunks of 4 with space and newline at 20 chars
for (i, c) in self.hex().chars().enumerate() {
if i > 0 && i % 20 == 0 {
writeln!(&mut f).ok();
} else if i > 0 && i % 4 == 0 {
write!(&mut f, " ").ok();
}
write!(&mut f, "{c}").ok();
}
f
}
} }
impl From<pgp::types::Fingerprint> for Fingerprint { impl From<pgp::types::Fingerprint> for Fingerprint {
@@ -599,22 +614,6 @@ impl fmt::Debug for Fingerprint {
} }
} }
/// Make a human-readable fingerprint.
impl fmt::Display for Fingerprint {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// Split key into chunks of 4 with space and newline at 20 chars
for (i, c) in self.hex().chars().enumerate() {
if i > 0 && i % 20 == 0 {
writeln!(f)?;
} else if i > 0 && i % 4 == 0 {
write!(f, " ")?;
}
write!(f, "{c}")?;
}
Ok(())
}
}
/// Parse a human-readable or otherwise formatted fingerprint. /// Parse a human-readable or otherwise formatted fingerprint.
impl std::str::FromStr for Fingerprint { impl std::str::FromStr for Fingerprint {
type Err = anyhow::Error; type Err = anyhow::Error;
@@ -890,7 +889,7 @@ i8pcjGO+IZffvyZJVRWfVooBJmWWbPB1pueo3tx8w3+fcuzpxz+RLFKaPyqXO+dD
1, 2, 4, 8, 16, 32, 64, 128, 255, 1, 2, 4, 8, 16, 32, 64, 128, 255, 19, 20, 1, 2, 4, 8, 16, 32, 64, 128, 255, 1, 2, 4, 8, 16, 32, 64, 128, 255, 19, 20,
]); ]);
assert_eq!( assert_eq!(
fp.to_string(), fp.human_readable(),
"0102 0408 1020 4080 FF01\n0204 0810 2040 80FF 1314" "0102 0408 1020 4080 FF01\n0204 0810 2040 80FF 1314"
); );
} }

View File

@@ -645,7 +645,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result<Qr> {
} }
} else { } else {
Ok(Qr::FprWithoutAddr { Ok(Qr::FprWithoutAddr {
fingerprint: fingerprint.to_string(), fingerprint: fingerprint.human_readable(),
}) })
} }
} }

View File

@@ -388,7 +388,7 @@ async fn test_decode_openpgp_fingerprint() -> Result<()> {
bob, bob,
&format!( &format!(
"OPENPGP4FPR:{}#a=alice@example.org", "OPENPGP4FPR:{}#a=alice@example.org",
alice_contact.fingerprint().unwrap() alice_contact.fingerprint().unwrap().hex()
), ),
) )
.await?; .await?;

View File

@@ -861,7 +861,8 @@ fn encrypted_and_signed(
} else { } else {
warn!( warn!(
context, context,
"Message does not match expected fingerprint {expected_fingerprint}.", "Message does not match expected fingerprint {}.",
expected_fingerprint.human_readable()
); );
false false
} }