From ec4fe9b79d39fbb8bf00d4eb54f9599c7c06a317 Mon Sep 17 00:00:00 2001 From: Boog900 <54e72d8a-345f-4599-bd90-c6b9bc7d0ec5@aleeas.com> Date: Tue, 30 Jan 2024 17:53:03 +0000 Subject: [PATCH] net: fix network address parsing --- net/epee-encoding/src/container_as_blob.rs | 4 ++- net/epee-encoding/src/error.rs | 8 ++--- net/epee-encoding/src/lib.rs | 6 ++-- net/epee-encoding/src/macros.rs | 4 +-- net/epee-encoding/src/value.rs | 34 ++++++++++++++++++- net/monero-wire/src/network_address.rs | 4 +-- .../src/network_address/serde_helper.rs | 14 ++++---- net/monero-wire/src/p2p/admin.rs | 2 +- net/monero-wire/src/p2p/common.rs | 11 ++---- 9 files changed, 58 insertions(+), 29 deletions(-) diff --git a/net/epee-encoding/src/container_as_blob.rs b/net/epee-encoding/src/container_as_blob.rs index 1b1bc4b9..164acb25 100644 --- a/net/epee-encoding/src/container_as_blob.rs +++ b/net/epee-encoding/src/container_as_blob.rs @@ -33,7 +33,9 @@ impl EpeeValue for ContainerAsBlob { fn read(r: &mut B, marker: &Marker) -> Result { let bytes = Bytes::read(r, marker)?; if bytes.len() % T::SIZE != 0 { - return Err(Error::Value("Can't convert blob container to Vec type.")); + return Err(Error::Value( + "Can't convert blob container to Vec type.".to_string(), + )); } Ok(ContainerAsBlob( diff --git a/net/epee-encoding/src/error.rs b/net/epee-encoding/src/error.rs index efe517f9..e344322c 100644 --- a/net/epee-encoding/src/error.rs +++ b/net/epee-encoding/src/error.rs @@ -10,7 +10,7 @@ pub enum Error { #[cfg_attr(feature = "std", error("Format error: {0}"))] Format(&'static str), #[cfg_attr(feature = "std", error("Value error: {0}"))] - Value(&'static str), + Value(String), } impl Error { @@ -22,7 +22,7 @@ impl Error { } } - fn field_data(&self) -> &'static str { + fn field_data(&self) -> &str { match self { Error::IO(data) => data, Error::Format(data) => data, @@ -41,12 +41,12 @@ impl Debug for Error { impl From for Error { fn from(_: TryFromIntError) -> Self { - Error::Value("Int is too large") + Error::Value("Int is too large".to_string()) } } impl From for Error { fn from(_: Utf8Error) -> Self { - Error::Value("Invalid utf8 str") + Error::Value("Invalid utf8 str".to_string()) } } diff --git a/net/epee-encoding/src/lib.rs b/net/epee-encoding/src/lib.rs index 702ecc6a..f9f354da 100644 --- a/net/epee-encoding/src/lib.rs +++ b/net/epee-encoding/src/lib.rs @@ -51,7 +51,7 @@ //! //! //! let data = [1, 17, 1, 1, 1, 1, 2, 1, 1, 4, 3, 118, 97, 108, 5, 4, 0, 0, 0, 0, 0, 0, 0]; // the data to decode; -//! let val: Test = from_bytes(&mut &data).unwrap(); +//! let val: Test = from_bytes(&mut data.as_slice()).unwrap(); //! let data = to_bytes(val).unwrap(); //! //! @@ -79,7 +79,7 @@ //! //! //! let data = [1, 17, 1, 1, 1, 1, 2, 1, 1, 4, 3, 118, 97, 108, 5, 4, 0, 0, 0, 0, 0, 0, 0]; // the data to decode; -//! let val: Test2 = from_bytes(&mut &data).unwrap(); +//! let val: Test2 = from_bytes(&mut data.as_slice()).unwrap(); //! let data = to_bytes(val).unwrap(); //! //! ``` @@ -279,7 +279,7 @@ fn skip_epee_value(r: &mut B, skipped_objects: &mut u8) -> Result<()> { if let Some(size) = marker.inner_marker.size() { let bytes_to_skip = size .checked_mul(len.try_into()?) - .ok_or(Error::Value("List is too big"))?; + .ok_or(Error::Value("List is too big".to_string()))?; return advance(bytes_to_skip, r); }; diff --git a/net/epee-encoding/src/macros.rs b/net/epee-encoding/src/macros.rs index 9079a052..de89fc34 100644 --- a/net/epee-encoding/src/macros.rs +++ b/net/epee-encoding/src/macros.rs @@ -57,7 +57,7 @@ macro_rules! epee_object { if core::mem::replace(&mut self.$field, Some( epee_encoding::try_right_then_left!(epee_encoding::read_epee_value(b)?, $($read_fn(b)?)?) )).is_some() { - Err(epee_encoding::error::Error::Value("Duplicate field in data"))?; + Err(epee_encoding::error::Error::Value(format!("Duplicate field in data: {}", epee_encoding::field_name!($field, $($alt_name)?))))?; } Ok(true) },)+ @@ -87,7 +87,7 @@ macro_rules! epee_object { $(.or(Some($default)))? .or(epee_default_value) $(.map(<$ty_as>::into))? - .ok_or(epee_encoding::error::Error::Value("Missing field in data"))? + .ok_or(epee_encoding::error::Error::Value(format!("Missing field in data: {}", epee_encoding::field_name!($field, $($alt_name)?))))? }, )+ diff --git a/net/epee-encoding/src/value.rs b/net/epee-encoding/src/value.rs index 75e50b2f..540b1295 100644 --- a/net/epee-encoding/src/value.rs +++ b/net/epee-encoding/src/value.rs @@ -189,6 +189,14 @@ impl EpeeValue for Vec { Ok(res) } + fn epee_default_value() -> Option { + Some(Vec::new()) + } + + fn should_write(&self) -> bool { + !self.is_empty() + } + fn write(self, w: &mut B) -> Result<()> { write_varint(self.len().try_into()?, w)?; @@ -222,6 +230,14 @@ impl EpeeValue for Bytes { Ok(r.copy_to_bytes(len.try_into()?)) } + fn epee_default_value() -> Option { + Some(Bytes::new()) + } + + fn should_write(&self) -> bool { + !self.is_empty() + } + fn write(self, w: &mut B) -> Result<()> { write_varint(self.len().try_into()?, w)?; @@ -258,6 +274,14 @@ impl EpeeValue for BytesMut { Ok(bytes) } + fn epee_default_value() -> Option { + Some(BytesMut::new()) + } + + fn should_write(&self) -> bool { + !self.is_empty() + } + fn write(self, w: &mut B) -> Result<()> { write_varint(self.len().try_into()?, w)?; @@ -287,7 +311,7 @@ impl EpeeValue for ByteArrayVec { if r.remaining() < usize::try_from(len)? .checked_mul(N) - .ok_or(Error::Value("Length of field is too long"))? + .ok_or(Error::Value("Length of field is too long".to_string()))? { return Err(Error::IO("Not enough bytes to fill object")); } @@ -296,6 +320,14 @@ impl EpeeValue for ByteArrayVec { .map_err(|_| Error::Format("Field has invalid length")) } + fn epee_default_value() -> Option { + Some(ByteArrayVec::try_from(Bytes::new()).unwrap()) + } + + fn should_write(&self) -> bool { + !self.is_empty() + } + fn write(self, w: &mut B) -> Result<()> { let bytes = self.take_bytes(); diff --git a/net/monero-wire/src/network_address.rs b/net/monero-wire/src/network_address.rs index 29cf2486..bd4068a2 100644 --- a/net/monero-wire/src/network_address.rs +++ b/net/monero-wire/src/network_address.rs @@ -18,7 +18,7 @@ //! I2p. Currently this module only has IPv(4/6). //! use bytes::BufMut; -use epee_encoding::{EpeeObject, EpeeValue}; +use epee_encoding::EpeeObject; use std::{hash::Hash, net, net::SocketAddr}; mod serde_helper; @@ -46,7 +46,7 @@ impl EpeeObject for NetworkAddress { } fn write_fields(self, w: &mut B) -> epee_encoding::Result<()> { - TaggedNetworkAddress::from(self).write(w) + TaggedNetworkAddress::from(self).write_fields(w) } } diff --git a/net/monero-wire/src/network_address/serde_helper.rs b/net/monero-wire/src/network_address/serde_helper.rs index 13555cdc..0678b22e 100644 --- a/net/monero-wire/src/network_address/serde_helper.rs +++ b/net/monero-wire/src/network_address/serde_helper.rs @@ -14,7 +14,7 @@ pub struct TaggedNetworkAddress { epee_object!( TaggedNetworkAddress, - ty: Option, + ty("type"): Option, addr: Option, ); @@ -27,21 +27,21 @@ impl EpeeObjectBuilder for TaggedNetworkAddress { { return Err(epee_encoding::Error::Format("Duplicate field in data.")); } + Ok(true) } "addr" => { if std::mem::replace(&mut self.addr, epee_encoding::read_epee_value(b)?).is_some() { return Err(epee_encoding::Error::Format("Duplicate field in data.")); } + Ok(true) } - _ => return Ok(false), + _ => Ok(false), } - - Ok(true) } fn finish(self) -> epee_encoding::Result { self.try_into() - .map_err(|_| epee_encoding::Error::Value("Invalid network address")) + .map_err(|_| epee_encoding::Error::Value("Invalid network address".to_string())) } } @@ -70,7 +70,7 @@ impl From for TaggedNetworkAddress { addr: Some(AllFieldsNetworkAddress { m_ip: Some(u32::from_be_bytes(addr.ip().octets())), m_port: Some(addr.port()), - ..Default::default() + addr: None, }), }, SocketAddr::V6(addr) => TaggedNetworkAddress { @@ -78,7 +78,7 @@ impl From for TaggedNetworkAddress { addr: Some(AllFieldsNetworkAddress { addr: Some(addr.ip().octets()), m_port: Some(addr.port()), - ..Default::default() + m_ip: None, }), }, }, diff --git a/net/monero-wire/src/p2p/admin.rs b/net/monero-wire/src/p2p/admin.rs index c47af592..95d2f1b0 100644 --- a/net/monero-wire/src/p2p/admin.rs +++ b/net/monero-wire/src/p2p/admin.rs @@ -115,7 +115,6 @@ epee_object!( #[cfg(test)] mod tests { - use super::{BasicNodeData, CoreSyncData, HandshakeRequest, HandshakeResponse}; use crate::p2p::common::PeerSupportFlags; @@ -971,6 +970,7 @@ mod tests { assert_eq!(250, handshake.local_peerlist_new.len()); let mut encoded_bytes = epee_encoding::to_bytes(handshake.clone()).unwrap(); + let handshake_2: HandshakeResponse = epee_encoding::from_bytes(&mut encoded_bytes).unwrap(); assert_eq!(handshake, handshake_2); diff --git a/net/monero-wire/src/p2p/common.rs b/net/monero-wire/src/p2p/common.rs index 3d37c2dc..c05bfcfa 100644 --- a/net/monero-wire/src/p2p/common.rs +++ b/net/monero-wire/src/p2p/common.rs @@ -171,13 +171,6 @@ epee_object! { rpc_credits_per_hash: u32 = 0_u32, } -impl std::hash::Hash for PeerListEntryBase { - fn hash(&self, state: &mut H) { - // We only hash the adr so we can look this up in a HashSet. - self.adr.hash(state) - } -} - /// A pruned tx with the hash of the missing prunable data #[derive(Clone, Debug, PartialEq, Eq)] pub struct PrunedTxBlobEntry { @@ -256,7 +249,9 @@ fn tx_blob_read(b: &mut B) -> epee_encoding::Result { match marker.inner_marker { InnerMarker::Object => Ok(TransactionBlobs::Pruned(Vec::read(b, &marker)?)), InnerMarker::String => Ok(TransactionBlobs::Normal(Vec::read(b, &marker)?)), - _ => Err(epee_encoding::Error::Value("Invalid marker for tx blobs")), + _ => Err(epee_encoding::Error::Value( + "Invalid marker for tx blobs".to_string(), + )), } }