tor-cell: error usefulness and style fixes

crates/tor-bytes/semver.md

@@ -0,0 +1 @@
+BREAKING: new fields in error variants

crates/tor-cell/src/chancell/codec.rs

@@ -81,6 +81,11 @@ impl ChannelCodec {
     /// On a definite decoding error, return Err(_).  On a cell that might
     /// just be truncated, return Ok(None).
     pub fn decode_cell(&mut self, src: &mut BytesMut) -> crate::Result<Option<ChanCell>> {
+        /// Wrap `be` as an appropriate type.
+        fn wrap_err(be: tor_bytes::Error) -> crate::Error {
+            crate::Error::BytesErr(be, "channel cell")
+        }
+
         if src.len() < 7 {
             // Smallest possible command: varcell with len 0
             return Ok(None);
@@ -100,9 +105,9 @@ impl ChannelCodec {
         let cell = src.split_to(cell_len).freeze();
         //trace!("{:?} cell body ({}) is {:?}", cmd, cell.len(), &cell[..]);
         let mut r = Reader::from_bytes(&cell);
-        let circid: CircId = r.take_u32()?.into();
-        r.advance(if varcell { 3 } else { 1 })?;
-        let msg = msg::ChanMsg::take(&mut r, cmd)?;
+        let circid: CircId = r.take_u32().map_err(wrap_err)?.into();
+        r.advance(if varcell { 3 } else { 1 }).map_err(wrap_err)?;
+        let msg = msg::ChanMsg::take(&mut r, cmd).map_err(wrap_err)?;
 
         if !cmd.accepts_circid_val(circid) {
             return Err(Error::ChanProto(format!(

crates/tor-cell/src/chancell/msg.rs

@@ -731,7 +731,7 @@ impl Versions {
         if versions.len() < (std::u16::MAX / 2) as usize {
             Ok(Self { versions })
         } else {
-            Err(crate::Error::CantEncode)
+            Err(crate::Error::CantEncode("Too many versions"))
         }
     }
     /// Encode this VERSIONS cell in the manner expected for a handshake.
@@ -919,7 +919,8 @@ impl Certs {
             .cert_body(tp)
             .ok_or_else(|| crate::Error::ChanProto(format!("Missing {} certificate", tp)))?;
 
-        let cert = tor_cert::Ed25519Cert::decode(body)?;
+        let cert = tor_cert::Ed25519Cert::decode(body)
+            .map_err(|be| crate::Error::BytesErr(be, "ed25519 certificate"))?;
         if cert.peek_cert_type() != tp {
             return Err(crate::Error::ChanProto(format!(
                 "Found a {} certificate labeled as {}",

crates/tor-cell/src/err.rs

@@ -12,20 +12,20 @@ use tor_error::{ErrorKind, HasKind};
 pub enum Error {
     /// An error that occurred in the tor_bytes crate while decoding an
     /// object.
-    #[error("parsing error")]
-    BytesErr(#[from] tor_bytes::Error),
+    #[error("Error while parsing {1}")]
+    BytesErr(#[source] tor_bytes::Error, &'static str),
     /// There was a programming error somewhere in the code.
     #[error("Internal programming error")]
     Internal(tor_error::Bug),
     /// Protocol violation at the channel level
-    #[error("channel protocol violation")]
+    #[error("Channel protocol violation: {0}")]
     ChanProto(String),
     /// Tried to make or use a stream to an invalid destination address.
-    #[error("invalid stream target address")]
+    #[error("Invalid stream target address")]
     BadStreamAddress,
     /// Tried to construct a message that Tor can't represent.
-    #[error("Message can't be represented in a Tor cell.")]
-    CantEncode,
+    #[error("Message can't be represented in a Tor cell: {0}")]
+    CantEncode(&'static str),
 }
 
 impl HasKind for Error {
@@ -34,12 +34,12 @@ impl HasKind for Error {
         use Error as E;
         use ErrorKind as EK;
         match self {
-            E::BytesErr(ByE::Truncated) => EK::Internal,
-            E::BytesErr(_) => EK::TorProtocolViolation,
+            E::BytesErr(ByE::Truncated, _) => EK::Internal,
+            E::BytesErr(_, _) => EK::TorProtocolViolation,
             E::Internal(_) => EK::Internal,
             E::ChanProto(_) => EK::TorProtocolViolation,
             E::BadStreamAddress => EK::BadApiUsage,
-            E::CantEncode => EK::Internal,
+            E::CantEncode(_) => EK::Internal,
         }
     }
 }

crates/tor-cell/src/relaycell/msg.rs

@@ -338,7 +338,7 @@ impl Data {
     /// Returns an error if `inp` is longer than [`Data::MAXLEN`] bytes.
     pub fn new(inp: &[u8]) -> crate::Result<Self> {
         if inp.len() > Data::MAXLEN {
-            return Err(crate::Error::CantEncode);
+            return Err(crate::Error::CantEncode("Data message too long"));
         }
         Ok(Self::new_unchecked(inp.into()))
     }

crates/tor-cell/src/relaycell/udp.rs

@@ -178,7 +178,9 @@ impl ConnectUdp {
         F: Into<msg::BeginFlags>,
     {
         Ok(Self {
-            addr: (addr, port).try_into()?,
+            addr: (addr, port)
+                .try_into()
+                .map_err(|_| crate::Error::BadStreamAddress)?,
             flags: flags.into(),
         })
     }
@@ -274,7 +276,7 @@ impl Datagram {
     /// Returns an error if `inp` is longer than [`Datagram::MAXLEN`] bytes.
     pub fn new(inp: &[u8]) -> crate::Result<Self> {
         if inp.len() > msg::Data::MAXLEN {
-            return Err(crate::Error::CantEncode);
+            return Err(crate::Error::CantEncode("Datagram too long"));
         }
         Ok(Self::new_unchecked(inp.into()))
     }