Fix serde implementation

Cargo.toml

@@ -33,7 +33,8 @@ zeroize = { version = "1", default-features = false, features = ["zeroize_derive
 
 [dev-dependencies]
 hex = "^0.4"
-bincode = "^0.9"
+bincode = "1.0"
+serde_json = "1.0"
 criterion = "0.3"
 rand = "0.7"
 serde_crate = { package = "serde", version = "1.0", features = ["derive"] }

src/keypair.rs

@@ -13,11 +13,11 @@
 use rand::{CryptoRng, RngCore};
 
 #[cfg(feature = "serde")]
-use serde::de::Error as SerdeError;
+use serde::ser::SerializeTuple;
 #[cfg(feature = "serde")]
-use serde::de::Visitor;
+use serde::de::Error as SerdeError;
 #[cfg(feature = "serde")]
-use serde::de::SeqAccess;
+use serde::de::{Visitor, SeqAccess};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
@@ -427,7 +427,13 @@ impl Serialize for Keypair {
     where
         S: Serializer,
     {
-        serializer.serialize_bytes(&self.to_bytes()[..])
+        let mut seq = serializer.serialize_tuple(KEYPAIR_LENGTH)?;
+
+        for byte in &self.to_bytes()[..] {
+            seq.serialize_element(byte)?;
+        }
+
+        seq.end()
     }
 }
 
@@ -440,60 +446,32 @@ impl<'d> Deserialize<'d> for Keypair {
         struct KeypairVisitor;
 
         impl<'d> Visitor<'d> for KeypairVisitor {
-            type Value = Keypair;
+            type Value = [u8; KEYPAIR_LENGTH];
 
             fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
                 formatter.write_str("An ed25519 keypair, 64 bytes in total where the secret key is \
                                      the first 32 bytes and is in unexpanded form, and the second \
                                      32 bytes is a compressed point for a public key.")
             }
 
-            fn visit_bytes<E>(self, bytes: &[u8]) -> Result<Keypair, E>
+            fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
             where
-                E: SerdeError,
+                A: SeqAccess<'d>,
             {
-                if bytes.len() != KEYPAIR_LENGTH {
-                    return Err(SerdeError::invalid_length(bytes.len(), &self));
-                }
-
-                let secret_key = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH]);
-                let public_key = PublicKey::from_bytes(&bytes[SECRET_KEY_LENGTH..]);
+                let mut arr = [0u8; KEYPAIR_LENGTH];
 
-                if let (Ok(secret), Ok(public)) = (secret_key, public_key) {
-                    Ok(Keypair{ secret, public })
-                } else {
-                    Err(SerdeError::invalid_length(bytes.len(), &self))
+                for (i, byte) in arr.iter_mut().enumerate() {
+                    *byte = seq
+                        .next_element()?
+                        .ok_or_else(|| SerdeError::invalid_length(i, &self))?;
                 }
-            }
 
-            fn visit_seq<A>(self, mut seq: A) -> Result<Keypair, A::Error>
-            where
-                A: SeqAccess<'d>
-            {
-                if let Some(len) = seq.size_hint() {
-                    if len != KEYPAIR_LENGTH {
-                        return Err(SerdeError::invalid_length(len, &self));
-                    }
-                }
-
-                // TODO: We could do this with `MaybeUninit` to avoid unnecessary initialization costs
-                let mut bytes: [u8; KEYPAIR_LENGTH] = [0u8; KEYPAIR_LENGTH];
-
-                for i in 0..KEYPAIR_LENGTH {
-                    bytes[i] = seq.next_element()?.ok_or_else(|| SerdeError::invalid_length(i, &self))?;
-                }
-
-                let secret_key = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH]);
-                let public_key = PublicKey::from_bytes(&bytes[SECRET_KEY_LENGTH..]);
-
-                if let (Ok(secret), Ok(public)) = (secret_key, public_key) {
-                    Ok(Keypair{ secret, public })
-                } else {
-                    Err(SerdeError::invalid_length(bytes.len(), &self))
-                }
+                Ok(arr)
             }
-
         }
-        deserializer.deserialize_bytes(KeypairVisitor)
+
+        deserializer
+            .deserialize_tuple(KEYPAIR_LENGTH, KeypairVisitor)
+            .and_then(|bytes| Keypair::from_bytes(&bytes[..]).map_err(SerdeError::custom))
     }
 }

src/lib.rs

@@ -176,16 +176,16 @@
 //! # fn main() {
 //! # use rand::rngs::OsRng;
 //! # use ed25519_dalek::{Keypair, Signature, Signer, Verifier, PublicKey};
-//! use bincode::{serialize, Infinite};
+//! use bincode::serialize;
 //! # let mut csprng = OsRng{};
 //! # let keypair: Keypair = Keypair::generate(&mut csprng);
 //! # let message: &[u8] = b"This is a test of the tsunami alert system.";
 //! # let signature: Signature = keypair.sign(message);
 //! # let public_key: PublicKey = keypair.public;
 //! # let verified: bool = public_key.verify(message, &signature).is_ok();
 //!
-//! let encoded_public_key: Vec<u8> = serialize(&public_key, Infinite).unwrap();
-//! let encoded_signature: Vec<u8> = serialize(&signature, Infinite).unwrap();
+//! let encoded_public_key: Vec<u8> = serialize(&public_key).unwrap();
+//! let encoded_signature: Vec<u8> = serialize(&signature).unwrap();
 //! # }
 //! # #[cfg(not(feature = "serde"))]
 //! # fn main() {}
@@ -206,7 +206,7 @@
 //! # fn main() {
 //! # use rand::rngs::OsRng;
 //! # use ed25519_dalek::{Keypair, Signature, Signer, Verifier, PublicKey};
-//! # use bincode::{serialize, Infinite};
+//! # use bincode::serialize;
 //! use bincode::deserialize;
 //!
 //! # let mut csprng = OsRng{};
@@ -215,8 +215,8 @@
 //! # let signature: Signature = keypair.sign(message);
 //! # let public_key: PublicKey = keypair.public;
 //! # let verified: bool = public_key.verify(message, &signature).is_ok();
-//! # let encoded_public_key: Vec<u8> = serialize(&public_key, Infinite).unwrap();
-//! # let encoded_signature: Vec<u8> = serialize(&signature, Infinite).unwrap();
+//! # let encoded_public_key: Vec<u8> = serialize(&public_key).unwrap();
+//! # let encoded_signature: Vec<u8> = serialize(&signature).unwrap();
 //! let decoded_public_key: PublicKey = deserialize(&encoded_public_key).unwrap();
 //! let decoded_signature: Signature = deserialize(&encoded_signature).unwrap();
 //!

src/public.rs

@@ -26,8 +26,6 @@ pub use sha2::Sha512;
 #[cfg(feature = "serde")]
 use serde::de::Error as SerdeError;
 #[cfg(feature = "serde")]
-use serde::de::Visitor;
-#[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 #[cfg(feature = "serde")]
 use serde::{Deserializer, Serializer};
@@ -362,7 +360,7 @@ impl Serialize for PublicKey {
     where
         S: Serializer,
     {
-        serializer.serialize_bytes(self.as_bytes())
+        self.to_bytes().serialize(serializer)
     }
 }
 
@@ -372,24 +370,7 @@ impl<'d> Deserialize<'d> for PublicKey {
     where
         D: Deserializer<'d>,
     {
-        struct PublicKeyVisitor;
-
-        impl<'d> Visitor<'d> for PublicKeyVisitor {
-            type Value = PublicKey;
-
-            fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
-                formatter.write_str(
-                    "An ed25519 public key as a 32-byte compressed point, as specified in RFC8032",
-                )
-            }
-
-            fn visit_bytes<E>(self, bytes: &[u8]) -> Result<PublicKey, E>
-            where
-                E: SerdeError,
-            {
-                PublicKey::from_bytes(bytes).or(Err(SerdeError::invalid_length(bytes.len(), &self)))
-            }
-        }
-        deserializer.deserialize_bytes(PublicKeyVisitor)
+        let bytes = <[u8; PUBLIC_KEY_LENGTH]>::deserialize(deserializer)?;
+        PublicKey::from_bytes(&bytes[..]).map_err(SerdeError::custom)
     }
 }

src/secret.rs

@@ -21,10 +21,12 @@ use rand::{CryptoRng, RngCore};
 
 use sha2::Sha512;
 
+#[cfg(feature = "serde")]
+use serde::ser::SerializeTuple;
 #[cfg(feature = "serde")]
 use serde::de::Error as SerdeError;
 #[cfg(feature = "serde")]
-use serde::de::Visitor;
+use serde::de::{Visitor, SeqAccess};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 #[cfg(feature = "serde")]
@@ -182,7 +184,7 @@ impl Serialize for SecretKey {
     where
         S: Serializer,
     {
-        serializer.serialize_bytes(self.as_bytes())
+        self.to_bytes().serialize(serializer)
     }
 }
 
@@ -192,23 +194,8 @@ impl<'d> Deserialize<'d> for SecretKey {
     where
         D: Deserializer<'d>,
     {
-        struct SecretKeyVisitor;
-
-        impl<'d> Visitor<'d> for SecretKeyVisitor {
-            type Value = SecretKey;
-
-            fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
-                formatter.write_str("An ed25519 secret key as 32 bytes, as specified in RFC8032.")
-            }
-
-            fn visit_bytes<E>(self, bytes: &[u8]) -> Result<SecretKey, E>
-            where
-                E: SerdeError,
-            {
-                SecretKey::from_bytes(bytes).or(Err(SerdeError::invalid_length(bytes.len(), &self)))
-            }
-        }
-        deserializer.deserialize_bytes(SecretKeyVisitor)
+        let bytes = <[u8; SECRET_KEY_LENGTH]>::deserialize(deserializer)?;
+        SecretKey::from_bytes(&bytes[..]).map_err(SerdeError::custom)
     }
 }
 
@@ -519,7 +506,13 @@ impl Serialize for ExpandedSecretKey {
     where
         S: Serializer,
     {
-        serializer.serialize_bytes(&self.to_bytes()[..])
+        let mut seq = serializer.serialize_tuple(EXPANDED_SECRET_KEY_LENGTH)?;
+
+        for byte in &self.to_bytes()[..] {
+            seq.serialize_element(byte)?;
+        }
+
+        seq.end()
     }
 }
 
@@ -532,23 +525,33 @@ impl<'d> Deserialize<'d> for ExpandedSecretKey {
         struct ExpandedSecretKeyVisitor;
 
         impl<'d> Visitor<'d> for ExpandedSecretKeyVisitor {
-            type Value = ExpandedSecretKey;
+            type Value = [u8; EXPANDED_SECRET_KEY_LENGTH];
 
             fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
                 formatter.write_str(
                     "An ed25519 expanded secret key as 64 bytes, as specified in RFC8032.",
                 )
             }
 
-            fn visit_bytes<E>(self, bytes: &[u8]) -> Result<ExpandedSecretKey, E>
+            fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
             where
-                E: SerdeError,
+                A: SeqAccess<'d>,
             {
-                ExpandedSecretKey::from_bytes(bytes)
-                    .or(Err(SerdeError::invalid_length(bytes.len(), &self)))
+                let mut arr = [0u8; EXPANDED_SECRET_KEY_LENGTH];
+
+                for (i, byte) in arr.iter_mut().enumerate() {
+                    *byte = seq
+                        .next_element()?
+                        .ok_or_else(|| SerdeError::invalid_length(i, &self))?;
+                }
+
+                Ok(arr)
             }
         }
-        deserializer.deserialize_bytes(ExpandedSecretKeyVisitor)
+
+        deserializer
+            .deserialize_tuple(EXPANDED_SECRET_KEY_LENGTH, ExpandedSecretKeyVisitor)
+            .and_then(|bytes| ExpandedSecretKey::from_bytes(&bytes[..]).map_err(SerdeError::custom))
     }
 }