5f4e045 protocol: Fix panic when encoding large frames — heartwood

modified crates/radicle-node/src/wire.rs

@@ -427,7 +427,7 @@ where

                         stream: task.stream,
                     },
                 );
                 self.actions.push_back(Action::Send(fd, frame.to_bytes()));
                 self.actions.push_back(Action::Send(fd, frame.encode_vec()));
             }
         } else {
             // If the peer disconnected, we'll get here, but we still want to let the service know

@@ -479,7 +479,7 @@ where

                 ChannelEvent::Eof => Frame::control(*link, frame::Control::Eof { stream }),
             };
             self.actions
                 .push_back(reactor::Action::Send(fd, frame.to_bytes()));
                 .push_back(reactor::Action::Send(fd, frame.encode_vec()));
         }
     }

@@ -1126,7 +1126,7 @@ where

                     self.actions.push_back(Action::Send(
                         fd,
                         Frame::<service::Message>::control(link, frame::Control::Open { stream })
                             .to_bytes(),
                             .encode_vec(),
                     ));
                 }
             }

@@ -1264,7 +1264,7 @@ mod test {

         frame::StreamId::gossip(Link::Outbound).encode(&mut stream);
         // Serialize gossip message with some extension fields.
         let mut gossip = wire::serialize(&pong);
         let mut gossip = pong.encode_vec();
         String::from("extra").encode(&mut gossip);
 u8.encode(&mut gossip);

modified crates/radicle-protocol/src/service/gossip/store.rs

@@ -10,7 +10,7 @@ use crate::service::message::{

     Announcement, AnnouncementMessage, InventoryAnnouncement, NodeAnnouncement, RefsAnnouncement,
 };
 use crate::wire;
 use crate::wire::Decode;
 use crate::wire::{Decode as _, Encode as _};
 use radicle::node::Database;
 use radicle::node::NodeId;
 use radicle::prelude::Timestamp;

@@ -117,17 +117,17 @@ impl Store for Database {

             AnnouncementMessage::Node(msg) => {
                 stmt.bind((2, sql::Value::String(String::new())))?;
                 stmt.bind((3, &GossipType::Node))?;
                 stmt.bind((4, msg))?;
                 stmt.bind((4, &msg.encode_vec()[..]))?;
             }
             AnnouncementMessage::Refs(msg) => {
                 stmt.bind((2, &msg.rid))?;
                 stmt.bind((3, &GossipType::Refs))?;
                 stmt.bind((4, msg))?;
                 stmt.bind((4, &msg.encode_vec()[..]))?;
             }
             AnnouncementMessage::Inventory(msg) => {
                 stmt.bind((2, sql::Value::String(String::new())))?;
                 stmt.bind((3, &GossipType::Inventory))?;
                 stmt.bind((4, msg))?;
                 stmt.bind((4, &msg.encode_vec()[..]))?;
             }
         }
         stmt.bind((5, &ann.signature))?;

@@ -231,12 +231,6 @@ impl TryFrom<&sql::Value> for NodeAnnouncement {

     }
 }
 impl sql::BindableWithIndex for &NodeAnnouncement {
     fn bind<I: sql::ParameterIndex>(self, stmt: &mut sql::Statement<'_>, i: I) -> sql::Result<()> {
         wire::serialize(self).bind(stmt, i)
     }
 }
 impl TryFrom<&sql::Value> for RefsAnnouncement {
     type Error = sql::Error;

@@ -254,12 +248,6 @@ impl TryFrom<&sql::Value> for RefsAnnouncement {

     }
 }
 impl sql::BindableWithIndex for &RefsAnnouncement {
     fn bind<I: sql::ParameterIndex>(self, stmt: &mut sql::Statement<'_>, i: I) -> sql::Result<()> {
         wire::serialize(self).bind(stmt, i)
     }
 }
 impl TryFrom<&sql::Value> for InventoryAnnouncement {
     type Error = sql::Error;

@@ -277,12 +265,6 @@ impl TryFrom<&sql::Value> for InventoryAnnouncement {

     }
 }
 impl sql::BindableWithIndex for &InventoryAnnouncement {
     fn bind<I: sql::ParameterIndex>(self, stmt: &mut sql::Statement<'_>, i: I) -> sql::Result<()> {
         wire::serialize(self).bind(stmt, i)
     }
 }
 impl From<wire::Error> for sql::Error {
     fn from(other: wire::Error) -> Self {
         sql::Error {

modified crates/radicle-protocol/src/service/message.rs

@@ -16,6 +16,7 @@ use crate::bounded::BoundedVec;

 use crate::service::filter::Filter;
 use crate::service::{Link, NodeId, Timestamp};
 use crate::wire;
 use crate::wire::Encode as _;
 /// Maximum number of addresses which can be announced to other nodes.
 pub const ADDRESS_LIMIT: usize = 16;

@@ -82,7 +83,7 @@ impl NodeAnnouncement {

         let mut output = [0u8; 32];
         scrypt::scrypt(
             wire::serialize(self).as_ref(),
             &self.encode_vec(),
             Announcement::POW_SALT,
             &params,
             &mut output,

@@ -265,7 +266,8 @@ impl AnnouncementMessage {

     {
         use crypto::signature::Signer as _;
         let msg = wire::serialize(&self);
         let msg = self.encode_vec();
         let signature = signer.sign(&msg);
         Announcement {

@@ -365,7 +367,7 @@ impl Announcement {

     /// Verify this announcement's signature.
     pub fn verify(&self) -> bool {
         let msg = wire::serialize(&self.message);
         let msg = self.message.encode_vec();
         self.node.verify(msg, &self.signature).is_ok()
     }

@@ -425,8 +427,8 @@ impl PartialOrd for Message {

 impl Ord for Message {
     fn cmp(&self, other: &Self) -> std::cmp::Ordering {
         let this = wire::serialize(self);
         let other = wire::serialize(other);
         let this = self.encode_vec();
         let other = other.encode_vec();
         this.cmp(&other)
     }

@@ -678,7 +680,6 @@ mod tests {

     use radicle::git::raw;
     use super::*;
     use crate::wire::Encode;
     use localtime::LocalTime;
     use radicle::test::arbitrary;

modified crates/radicle-protocol/src/wire.rs

@@ -7,6 +7,7 @@ pub use message::{AddressType, MessageType};

 use std::collections::BTreeMap;
 use std::convert::TryFrom;
 use std::fmt::Debug;
 use std::mem;
 use std::ops::Deref;
 use std::str::FromStr;

@@ -94,7 +95,16 @@ impl From<bytes::TryGetError> for Error {

 /// Things that can be encoded as binary.
 pub trait Encode {
     /// Encode self by writing it to the given buffer.
     fn encode(&self, buffer: &mut impl BufMut);
     /// A convenience wrapper around [`Encode::encode`]
     /// that allocates a [`Vec`].
     fn encode_vec(&self) -> Vec<u8> {
         let mut buf = Vec::new();
         self.encode(&mut buf);
         buf
     }
 }
 /// Things that can be decoded from binary.

@@ -102,17 +112,6 @@ pub trait Decode: Sized {

     fn decode(buffer: &mut impl Buf) -> Result<Self, Error>;
 }
 /// Encode an object into a byte vector.
 ///
 /// # Panics
 ///
 /// If the encoded object exceeds [`Size::MAX`].
 pub fn serialize<E: Encode + ?Sized>(data: &E) -> Vec<u8> {
     let mut buffer = Vec::new().limit(Size::MAX as usize);
     data.encode(&mut buffer);
     buffer.into_inner()
 }
 /// Decode an object from a slice.
 pub fn deserialize<T: Decode>(mut data: &[u8]) -> Result<T, Error> {
     let result = T::decode(&mut data)?;

@@ -548,22 +547,22 @@ mod tests {

     #[quickcheck]
     fn prop_u8(input: u8) {
         assert_eq!(deserialize::<u8>(&serialize(&input)).unwrap(), input);
         assert_eq!(deserialize::<u8>(&input.encode_vec()).unwrap(), input);
     }
     #[quickcheck]
     fn prop_u16(input: u16) {
         assert_eq!(deserialize::<u16>(&serialize(&input)).unwrap(), input);
         assert_eq!(deserialize::<u16>(&input.encode_vec()).unwrap(), input);
     }
     #[quickcheck]
     fn prop_u32(input: u32) {
         assert_eq!(deserialize::<u32>(&serialize(&input)).unwrap(), input);
         assert_eq!(deserialize::<u32>(&input.encode_vec()).unwrap(), input);
     }
     #[quickcheck]
     fn prop_u64(input: u64) {
         assert_eq!(deserialize::<u64>(&serialize(&input)).unwrap(), input);
         assert_eq!(deserialize::<u64>(&input.encode_vec()).unwrap(), input);
     }
     #[quickcheck]

@@ -571,7 +570,7 @@ mod tests {

         if input.len() > u8::MAX as usize {
             return qcheck::TestResult::discard();
         }
         assert_eq!(deserialize::<String>(&serialize(&input)).unwrap(), input);
         assert_eq!(deserialize::<String>(&input.encode_vec()).unwrap(), input);
         qcheck::TestResult::passed()
     }

@@ -579,38 +578,41 @@ mod tests {

     #[quickcheck]
     fn prop_vec(input: BoundedVec<String, 16>) {
         assert_eq!(
             deserialize::<BoundedVec<String, 16>>(&serialize(&input.as_slice())).unwrap(),
             deserialize::<BoundedVec<String, 16>>(&input.encode_vec()).unwrap(),
             input
         );
     }
     #[quickcheck]
     fn prop_pubkey(input: PublicKey) {
         assert_eq!(deserialize::<PublicKey>(&serialize(&input)).unwrap(), input);
         assert_eq!(
             deserialize::<PublicKey>(&input.encode_vec()).unwrap(),
             input
         );
     }
     #[quickcheck]
     fn prop_filter(input: filter::Filter) {
         assert_eq!(
             deserialize::<filter::Filter>(&serialize(&input)).unwrap(),
             deserialize::<filter::Filter>(&input.encode_vec()).unwrap(),
             input
         );
     }
     #[quickcheck]
     fn prop_id(input: RepoId) {
         assert_eq!(deserialize::<RepoId>(&serialize(&input)).unwrap(), input);
         assert_eq!(deserialize::<RepoId>(&input.encode_vec()).unwrap(), input);
     }
     #[quickcheck]
     fn prop_refs(input: Refs) {
         assert_eq!(deserialize::<Refs>(&serialize(&input)).unwrap(), input);
         assert_eq!(deserialize::<Refs>(&input.encode_vec()).unwrap(), input);
     }
     #[quickcheck]
     fn prop_tuple(input: (String, String)) {
         assert_eq!(
             deserialize::<(String, String)>(&serialize(&input)).unwrap(),
             deserialize::<(String, String)>(&input.encode_vec()).unwrap(),
             input
         );
     }

@@ -620,7 +622,7 @@ mod tests {

         let signature = Signature::from(input);
         assert_eq!(
             deserialize::<Signature>(&serialize(&signature)).unwrap(),
             deserialize::<Signature>(&signature.encode_vec()).unwrap(),
             signature
         );
     }

@@ -629,13 +631,13 @@ mod tests {

     fn prop_oid(input: [u8; 20]) {
         let oid = git::Oid::try_from(input.as_slice()).unwrap();
         assert_eq!(deserialize::<git::Oid>(&serialize(&oid)).unwrap(), oid);
         assert_eq!(deserialize::<git::Oid>(&oid.encode_vec()).unwrap(), oid);
     }
     #[quickcheck]
     fn prop_signed_refs(input: SignedRefs<Unverified>) {
         assert_eq!(
             deserialize::<SignedRefs<Unverified>>(&serialize(&input)).unwrap(),
             deserialize::<SignedRefs<Unverified>>(&input.encode_vec()).unwrap(),
             input
         );
     }

@@ -643,7 +645,7 @@ mod tests {

     #[test]
     fn test_string() {
         assert_eq!(
             serialize(&String::from("hello")),
             String::from("hello").encode_vec(),
             vec![5, b'h', b'e', b'l', b'l', b'o']
         );
     }

@@ -651,7 +653,7 @@ mod tests {

     #[test]
     fn test_alias() {
         assert_eq!(
             serialize(&Alias::from_str("hello").unwrap()),
             Alias::from_str("hello").unwrap().encode_vec(),
             vec![5, b'h', b'e', b'l', b'l', b'o']
         );
     }

@@ -660,7 +662,7 @@ mod tests {

     fn test_filter_invalid() {
         let b = bloomy::BloomFilter::with_size(filter::FILTER_SIZE_M / 3);
         let f = filter::Filter::from(b);
         let bytes = serialize(&f);
         let bytes = f.encode_vec();
         assert_matches!(
             deserialize::<filter::Filter>(&bytes).unwrap_err(),

@@ -671,10 +673,10 @@ mod tests {

     #[test]
     fn test_bounded_vec_limit() {
         let v: BoundedVec<u8, 2> = vec![1, 2].try_into().unwrap();
         let buf = serialize(&v);
         let buf = &v.encode_vec();
         assert_matches!(
             deserialize::<BoundedVec<u8, 1>>(&buf),
             deserialize::<BoundedVec<u8, 1>>(buf),
             Err(Error::InvalidSize {
                 expected: 1,
                 actual: 2

@@ -683,7 +685,7 @@ mod tests {

         );
         assert!(
             deserialize::<BoundedVec<u8, 2>>(&buf).is_ok(),
             deserialize::<BoundedVec<u8, 2>>(buf).is_ok(),
             "successfully decode vector of same size",
         );
     }

modified crates/radicle-protocol/src/wire/frame.rs

@@ -234,13 +234,6 @@ impl<M> Frame<M> {

     }
 }
 impl<M: wire::Encode> Frame<M> {
     /// Serialize frame to bytes.
     pub fn to_bytes(&self) -> Vec<u8> {
         wire::serialize(self)
     }
 }
 /// Frame payload.
 #[derive(Debug, PartialEq, Eq)]
 pub enum FrameData<M> {

@@ -363,7 +356,7 @@ impl<M: wire::Encode> wire::Encode for Frame<M> {

         match &self.data {
             FrameData::Control(ctrl) => ctrl.encode(buf),
             FrameData::Git(data) => varint::payload::encode(data, buf),
             FrameData::Gossip(msg) => varint::payload::encode(&wire::serialize(msg), buf),
             FrameData::Gossip(msg) => varint::payload::encode(&msg.encode_vec(), buf),
         }
     }
 }

@@ -390,4 +383,29 @@ mod test {

         assert_eq!(StreamId::control(Link::Inbound), StreamId(VarInt(0b001)));
         assert_eq!(StreamId::gossip(Link::Inbound), StreamId(VarInt(0b011)));
     }
     #[test]
     fn test_encode_git_large() {
         use crate::wire::Encode as _;
         let size = u16::MAX as usize * 3;
         assert!(
             size > (wire::Size::MAX as usize * 2),
             "we want to test sizes that are way larger than any gossip message"
         );
         let a_lot_of_data = vec![0u8; 2 * (wire::Size::MAX as usize)];
         let frame: Frame<Message> = Frame::git(StreamId(0u8.into()), a_lot_of_data);
         // In previous versions, this would panic.
         // Such bug was introduced e.g. in 3c5668edd22ae4b9a0852
         // and fixed subsequently.
         let bytes = frame.encode_vec();
         assert!(
             bytes.len() > wire::Size::MAX as usize * 2,
             "just making sure that whatever was encoded is still quite large"
         );
     }
 }

modified crates/radicle-protocol/src/wire/message.rs

@@ -234,6 +234,8 @@ impl wire::Decode for Info {

 impl wire::Encode for Message {
     fn encode(&self, buf: &mut impl BufMut) {
         let buf = &mut buf.limit(wire::Size::MAX as usize);
         self.type_id().encode(buf);
         match self {

@@ -444,7 +446,7 @@ mod tests {

         });
         let ann = ann.signed(&signer);
         let msg = Message::Announcement(ann);
         let data = wire::serialize(&msg);
         let data = msg.encode_vec();
         assert!(data.len() < wire::Size::MAX as usize);
     }

@@ -459,7 +461,7 @@ mod tests {

         });
         let ann = ann.signed(&signer);
         let msg = Message::Announcement(ann);
         let data = wire::serialize(&msg);
         let data = msg.encode_vec();
         assert!(data.len() < wire::Size::MAX as usize);
     }

@@ -480,44 +482,48 @@ mod tests {

         });
         let ann = ann.signed(&signer);
         let msg = Message::Announcement(ann);
         let data = wire::serialize(&msg);
         let data = msg.encode_vec();
         assert!(data.len() < wire::Size::MAX as usize);
     }
     #[test]
     fn test_pingpong_encode_max_size() {
         wire::serialize(&Message::Ping(Ping {
         Message::Ping(Ping {
             ponglen: 0,
             zeroes: ZeroBytes::new(Ping::MAX_PING_ZEROES),
         }));
         })
         .encode_vec();
         wire::serialize(&Message::Pong {
         (Message::Pong {
             zeroes: ZeroBytes::new(Ping::MAX_PONG_ZEROES),
         });
         })
         .encode_vec();
     }
     #[test]
     #[should_panic(expected = "advance out of bounds")]
     fn test_ping_encode_size_overflow() {
         wire::serialize(&Message::Ping(Ping {
         Message::Ping(Ping {
             ponglen: 0,
             zeroes: ZeroBytes::new(Ping::MAX_PING_ZEROES + 1),
         }));
         })
         .encode_vec();
     }
     #[test]
     #[should_panic(expected = "advance out of bounds")]
     fn test_pong_encode_size_overflow() {
         wire::serialize(&Message::Pong {
         Message::Pong {
             zeroes: ZeroBytes::new(Ping::MAX_PONG_ZEROES + 1),
         });
         }
         .encode_vec();
     }
     #[quickcheck]
     fn prop_message_encode_decode(message: Message) {
         let encoded = &wire::serialize(&message);
         let decoded = wire::deserialize::<Message>(encoded).unwrap();
         let encoded = message.encode_vec();
         let decoded = wire::deserialize::<Message>(&encoded).unwrap();
         assert_eq!(message, decoded);
     }

@@ -550,7 +556,7 @@ mod tests {

             let zeroes = ZeroBytes::new(zeroes);
             assert_eq!(
                 wire::deserialize::<ZeroBytes>(&wire::serialize(&zeroes)).unwrap(),
                 wire::deserialize::<ZeroBytes>(&zeroes.encode_vec()).unwrap(),
                 zeroes
             );
         }

@@ -563,7 +569,7 @@ mod tests {

     #[quickcheck]
     fn prop_addr(addr: Address) {
         assert_eq!(
             wire::deserialize::<Address>(&wire::serialize(&addr)).unwrap(),
             wire::deserialize::<Address>(&addr.encode_vec()).unwrap(),
             addr
         );
     }

modified crates/radicle-protocol/src/wire/varint.rs

@@ -213,7 +213,7 @@ mod test {

     #[quickcheck]
     fn prop_encode_decode(input: VarInt) {
         let encoded = wire::serialize(&input);
         let encoded = input.encode_vec();
         let decoded: VarInt = wire::deserialize(&encoded).unwrap();
         assert_eq!(decoded, input);

@@ -222,30 +222,27 @@ mod test {

     #[test]
     #[should_panic]
     fn test_encode_overflow() {
         wire::serialize(&VarInt(u64::MAX));
         VarInt(u64::MAX).encode_vec();
     }
     #[test]
     fn test_encoding() {
         assert_eq!(wire::serialize(&VarInt(0)), vec![0x0]);
         assert_eq!(wire::serialize(&VarInt(1)), vec![0x01]);
         assert_eq!(wire::serialize(&VarInt(10)), vec![0x0a]);
         assert_eq!(wire::serialize(&VarInt(37)), vec![0x25]);
         assert_eq!(VarInt(0).encode_vec(), vec![0x0]);
         assert_eq!(VarInt(1).encode_vec(), vec![0x01]);
         assert_eq!(VarInt(10).encode_vec(), vec![0x0a]);
         assert_eq!(VarInt(37).encode_vec(), vec![0x25]);
         assert_eq!(
             wire::deserialize::<VarInt>(&[0x40, 0x25]).unwrap(),
             VarInt(37)
         );
         assert_eq!(wire::serialize(&VarInt(15293)), vec![0x7b, 0xbd]);
         assert_eq!(VarInt(15293).encode_vec(), vec![0x7b, 0xbd]);
         assert_eq!(VarInt(494878333).encode_vec(), vec![0x9d, 0x7f, 0x3e, 0x7d],);
         assert_eq!(
             wire::serialize(&VarInt(494878333)),
             vec![0x9d, 0x7f, 0x3e, 0x7d],
         );
         assert_eq!(
             wire::serialize(&VarInt(151288809941952652)),
             VarInt(151288809941952652).encode_vec(),
             vec![0xc2, 0x19, 0x7c, 0x5e, 0xff, 0x14, 0xe8, 0x8c]
         );
         assert_eq!(
             wire::serialize(&VarInt(10000000000)),
             VarInt(10000000000).encode_vec(),
             vec![0xc0, 0x00, 0x00, 0x02, 0x54, 0x0b, 0xe4, 0x00],
         );
     }