Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to rPGP 0.14 #6026

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Upgrade to rPGP 0.14 #6026

merged 1 commit into from
Nov 14, 2024

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Oct 4, 2024

@link2xt link2xt marked this pull request as ready for review November 13, 2024 08:15

impl PublicKeyTrait for SignedPublicKeyOrSubkey<'_> {
fn created_at(&self) -> &chrono::DateTime<chrono::Utc> {
match self {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both SignedPublicKey and SignedPublicSubKey implement PulicKeyTrait, so i think these matches everywhere can be replaced with a function returning a reference to the trait, though there are not many of them

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried:

diff --git a/src/pgp.rs b/src/pgp.rs
index f729237d9..6588bded0 100644
--- a/src/pgp.rs
+++ b/src/pgp.rs
@@ -41,47 +41,35 @@ enum SignedPublicKeyOrSubkey<'a> {
     Subkey(&'a SignedPublicSubKey),
 }
 
+impl SignedPublicKeyOrSubkey<'_> {
+    fn as_public_key_trait(&self) -> &dyn PublicKeyTrait {
+        self
+    }
+}
+
 impl PublicKeyTrait for SignedPublicKeyOrSubkey<'_> {
     fn version(&self) -> pgp::types::KeyVersion {
-        match self {
-            Self::Key(k) => k.version(),
-            Self::Subkey(k) => k.version(),
-        }
+        self.as_public_key_trait().version()
     }
 
     fn fingerprint(&self) -> pgp::types::Fingerprint {
-        match self {
-            Self::Key(k) => k.fingerprint(),
-            Self::Subkey(k) => k.fingerprint(),
-        }
+        self.as_public_key_trait().fingerprint()
     }
 
     fn key_id(&self) -> pgp::types::KeyId {
-        match self {
-            Self::Key(k) => k.key_id(),
-            Self::Subkey(k) => k.key_id(),
-        }
+        self.as_public_key_trait().key_id()
     }
 
     fn algorithm(&self) -> pgp::crypto::public_key::PublicKeyAlgorithm {
-        match self {
-            Self::Key(k) => k.algorithm(),
-            Self::Subkey(k) => k.algorithm(),
-        }
+        self.as_public_key_trait().algorithm()
     }
 
     fn created_at(&self) -> &chrono::DateTime<chrono::Utc> {
-        match self {
-            Self::Key(k) => k.created_at(),
-            Self::Subkey(k) => k.created_at(),
-        }
+        self.as_public_key_trait().created_at()
     }
 
     fn expiration(&self) -> Option<u16> {
-        match self {
-            Self::Key(k) => k.expiration(),
-            Self::Subkey(k) => k.expiration(),
-        }
+        self.as_public_key_trait().expiration()
     }
 
     fn verify_signature(
@@ -90,10 +78,7 @@ fn verify_signature(
         data: &[u8],
         sig: &SignatureBytes,
     ) -> pgp::errors::Result<()> {
-        match self {
-            Self::Key(k) => k.verify_signature(hash, data, sig),
-            Self::Subkey(k) => k.verify_signature(hash, data, sig),
-        }
+        self.as_public_key_trait().verify_signature(hash, data, sig)
     }
 
     fn encrypt<R: Rng + CryptoRng>(
@@ -102,24 +87,15 @@ fn encrypt<R: Rng + CryptoRng>(
         plain: &[u8],
         typ: pgp::types::EskType,
     ) -> pgp::errors::Result<pgp::types::PkeskBytes> {
-        match self {
-            Self::Key(k) => k.encrypt(rng, plain, typ),
-            Self::Subkey(k) => k.encrypt(rng, plain, typ),
-        }
+        self.as_public_key_trait().encrypt(rng, plain, typ)
     }
 
     fn serialize_for_hashing(&self, writer: &mut impl io::Write) -> pgp::errors::Result<()> {
-        match self {
-            Self::Key(k) => k.serialize_for_hashing(writer),
-            Self::Subkey(k) => k.serialize_for_hashing(writer),
-        }
+        self.as_public_key_trait().serialize_for_hashing(writer)
     }
 
     fn public_params(&self) -> &pgp::types::PublicParams {
-        match self {
-            Self::Key(k) => k.public_params(),
-            Self::Subkey(k) => k.public_params(),
-        }
+        self.as_public_key_trait().public_params()
     }
 }

This does not compile with errors like "the trait cannot be made into an object because method encrypt has generic type parameters" and "the trait cannot be made into an object because method serialize_for_hashing has generic type parameters".

Copy link
Collaborator

@iequidoo iequidoo Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Maybe pgp should switch to dynamic dispatch then, i.e. change

fn serialize_for_hashing(&self, writer: &mut impl Write) -> Result<()>;

to

fn serialize_for_hashing(&self, writer: &mut dyn Write) -> Result<()>;

(and the same for encrypt(), though it needs CryptoRng + Rng, so it will be more complex) or that would affect performance? CC @dignifiedquire

@link2xt link2xt merged commit e7a29f0 into main Nov 14, 2024
37 checks passed
@link2xt link2xt deleted the link2xt/pgp-0.14 branch November 14, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants