From 1961af9046009eb2569bde2aad4aa35c4fdd98e0 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Fri, 15 May 2020 15:37:08 +0300 Subject: [PATCH 1/4] Fix SecretKey's serde impl to support owned data --- src/key.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++- src/macros.rs | 44 -------------------------------- 2 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/key.rs b/src/key.rs index ba1d22177..84b6a9d5c 100644 --- a/src/key.rs +++ b/src/key.rs @@ -199,7 +199,74 @@ impl SecretKey { } } -serde_impl!(SecretKey, constants::SECRET_KEY_SIZE); + +#[cfg(feature = "serde")] +impl ::serde::Serialize for SecretKey { + fn serialize(&self, s: S) -> Result { + if s.is_human_readable() { + s.collect_str(self) + } else { + s.serialize_bytes(&self[..]) + } + } +} + +#[cfg(feature = "serde")] +impl<'de> ::serde::Deserialize<'de> for SecretKey { + fn deserialize>(d: D) -> Result { + if d.is_human_readable() { + struct HexVisitor; + + impl<'de> ::serde::de::Visitor<'de> for HexVisitor { + type Value = SecretKey; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a hex string representing 32 byte SecretKey") + } + + fn visit_bytes(self, v: &[u8]) -> Result + where + E: ::serde::de::Error, + { + if let Ok(hex) = str::from_utf8(v) { + str::FromStr::from_str(hex).map_err(E::custom) + } else { + Err(E::invalid_value(::serde::de::Unexpected::Bytes(v), &self)) + } + } + + fn visit_str(self, v: &str) -> Result + where + E: ::serde::de::Error, + { + str::FromStr::from_str(v).map_err(E::custom) + } + } + + d.deserialize_str(HexVisitor) + } else { + struct BytesVisitor; + + impl<'de> ::serde::de::Visitor<'de> for BytesVisitor { + type Value = SecretKey; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("raw 32 bytes SecretKey") + } + + fn visit_bytes(self, v: &[u8]) -> Result + where + E: ::serde::de::Error, + { + SecretKey::from_slice(v).map_err(E::custom) + } + } + + d.deserialize_bytes(BytesVisitor) + } + } +} + impl PublicKey { /// Obtains a raw const pointer suitable for use with FFI functions diff --git a/src/macros.rs b/src/macros.rs index 9cf9ba6d3..bfd41b7bb 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -43,47 +43,3 @@ macro_rules! impl_from_array_len { )+ } } - -#[cfg(feature="serde")] -/// Implements `Serialize` and `Deserialize` for a type `$t` which represents -/// a newtype over a byte-slice over length `$len`. Type `$t` must implement -/// the `FromStr` and `Display` trait. -macro_rules! serde_impl( - ($t:ident, $len:expr) => ( - impl ::serde::Serialize for $t { - fn serialize(&self, s: S) -> Result { - if s.is_human_readable() { - s.collect_str(self) - } else { - s.serialize_bytes(&self[..]) - } - } - } - - impl<'de> ::serde::Deserialize<'de> for $t { - fn deserialize>(d: D) -> Result<$t, D::Error> { - use ::serde::de::Error; - use core::str::FromStr; - - if d.is_human_readable() { - let sl: &str = ::serde::Deserialize::deserialize(d)?; - SecretKey::from_str(sl).map_err(D::Error::custom) - } else { - let sl: &[u8] = ::serde::Deserialize::deserialize(d)?; - if sl.len() != $len { - Err(D::Error::invalid_length(sl.len(), &stringify!($len))) - } else { - let mut ret = [0; $len]; - ret.copy_from_slice(sl); - Ok($t(ret)) - } - } - } - } - ) -); - -#[cfg(not(feature="serde"))] -macro_rules! serde_impl( - ($t:ident, $len:expr) => () -); From 53c8dedc01a536a47e7d45debf6b784d0d2169bb Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Fri, 15 May 2020 15:37:26 +0300 Subject: [PATCH 2/4] Fix Signature's serde impl to support owned data --- src/lib.rs | 55 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 92c9a91dc..9782ac38d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -432,15 +432,56 @@ impl ::serde::Serialize for Signature { #[cfg(feature = "serde")] impl<'de> ::serde::Deserialize<'de> for Signature { - fn deserialize>(d: D) -> Result { - use ::serde::de::Error; - use str::FromStr; + fn deserialize>(d: D) -> Result { if d.is_human_readable() { - let sl: &str = ::serde::Deserialize::deserialize(d)?; - Signature::from_str(sl).map_err(D::Error::custom) + struct HexVisitor; + + impl<'de> ::serde::de::Visitor<'de> for HexVisitor { + type Value = Signature; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a hex string representing a DER encoded Signature") + } + + fn visit_bytes(self, v: &[u8]) -> Result + where + E: ::serde::de::Error, + { + if let Ok(hex) = str::from_utf8(v) { + str::FromStr::from_str(hex).map_err(E::custom) + } else { + Err(E::invalid_value(::serde::de::Unexpected::Bytes(v), &self)) + } + } + + fn visit_str(self, v: &str) -> Result + where + E: ::serde::de::Error, + { + str::FromStr::from_str(v).map_err(E::custom) + } + } + + d.deserialize_str(HexVisitor) } else { - let sl: &[u8] = ::serde::Deserialize::deserialize(d)?; - Signature::from_der(sl).map_err(D::Error::custom) + struct BytesVisitor; + + impl<'de> ::serde::de::Visitor<'de> for BytesVisitor { + type Value = Signature; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("raw byte stream, that represents a DER encoded Signature") + } + + fn visit_bytes(self, v: &[u8]) -> Result + where + E: ::serde::de::Error, + { + Signature::from_der(v).map_err(E::custom) + } + } + + d.deserialize_bytes(BytesVisitor) } } } From 88bf5b352a9ec0159e9de85ba8b3a4ca3a417f3c Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Fri, 15 May 2020 15:44:17 +0300 Subject: [PATCH 3/4] Add more serde tests for owned data buffers --- src/ecdh.rs | 3 +-- src/key.rs | 12 ++++++++++++ src/lib.rs | 6 ++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/ecdh.rs b/src/ecdh.rs index 394abe349..b72d6b75e 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -167,7 +167,6 @@ mod tests { use rand::thread_rng; use super::SharedSecret; use super::super::Secp256k1; - use Error; #[test] fn ecdh() { @@ -187,7 +186,7 @@ mod tests { let s = Secp256k1::signing_only(); let (sk1, pk1) = s.generate_keypair(&mut thread_rng()); let (sk2, pk2) = s.generate_keypair(&mut thread_rng()); - + let sec1 = SharedSecret::new_with_hash(&pk1, &sk2, |x,_| x.into()); let sec2 = SharedSecret::new_with_hash(&pk2, &sk1, |x,_| x.into()); let sec_odd = SharedSecret::new_with_hash(&pk1, &sk1, |x,_| x.into()); diff --git a/src/key.rs b/src/key.rs index 84b6a9d5c..8166673e2 100644 --- a/src/key.rs +++ b/src/key.rs @@ -915,8 +915,20 @@ mod test { let pk = PublicKey::from_secret_key(&s, &sk); assert_tokens(&sk.compact(), &[Token::BorrowedBytes(&SK_BYTES[..])]); + assert_tokens(&sk.compact(), &[Token::Bytes(&SK_BYTES)]); + assert_tokens(&sk.compact(), &[Token::ByteBuf(&SK_BYTES)]); + assert_tokens(&sk.readable(), &[Token::BorrowedStr(SK_STR)]); + assert_tokens(&sk.readable(), &[Token::Str(SK_STR)]); + assert_tokens(&sk.readable(), &[Token::String(SK_STR)]); + assert_tokens(&pk.compact(), &[Token::BorrowedBytes(&PK_BYTES[..])]); + assert_tokens(&pk.compact(), &[Token::Bytes(&PK_BYTES)]); + assert_tokens(&pk.compact(), &[Token::ByteBuf(&PK_BYTES)]); + assert_tokens(&pk.readable(), &[Token::BorrowedStr(PK_STR)]); + assert_tokens(&pk.readable(), &[Token::Str(PK_STR)]); + assert_tokens(&pk.readable(), &[Token::String(PK_STR)]); + } } diff --git a/src/lib.rs b/src/lib.rs index 9782ac38d..44a1b3e6d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1122,7 +1122,13 @@ mod tests { "; assert_tokens(&sig.compact(), &[Token::BorrowedBytes(&SIG_BYTES[..])]); + assert_tokens(&sig.compact(), &[Token::Bytes(&SIG_BYTES)]); + assert_tokens(&sig.compact(), &[Token::ByteBuf(&SIG_BYTES)]); + assert_tokens(&sig.readable(), &[Token::BorrowedStr(SIG_STR)]); + assert_tokens(&sig.readable(), &[Token::Str(SIG_STR)]); + assert_tokens(&sig.readable(), &[Token::String(SIG_STR)]); + } // For WASM, just run through our general tests in this file all at once. From 7289facc59a28b44ad8c8b27f6a56656a15d3247 Mon Sep 17 00:00:00 2001 From: Sebastian Geisler Date: Fri, 15 May 2020 16:25:42 +0200 Subject: [PATCH 4/4] Deduplicate Visitor code --- src/key.rs | 107 ++++++---------------------------------------- src/lib.rs | 56 ++++-------------------- src/serde_util.rs | 76 ++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 140 deletions(-) create mode 100644 src/serde_util.rs diff --git a/src/key.rs b/src/key.rs index 8166673e2..ce25da96e 100644 --- a/src/key.rs +++ b/src/key.rs @@ -215,54 +215,14 @@ impl ::serde::Serialize for SecretKey { impl<'de> ::serde::Deserialize<'de> for SecretKey { fn deserialize>(d: D) -> Result { if d.is_human_readable() { - struct HexVisitor; - - impl<'de> ::serde::de::Visitor<'de> for HexVisitor { - type Value = SecretKey; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a hex string representing 32 byte SecretKey") - } - - fn visit_bytes(self, v: &[u8]) -> Result - where - E: ::serde::de::Error, - { - if let Ok(hex) = str::from_utf8(v) { - str::FromStr::from_str(hex).map_err(E::custom) - } else { - Err(E::invalid_value(::serde::de::Unexpected::Bytes(v), &self)) - } - } - - fn visit_str(self, v: &str) -> Result - where - E: ::serde::de::Error, - { - str::FromStr::from_str(v).map_err(E::custom) - } - } - - d.deserialize_str(HexVisitor) + d.deserialize_str(super::serde_util::HexVisitor::new( + "a hex string representing 32 byte SecretKey" + )) } else { - struct BytesVisitor; - - impl<'de> ::serde::de::Visitor<'de> for BytesVisitor { - type Value = SecretKey; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("raw 32 bytes SecretKey") - } - - fn visit_bytes(self, v: &[u8]) -> Result - where - E: ::serde::de::Error, - { - SecretKey::from_slice(v).map_err(E::custom) - } - } - - d.deserialize_bytes(BytesVisitor) + d.deserialize_bytes(super::serde_util::BytesVisitor::new( + "raw 32 bytes SecretKey", + SecretKey::from_slice + )) } } } @@ -459,53 +419,14 @@ impl ::serde::Serialize for PublicKey { impl<'de> ::serde::Deserialize<'de> for PublicKey { fn deserialize>(d: D) -> Result { if d.is_human_readable() { - struct HexVisitor; - - impl<'de> ::serde::de::Visitor<'de> for HexVisitor { - type Value = PublicKey; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("an ASCII hex string") - } - - fn visit_bytes(self, v: &[u8]) -> Result - where - E: ::serde::de::Error, - { - if let Ok(hex) = str::from_utf8(v) { - str::FromStr::from_str(hex).map_err(E::custom) - } else { - Err(E::invalid_value(::serde::de::Unexpected::Bytes(v), &self)) - } - } - - fn visit_str(self, v: &str) -> Result - where - E: ::serde::de::Error, - { - str::FromStr::from_str(v).map_err(E::custom) - } - } - d.deserialize_str(HexVisitor) + d.deserialize_str(super::serde_util::HexVisitor::new( + "an ASCII hex string representing a public key" + )) } else { - struct BytesVisitor; - - impl<'de> ::serde::de::Visitor<'de> for BytesVisitor { - type Value = PublicKey; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a bytestring") - } - - fn visit_bytes(self, v: &[u8]) -> Result - where - E: ::serde::de::Error, - { - PublicKey::from_slice(v).map_err(E::custom) - } - } - - d.deserialize_bytes(BytesVisitor) + d.deserialize_bytes(super::serde_util::BytesVisitor::new( + "a bytestring representing a public key", + PublicKey::from_slice + )) } } } diff --git a/src/lib.rs b/src/lib.rs index 44a1b3e6d..64c5c5095 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -165,6 +165,8 @@ pub mod ecdh; pub mod key; #[cfg(feature = "recovery")] pub mod recovery; +#[cfg(feature = "serde")] +mod serde_util; pub use key::SecretKey; pub use key::PublicKey; @@ -434,54 +436,14 @@ impl ::serde::Serialize for Signature { impl<'de> ::serde::Deserialize<'de> for Signature { fn deserialize>(d: D) -> Result { if d.is_human_readable() { - struct HexVisitor; - - impl<'de> ::serde::de::Visitor<'de> for HexVisitor { - type Value = Signature; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a hex string representing a DER encoded Signature") - } - - fn visit_bytes(self, v: &[u8]) -> Result - where - E: ::serde::de::Error, - { - if let Ok(hex) = str::from_utf8(v) { - str::FromStr::from_str(hex).map_err(E::custom) - } else { - Err(E::invalid_value(::serde::de::Unexpected::Bytes(v), &self)) - } - } - - fn visit_str(self, v: &str) -> Result - where - E: ::serde::de::Error, - { - str::FromStr::from_str(v).map_err(E::custom) - } - } - - d.deserialize_str(HexVisitor) + d.deserialize_str(serde_util::HexVisitor::new( + "a hex string representing a DER encoded Signature" + )) } else { - struct BytesVisitor; - - impl<'de> ::serde::de::Visitor<'de> for BytesVisitor { - type Value = Signature; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("raw byte stream, that represents a DER encoded Signature") - } - - fn visit_bytes(self, v: &[u8]) -> Result - where - E: ::serde::de::Error, - { - Signature::from_der(v).map_err(E::custom) - } - } - - d.deserialize_bytes(BytesVisitor) + d.deserialize_bytes(serde_util::BytesVisitor::new( + "raw byte stream, that represents a DER encoded Signature", + Signature::from_der + )) } } } diff --git a/src/serde_util.rs b/src/serde_util.rs new file mode 100644 index 000000000..50344167f --- /dev/null +++ b/src/serde_util.rs @@ -0,0 +1,76 @@ +use core::fmt; +use core::marker::PhantomData; +use core::str::{self, FromStr}; +use serde::de; + +pub struct HexVisitor { + expectation: &'static str, + _pd: PhantomData, +} + +impl HexVisitor { + pub fn new(expectation: &'static str) -> Self { + HexVisitor { + expectation, + _pd: PhantomData, + } + } +} + +impl<'de, T> de::Visitor<'de> for HexVisitor +where + T: FromStr, + ::Err: fmt::Display, +{ + type Value = T; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str(self.expectation) + } + + fn visit_bytes(self, v: &[u8]) -> Result { + if let Ok(hex) = str::from_utf8(v) { + FromStr::from_str(hex).map_err(E::custom) + } else { + Err(E::invalid_value(de::Unexpected::Bytes(v), &self)) + } + } + + fn visit_str(self, v: &str) -> Result { + FromStr::from_str(v).map_err(E::custom) + } +} + +pub struct BytesVisitor { + expectation: &'static str, + parse_fn: F, +} + +impl BytesVisitor +where + F: FnOnce(&[u8]) -> Result, + Err: fmt::Display, +{ + pub fn new(expectation: &'static str, parse_fn: F) -> Self { + BytesVisitor { + expectation, + parse_fn, + } + } +} + +impl<'de, F, T, Err> de::Visitor<'de> for BytesVisitor +where + F: FnOnce(&[u8]) -> Result, + Err: fmt::Display, +{ + type Value = T; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str(self.expectation) + } + + fn visit_bytes(self, v: &[u8]) -> Result { + (self.parse_fn)(v).map_err(E::custom) + } +}