From c631b45ab64c3707893f1ec7e42f381e52192d63 Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Tue, 13 Nov 2018 09:30:40 +0000 Subject: [PATCH] cleanup how we deal with flexible rproof size in ser/deser (#1965) --- core/src/ser.rs | 40 +++++++++++++++++------------------ p2p/src/msg.rs | 7 +++--- p2p/src/store.rs | 7 ++++-- wallet/src/libwallet/types.rs | 8 +++---- 4 files changed, 33 insertions(+), 29 deletions(-) diff --git a/core/src/ser.rs b/core/src/ser.rs index a9cd8c40..4a4648f0 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -184,10 +184,8 @@ pub trait Reader { fn read_i32(&mut self) -> Result; /// Read a i64 from the underlying Read fn read_i64(&mut self) -> Result; - /// first before the data bytes. - fn read_vec(&mut self) -> Result, Error>; - /// first before the data bytes limited to max bytes. - fn read_limited_vec(&mut self, max: usize) -> Result, Error>; + /// Read a u64 len prefix followed by that number of exact bytes. + fn read_bytes_len_prefix(&mut self) -> Result, Error>; /// Read a fixed number of bytes from the underlying reader. fn read_fixed_bytes(&mut self, length: usize) -> Result, Error>; /// Consumes a byte from the reader, producing an error if it doesn't have @@ -281,16 +279,11 @@ impl<'a> Reader for BinReader<'a> { self.source.read_i64::().map_err(map_io_err) } /// Read a variable size vector from the underlying Read. Expects a usize - fn read_vec(&mut self) -> Result, Error> { + fn read_bytes_len_prefix(&mut self) -> Result, Error> { let len = self.read_u64()?; self.read_fixed_bytes(len as usize) } - /// Read limited variable size vector from the underlying Read. Expects a - /// usize - fn read_limited_vec(&mut self, max: usize) -> Result, Error> { - let len = cmp::min(max, self.read_u64()? as usize); - self.read_fixed_bytes(len as usize) - } + /// Read a fixed number of bytes. fn read_fixed_bytes(&mut self, length: usize) -> Result, Error> { // not reading more than 100k in a single read if length > 100000 { @@ -369,27 +362,30 @@ impl Writeable for RangeProof { impl Readable for RangeProof { fn read(reader: &mut Reader) -> Result { - let p = reader.read_limited_vec(MAX_PROOF_SIZE)?; - let mut a = [0; MAX_PROOF_SIZE]; - a[..p.len()].clone_from_slice(&p[..]); + let len = reader.read_u64()?; + let max_len = cmp::min(len as usize, MAX_PROOF_SIZE); + let p = reader.read_fixed_bytes(max_len)?; + let mut proof = [0; MAX_PROOF_SIZE]; + proof[..p.len()].clone_from_slice(&p[..]); Ok(RangeProof { - proof: a, - plen: p.len(), + plen: proof.len(), + proof, }) } } impl FixedLength for RangeProof { - const LEN: usize = MAX_PROOF_SIZE + 8; + const LEN: usize = 8 // length prefix + + MAX_PROOF_SIZE; } impl PMMRable for RangeProof {} impl Readable for Signature { fn read(reader: &mut Reader) -> Result { - let a = reader.read_fixed_bytes(AGG_SIGNATURE_SIZE)?; - let mut c = [0; AGG_SIGNATURE_SIZE]; - c[..AGG_SIGNATURE_SIZE].clone_from_slice(&a[..AGG_SIGNATURE_SIZE]); + let a = reader.read_fixed_bytes(Signature::LEN)?; + let mut c = [0; Signature::LEN]; + c[..Signature::LEN].clone_from_slice(&a[..Signature::LEN]); Ok(Signature::from_raw_data(&c).unwrap()) } } @@ -400,6 +396,10 @@ impl Writeable for Signature { } } +impl FixedLength for Signature { + const LEN: usize = AGG_SIGNATURE_SIZE; +} + /// Utility wrapper for an underlying byte Writer. Defines higher level methods /// to write numbers, byte vectors, hashes, etc. pub struct BinWriter<'a> { diff --git a/p2p/src/msg.rs b/p2p/src/msg.rs index 7f3ee86e..7da0ef26 100644 --- a/p2p/src/msg.rs +++ b/p2p/src/msg.rs @@ -362,7 +362,7 @@ impl Readable for Hand { let total_diff = Difficulty::read(reader)?; let sender_addr = SockAddr::read(reader)?; let receiver_addr = SockAddr::read(reader)?; - let ua = reader.read_vec()?; + let ua = reader.read_bytes_len_prefix()?; let user_agent = String::from_utf8(ua).map_err(|_| ser::Error::CorruptedData)?; let genesis = Hash::read(reader)?; Ok(Hand { @@ -413,7 +413,7 @@ impl Readable for Shake { let (version, capab) = ser_multiread!(reader, read_u32, read_u32); let capabilities = Capabilities::from_bits_truncate(capab); let total_diff = Difficulty::read(reader)?; - let ua = reader.read_vec()?; + let ua = reader.read_bytes_len_prefix()?; let user_agent = String::from_utf8(ua).map_err(|_| ser::Error::CorruptedData)?; let genesis = Hash::read(reader)?; Ok(Shake { @@ -498,7 +498,8 @@ impl Writeable for PeerError { impl Readable for PeerError { fn read(reader: &mut Reader) -> Result { - let (code, msg) = ser_multiread!(reader, read_u32, read_vec); + let code = reader.read_u32()?; + let msg = reader.read_bytes_len_prefix()?; let message = String::from_utf8(msg).map_err(|_| ser::Error::CorruptedData)?; Ok(PeerError { code: code, diff --git a/p2p/src/store.rs b/p2p/src/store.rs index 6d0d305b..30016378 100644 --- a/p2p/src/store.rs +++ b/p2p/src/store.rs @@ -80,8 +80,10 @@ impl Writeable for PeerData { impl Readable for PeerData { fn read(reader: &mut Reader) -> Result { let addr = SockAddr::read(reader)?; - let (capab, ua, fl, lb, br) = - ser_multiread!(reader, read_u32, read_vec, read_u8, read_i64, read_i32); + let capab = reader.read_u32()?; + let ua = reader.read_bytes_len_prefix()?; + let (fl, lb, br) = ser_multiread!(reader, read_u8, read_i64, read_i32); + let lc = reader.read_i64(); // this only works because each PeerData is read in its own vector and this // is the last data element @@ -90,6 +92,7 @@ impl Readable for PeerData { } else { lc.unwrap() }; + let user_agent = String::from_utf8(ua).map_err(|_| ser::Error::CorruptedData)?; let capabilities = Capabilities::from_bits_truncate(capab); let ban_reason = ReasonForBan::from_i32(br).ok_or(ser::Error::CorruptedData)?; diff --git a/wallet/src/libwallet/types.rs b/wallet/src/libwallet/types.rs index 3203fc86..e0002665 100644 --- a/wallet/src/libwallet/types.rs +++ b/wallet/src/libwallet/types.rs @@ -262,7 +262,7 @@ impl ser::Writeable for OutputData { impl ser::Readable for OutputData { fn read(reader: &mut ser::Reader) -> Result { - let data = reader.read_vec()?; + let data = reader.read_bytes_len_prefix()?; serde_json::from_slice(&data[..]).map_err(|_| ser::Error::CorruptedData) } } @@ -431,7 +431,7 @@ impl ser::Writeable for Context { impl ser::Readable for Context { fn read(reader: &mut ser::Reader) -> Result { - let data = reader.read_vec()?; + let data = reader.read_bytes_len_prefix()?; serde_json::from_slice(&data[..]).map_err(|_| ser::Error::CorruptedData) } } @@ -615,7 +615,7 @@ impl ser::Writeable for TxLogEntry { impl ser::Readable for TxLogEntry { fn read(reader: &mut ser::Reader) -> Result { - let data = reader.read_vec()?; + let data = reader.read_bytes_len_prefix()?; serde_json::from_slice(&data[..]).map_err(|_| ser::Error::CorruptedData) } } @@ -663,7 +663,7 @@ impl ser::Writeable for AcctPathMapping { impl ser::Readable for AcctPathMapping { fn read(reader: &mut ser::Reader) -> Result { - let data = reader.read_vec()?; + let data = reader.read_bytes_len_prefix()?; serde_json::from_slice(&data[..]).map_err(|_| ser::Error::CorruptedData) } }