diff --git a/radicle-node/src/bounded.rs b/radicle-node/src/bounded.rs index 01ac79c8..35b32207 100644 --- a/radicle-node/src/bounded.rs +++ b/radicle-node/src/bounded.rs @@ -18,6 +18,25 @@ impl BoundedVec { BoundedVec { v: Vec::new() } } + /// Build a `BoundedVec` by consuming from the given iterator up to its limit. + /// + /// # Examples + /// + /// ``` + /// use radicle_node::bounded; + /// + /// let mut iter = (0..4).into_iter(); + /// let bounded: bounded::BoundedVec = bounded::BoundedVec::collect_from(&mut iter); + /// + /// assert_eq!(bounded.len(), 3); + /// assert_eq!(iter.count(), 1); + /// ``` + pub fn collect_from>(iter: &mut I) -> Self { + BoundedVec { + v: iter.into_iter().take(N).collect(), + } + } + /// Create a new `BoundedVec` which takes upto the first N values of its argument, taking /// ownership. /// diff --git a/radicle-node/src/service.rs b/radicle-node/src/service.rs index f86fd33e..9b3b6aa1 100644 --- a/radicle-node/src/service.rs +++ b/radicle-node/src/service.rs @@ -73,6 +73,8 @@ pub const MAX_CONNECTION_ATTEMPTS: usize = 3; pub use message::ADDRESS_LIMIT; /// Maximum inventory limit imposed by message size limits. pub use message::INVENTORY_LIMIT; +/// Maximum number of project git references imposed by message size limits. +pub use message::REF_LIMIT; /// A service event. #[derive(Debug, Clone)] @@ -959,12 +961,22 @@ where /// Announce local refs for given id. fn announce_refs(&mut self, id: Id) -> Result<(), storage::Error> { + type Refs = BoundedVec; + let node = self.node_id(); let repo = self.storage.repository(id)?; let remote = repo.remote(&node)?; let peers = self.sessions.negotiated().map(|(_, _, p)| p); - let refs = remote.refs.into(); let timestamp = self.clock.as_secs(); + + if remote.refs.len() > Refs::max() { + log::error!( + "refs announcement limit ({}) exceeded, other nodes will see only some of your project references", + Refs::max(), + ); + } + let refs = BoundedVec::collect_from(&mut remote.refs.iter().map(|(a, b)| (a.clone(), *b))); + let msg = AnnouncementMessage::from(RefsAnnouncement { id, refs, diff --git a/radicle-node/src/service/message.rs b/radicle-node/src/service/message.rs index 7794fb81..d7d6255e 100644 --- a/radicle-node/src/service/message.rs +++ b/radicle-node/src/service/message.rs @@ -4,16 +4,18 @@ use std::{fmt, io, mem, net}; use thiserror::Error; use crate::crypto; +use crate::git; use crate::identity::Id; use crate::node; use crate::prelude::BoundedVec; use crate::service::filter::Filter; use crate::service::{NodeId, Timestamp, PROTOCOL_VERSION}; -use crate::storage::refs::Refs; use crate::wire; /// Maximum number of addresses which can be announced to other nodes. pub const ADDRESS_LIMIT: usize = 16; +/// Maximum number of project git references. +pub const REF_LIMIT: usize = 235; /// Maximum number of inventory which can be announced to other nodes. pub const INVENTORY_LIMIT: usize = 2973; @@ -225,7 +227,7 @@ pub struct RefsAnnouncement { /// Repository identifier. pub id: Id, /// Updated refs. - pub refs: Refs, + pub refs: BoundedVec<(git::RefString, git::Oid), REF_LIMIT>, /// Time of announcement. pub timestamp: Timestamp, } @@ -508,8 +510,43 @@ mod tests { use crate::crypto::test::signer::MockSigner; use crate::test::arbitrary; + use fastrand; use qcheck_macros::quickcheck; + #[test] + fn test_ref_limit() { + let mut refs = Refs::default(); + while refs.len() < REF_LIMIT { + refs.insert(arbitrary::refstring(u8::MAX as usize), arbitrary::oid()); + } + + let bounded_refs = BoundedVec::collect_from(&mut refs.iter().map(|(a, b)| (a.clone(), *b))); + let msg: Message = AnnouncementMessage::from(RefsAnnouncement { + id: arbitrary::gen(1), + refs: bounded_refs, + timestamp: LocalTime::now().as_secs(), + }) + .signed(&MockSigner::default()) + .into(); + + let mut buf: Vec = Vec::new(); + assert!( + msg.encode(&mut buf).is_ok(), + "REF_LIMIT is too big to support message encoding", + ); + + let decoded = wire::deserialize(buf.as_slice()); + assert!( + decoded.is_ok(), + "REF_LIMIT is too big to support message decoding" + ); + assert_eq!( + msg, + decoded.unwrap(), + "encoding and decoding should be safe for message at REF_LIMIT", + ); + } + #[test] fn test_inventory_limit() { let msg = Message::inventory( @@ -543,9 +580,10 @@ mod tests { fn prop_refs_announcement_signing(id: Id, refs: Refs) { let signer = MockSigner::new(&mut fastrand::Rng::new()); let timestamp = 0; + let message = AnnouncementMessage::Refs(RefsAnnouncement { id, - refs, + refs: BoundedVec::collect_from(&mut refs.iter().map(|(k, v)| (k.clone(), *v))), timestamp, }); let ann = message.signed(&signer); diff --git a/radicle-node/src/test/arbitrary.rs b/radicle-node/src/test/arbitrary.rs index 0b9e85d7..141cae08 100644 --- a/radicle-node/src/test/arbitrary.rs +++ b/radicle-node/src/test/arbitrary.rs @@ -56,7 +56,9 @@ impl Arbitrary for Message { node: NodeId::arbitrary(g), message: RefsAnnouncement { id: Id::arbitrary(g), - refs: Refs::arbitrary(g), + refs: BoundedVec::collect_from( + &mut Refs::arbitrary(g).iter().map(|(k, v)| (k.clone(), *v)), + ), timestamp: Timestamp::arbitrary(g), } .into(), diff --git a/radicle-node/src/test/peer.rs b/radicle-node/src/test/peer.rs index 1f47aa79..b2d47077 100644 --- a/radicle-node/src/test/peer.rs +++ b/radicle-node/src/test/peer.rs @@ -1,5 +1,4 @@ #![allow(dead_code)] -use std::collections::BTreeMap; use std::iter; use std::net; use std::ops::{Deref, DerefMut}; @@ -13,7 +12,7 @@ use crate::crypto::test::signer::MockSigner; use crate::crypto::Signer; use crate::identity::Id; use crate::node; -use crate::prelude::NodeId; +use crate::prelude::*; use crate::service; use crate::service::message::*; use crate::service::reactor::Io; @@ -214,7 +213,7 @@ where } pub fn refs_announcement(&self, id: Id) -> Message { - let refs = BTreeMap::new().into(); + let refs = BoundedVec::new(); let ann = AnnouncementMessage::from(RefsAnnouncement { id, refs, diff --git a/radicle-node/src/wire.rs b/radicle-node/src/wire.rs index a85f4a07..ea22b3f6 100644 --- a/radicle-node/src/wire.rs +++ b/radicle-node/src/wire.rs @@ -226,6 +226,24 @@ impl Encode for Refs { } } +impl Encode for (A, B) +where + A: Encode, + B: Encode, +{ + fn encode(&self, writer: &mut W) -> Result { + let mut n = self.0.encode(writer)?; + n += self.1.encode(writer)?; + Ok(n) + } +} + +impl Encode for git::RefString { + fn encode(&self, writer: &mut W) -> Result { + self.as_str().encode(writer) + } +} + impl Encode for Signature { fn encode(&self, writer: &mut W) -> Result { self.deref().encode(writer) @@ -266,6 +284,25 @@ impl Decode for Refs { } } +impl Decode for git::RefString { + fn decode(reader: &mut R) -> Result { + let ref_str = String::decode(reader)?; + git::RefString::try_from(ref_str).map_err(Error::from) + } +} + +impl Decode for (A, B) +where + A: Decode, + B: Decode, +{ + fn decode(reader: &mut R) -> Result { + let a = A::decode(reader)?; + let b = B::decode(reader)?; + Ok((a, b)) + } +} + impl Decode for git::Oid { fn decode(reader: &mut R) -> Result { let len = Size::decode(reader)? as usize; @@ -628,6 +665,7 @@ impl Iterator for Wire { #[cfg(test)] mod tests { use super::*; + use qcheck; use qcheck_macros::quickcheck; use crate::crypto::Unverified; @@ -700,6 +738,14 @@ mod tests { assert_eq!(deserialize::(&serialize(&input)).unwrap(), input); } + #[quickcheck] + fn prop_tuple(input: (String, String)) { + assert_eq!( + deserialize::<(String, String)>(&serialize(&input)).unwrap(), + input + ); + } + #[quickcheck] fn prop_signature(input: [u8; 64]) { let signature = Signature::from(input); diff --git a/radicle-node/src/wire/message.rs b/radicle-node/src/wire/message.rs index e4086846..56dae578 100644 --- a/radicle-node/src/wire/message.rs +++ b/radicle-node/src/wire/message.rs @@ -130,7 +130,7 @@ impl wire::Encode for RefsAnnouncement { impl wire::Decode for RefsAnnouncement { fn decode(reader: &mut R) -> Result { let id = Id::decode(reader)?; - let refs = Refs::decode(reader)?; + let refs = BoundedVec::decode(reader)?; let timestamp = Timestamp::decode(reader)?; Ok(Self { diff --git a/radicle/src/test/arbitrary.rs b/radicle/src/test/arbitrary.rs index 80f2fce5..6104e088 100644 --- a/radicle/src/test/arbitrary.rs +++ b/radicle/src/test/arbitrary.rs @@ -15,6 +15,23 @@ use crate::storage; use crate::storage::refs::{Refs, SignedRefs}; use crate::test::storage::MockStorage; +pub fn oid() -> storage::Oid { + let oid_bytes: [u8; 20] = gen(1); + storage::Oid::try_from(oid_bytes.as_slice()).unwrap() +} + +pub fn refstring(len: usize) -> git::RefString { + let mut buf = Vec::::new(); + for _ in 0..len { + buf.push(fastrand::u8(0x61..0x7a)); + } + std::str::from_utf8(&buf) + .unwrap() + .to_string() + .try_into() + .unwrap() +} + pub fn set(range: impl RangeBounds) -> HashSet { let size = fastrand::usize(range); let mut set = HashSet::with_capacity(size);