Change tor_bytes::Error::BadMessage to a Cow.

Actually, to avoid making a breaking change, I'm deprecating
BadMessage and creating a new InvalidMessage variant that takes a
Cow.  This way I don't need to track every crate that re-exposes
tor_bytes::Error and call this a breaking change in those.

Making this change will allow tor_bytes errors to be much more
helpful.

crates/tor-bytes/semver.md

@@ -0,0 +1 @@
+MODIFIED: Error::BadMessage is deprecated, Error::InvalidMessage is new.

crates/tor-bytes/src/err.rs

@@ -1,5 +1,7 @@
 //! Internal: Declare an Error type for tor-bytes
 
+use std::borrow::Cow;
+
 use thiserror::Error;
 use tor_error::{into_internal, Bug};
 
@@ -26,8 +28,13 @@ pub enum Error {
     BadLengthValue,
     /// An attempt to parse an object failed for some reason related to its
     /// contents.
+    #[deprecated(since = "0.6.2", note = "Use InvalidMessage instead.")]
     #[error("Bad object: {0}")]
     BadMessage(&'static str),
+    /// An attempt to parse an object failed for some reason related to its
+    /// contents.
+    #[error("Bad object: {0}")]
+    InvalidMessage(Cow<'static, str>),
     /// A parsing error that should never happen.
     ///
     /// We use this one in lieu of calling assert() and expect() and
@@ -42,7 +49,9 @@ impl PartialEq for Error {
         match (self, other) {
             (Truncated, Truncated) => true,
             (ExtraneousBytes, ExtraneousBytes) => true,
+            #[allow(deprecated)]
             (BadMessage(a), BadMessage(b)) => a == b,
+            (InvalidMessage(a), InvalidMessage(b)) => a == b,
             (BadLengthValue, BadLengthValue) => true,
             // notably, this means that an internal error is equal to nothing, not even itself.
             (_, _) => false,

crates/tor-bytes/src/impls.rs

@@ -182,7 +182,7 @@ mod ed25519_impls {
         fn take_from(b: &mut Reader<'_>) -> Result<Self> {
             let bytes = b.take(32)?;
             Self::from_bytes(array_ref![bytes, 0, 32])
-                .map_err(|_| Error::BadMessage("Couldn't decode Ed25519 public key"))
+                .map_err(|_| Error::InvalidMessage("Couldn't decode Ed25519 public key".into()))
         }
     }
 
@@ -208,7 +208,7 @@ mod ed25519_impls {
         fn take_from(b: &mut Reader<'_>) -> Result<Self> {
             let bytes = b.take(64)?;
             Self::from_bytes(array_ref![bytes, 0, 64])
-                .map_err(|_| Error::BadMessage("Couldn't decode Ed25519 signature."))
+                .map_err(|_| Error::InvalidMessage("Couldn't decode Ed25519 signature.".into()))
         }
     }
 }

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

@@ -128,7 +128,9 @@ impl Body for Vpadding {
 impl Readable for Vpadding {
     fn take_from(r: &mut Reader<'_>) -> Result<Self> {
         if r.remaining() > std::u16::MAX as usize {
-            return Err(Error::BadMessage("Too many bytes in VPADDING cell"));
+            return Err(Error::InvalidMessage(
+                "Too many bytes in VPADDING cell".into(),
+            ));
         }
         Ok(Vpadding {
             len: r.remaining() as u16,
@@ -821,8 +823,8 @@ impl Readable for PaddingNegotiate {
     fn take_from(r: &mut Reader<'_>) -> Result<Self> {
         let v = r.take_u8()?;
         if v != 0 {
-            return Err(Error::BadMessage(
-                "Unrecognized padding negotiation version",
+            return Err(Error::InvalidMessage(
+                "Unrecognized padding negotiation version".into(),
             ));
         }
         let command = r.take_u8()?.into();

crates/tor-cell/src/relaycell.rs

@@ -293,7 +293,9 @@ impl<M: RelayMsg> RelayCell<M> {
         r.advance(4)?; // digest
         let len = r.take_u16()? as usize;
         if r.remaining() < len {
-            return Err(Error::BadMessage("Insufficient data in relay cell"));
+            return Err(Error::InvalidMessage(
+                "Insufficient data in relay cell".into(),
+            ));
         }
         r.truncate(len);
         let msg = M::decode_from_reader(cmd, r)?;

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

@@ -91,16 +91,16 @@ impl Readable for NtorV3Extension {
         Ok(match tag {
             NtorV3ExtensionType::CC_REQUEST => {
                 if len != 0 {
-                    return Err(tor_bytes::Error::BadMessage(
-                        "invalid length for RequestCongestionControl",
+                    return Err(tor_bytes::Error::InvalidMessage(
+                        "invalid length for RequestCongestionControl".into(),
                     ));
                 }
                 NtorV3Extension::RequestCongestionControl
             }
             NtorV3ExtensionType::CC_RESPONSE => {
                 if len != 1 {
-                    return Err(tor_bytes::Error::BadMessage(
-                        "invalid length for AckCongestionControl",
+                    return Err(tor_bytes::Error::InvalidMessage(
+                        "invalid length for AckCongestionControl".into(),
                     ));
                 }
                 let sendme_inc = reader.take_u8()?;

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

@@ -210,7 +210,7 @@ impl Body for Begin {
                 let a = r.take_until(b']')?;
                 let colon = r.take_u8()?;
                 if colon != b':' {
-                    return Err(Error::BadMessage("missing port in begin cell"));
+                    return Err(Error::InvalidMessage("missing port in begin cell".into()));
                 }
                 a
             } else {
@@ -222,15 +222,17 @@ impl Body for Begin {
         let flags = if r.remaining() >= 4 { r.take_u32()? } else { 0 };
 
         if !addr.is_ascii() {
-            return Err(Error::BadMessage("target address in begin cell not ascii"));
+            return Err(Error::InvalidMessage(
+                "target address in begin cell not ascii".into(),
+            ));
         }
 
         let port = std::str::from_utf8(port)
-            .map_err(|_| Error::BadMessage("port in begin cell not utf8"))?;
+            .map_err(|_| Error::InvalidMessage("port in begin cell not utf8".into()))?;
 
         let port = port
             .parse()
-            .map_err(|_| Error::BadMessage("port in begin cell not a valid port"))?;
+            .map_err(|_| Error::InvalidMessage("port in begin cell not a valid port".into()))?;
 
         Ok(Begin {
             addr: addr.into(),
@@ -519,7 +521,9 @@ impl Body for Connected {
         let ipv4 = r.take_u32()?;
         let addr = if ipv4 == 0 {
             if r.take_u8()? != 6 {
-                return Err(Error::BadMessage("Invalid address type in CONNECTED cell"));
+                return Err(Error::InvalidMessage(
+                    "Invalid address type in CONNECTED cell".into(),
+                ));
             }
             IpAddr::V6(r.extract()?)
         } else {
@@ -600,7 +604,7 @@ impl Body for Sendme {
                     Some(r.take(dlen as usize)?.into())
                 }
                 _ => {
-                    return Err(Error::BadMessage("Unrecognized SENDME version."));
+                    return Err(Error::InvalidMessage("Unrecognized SENDME version.".into()));
                 }
             }
         };
@@ -955,7 +959,9 @@ impl Readable for ResolvedVal {
         let len = r.take_u8()? as usize;
         if let Some(expected_len) = res_len(tp) {
             if len != expected_len {
-                return Err(Error::BadMessage("Wrong length for RESOLVED answer"));
+                return Err(Error::InvalidMessage(
+                    "Wrong length for RESOLVED answer".into(),
+                ));
             }
         }
         Ok(match tp {

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

@@ -331,7 +331,9 @@ impl Introduce {
     fn decode_from_reader(r: &mut Reader<'_>) -> Result<Self> {
         let legacy_key_id: RsaIdentity = r.extract()?;
         if !legacy_key_id.is_zero() {
-            return Err(BytesError::BadMessage("legacy key id in Introduce1."));
+            return Err(BytesError::InvalidMessage(
+                "legacy key id in Introduce1.".into(),
+            ));
         }
         let auth_key_type = r.take_u8()?.into();
         let auth_key_len = r.take_u16()?;

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

@@ -98,7 +98,7 @@ impl Readable for Address {
                 }
                 T_IPV4 => Self::Ipv4(r.extract()?),
                 T_IPV6 => Self::Ipv6(r.extract()?),
-                _ => return Err(Error::BadMessage("Invalid address type")),
+                _ => return Err(Error::InvalidMessage("Invalid address type".into())),
             })
         })
     }
@@ -133,10 +133,10 @@ impl FromStr for Address {
             Ok(Self::Ipv6(ipv6))
         } else {
             if s.len() > MAX_HOSTNAME_LEN {
-                return Err(Error::BadMessage("Hostname too long"));
+                return Err(Error::InvalidMessage("Hostname too long".into()));
             }
             if s.contains('\0') {
-                return Err(Error::BadMessage("Nul byte not permitted"));
+                return Err(Error::InvalidMessage("Nul byte not permitted".into()));
             }
 
             let mut addr = s.to_string();
@@ -229,11 +229,11 @@ impl msg::Body for ConnectedUdp {
     fn decode_from_reader(r: &mut Reader<'_>) -> Result<Self> {
         let our_address: AddressPort = r.extract()?;
         if our_address.addr.is_hostname() {
-            return Err(Error::BadMessage("Our address is a Hostname"));
+            return Err(Error::InvalidMessage("Our address is a Hostname".into()));
         }
         let their_address: AddressPort = r.extract()?;
         if their_address.addr.is_hostname() {
-            return Err(Error::BadMessage("Their address is a Hostname"));
+            return Err(Error::InvalidMessage("Their address is a Hostname".into()));
         }
 
         Ok(Self {

crates/tor-cell/src/restrict.rs

@@ -158,7 +158,7 @@ macro_rules! restricted_msg {
                     )?
                     // TODO: This message is too terse! This message type should maybe take a Cow?
                     #[allow(unreachable_patterns)] // This is unreachable if we had an Unrecognized variant above.
-                    _ => return Err($crate::restrict::tor_bytes::Error::BadMessage("Unexpected command")),
+                    _ => return Err($crate::restrict::tor_bytes::Error::InvalidMessage("Unexpected command".into())),
                 })
             }
         }

crates/tor-cell/tests/test_relaycell.rs

@@ -91,7 +91,9 @@ fn test_cells() {
     let m = decode("02 0000 9999 12345678 01f3 6e6565642d746f2d6b6e6f77 00000000");
     assert_eq!(
         AnyRelayCell::decode(m).err(),
-        Some(Error::BadMessage("Insufficient data in relay cell"))
+        Some(Error::InvalidMessage(
+            "Insufficient data in relay cell".into()
+        ))
     );
 
     // check accessors.
@@ -173,7 +175,10 @@ fn test_address() {
     let hostname = "a".repeat(256);
     let addr = Address::from_str(hostname.as_str());
     assert!(addr.is_err());
-    assert_eq!(addr.err(), Some(Error::BadMessage("Hostname too long")));
+    assert_eq!(
+        addr.err(),
+        Some(Error::InvalidMessage("Hostname too long".into()))
+    );
 
     // Some Unicode emojis (go Gen-Z!).
     let hostname = "👍️👍️👍️";
@@ -187,6 +192,6 @@ fn test_address() {
     assert!(addr.is_err());
     assert_eq!(
         addr.err(),
-        Some(Error::BadMessage("Nul byte not permitted"))
+        Some(Error::InvalidMessage("Nul byte not permitted".into()))
     );
 }

crates/tor-cell/tests/testvec_chanmsg.rs

@@ -406,6 +406,6 @@ fn test_padding_negotiate() {
 
     assert_eq!(
         decode_err(cmd, "90 0303", true),
-        BytesError::BadMessage("Unrecognized padding negotiation version")
+        BytesError::InvalidMessage("Unrecognized padding negotiation version".into())
     );
 }

crates/tor-cell/tests/testvec_relaymsg.rs

@@ -101,14 +101,14 @@ fn test_begin() {
     msg_error(
         cmd,
         "5b3a3a5d21", // [::]!
-        BytesError::BadMessage("missing port in begin cell"),
+        BytesError::InvalidMessage("missing port in begin cell".into()),
     );
 
     // hand-generated failure case: not ascii.
     msg_error(
         cmd,
         "746f7270726f6a656374e284a22e6f72673a34343300", // torproject™.org:443
-        BytesError::BadMessage("target address in begin cell not ascii"),
+        BytesError::InvalidMessage("target address in begin cell not ascii".into()),
     );
 
     // failure on construction: bad address.
@@ -151,7 +151,7 @@ fn test_connected() {
     msg_error(
         cmd,
         "00000000 07 20010db8 00000000 00000000 00001122 00000E10",
-        BytesError::BadMessage("Invalid address type in CONNECTED cell"),
+        BytesError::InvalidMessage("Invalid address type in CONNECTED cell".into()),
     );
 }
 
@@ -418,7 +418,7 @@ fn test_resolved() {
     msg_error(
         cmd,
         "04 03 010203 00000001",
-        BytesError::BadMessage("Wrong length for RESOLVED answer"),
+        BytesError::InvalidMessage("Wrong length for RESOLVED answer".into()),
     );
 }
 
@@ -564,7 +564,7 @@ fn test_connect_udp() {
     msg_error(
         cmd,
         "00000000 07 04 01020304",
-        BytesError::BadMessage("Invalid address type"),
+        BytesError::InvalidMessage("Invalid address type".into()),
     );
 
     // A zero length address with and without hostname payload.
@@ -608,14 +608,14 @@ fn test_connected_udp() {
         cmd,
         "01 04 01020304 0050
          04 04 05060708 0050",
-        BytesError::BadMessage("Our address is a Hostname"),
+        BytesError::InvalidMessage("Our address is a Hostname".into()),
     );
     // Invalid their_address
     msg_error(
         cmd,
         "04 04 01020304 0050
          01 04 05060708 0050",
-        BytesError::BadMessage("Their address is a Hostname"),
+        BytesError::InvalidMessage("Their address is a Hostname".into()),
     );
 }
 
@@ -773,7 +773,7 @@ fn test_introduce() {
          02 0004 00010203
          00
          01090804",
-        BytesError::BadMessage("legacy key id in Introduce1."),
+        BytesError::InvalidMessage("legacy key id in Introduce1.".into()),
     );
 }
 // TODO: need to add tests for:

crates/tor-cert/src/lib.rs

@@ -301,13 +301,15 @@ impl Readable for CertExt {
 
         Ok(match ext_type {
             ExtType::SIGNED_WITH_ED25519_KEY => CertExt::SignedWithEd25519(SignedWithEd25519Ext {
-                pk: ed25519::Ed25519Identity::from_bytes(body)
-                    .ok_or(BytesError::BadMessage("wrong length on Ed25519 key"))?,
+                pk: ed25519::Ed25519Identity::from_bytes(body).ok_or(
+                    BytesError::InvalidMessage("wrong length on Ed25519 key".into()),
+                )?,
             }),
             _ => {
                 if (flags & 1) != 0 {
-                    return Err(BytesError::BadMessage(
-                        "unrecognized certificate extension, with 'affects_validation' flag set.",
+                    return Err(BytesError::InvalidMessage(
+                        "unrecognized certificate extension, with 'affects_validation' flag set."
+                            .into(),
                     ));
                 }
                 CertExt::Unrecognized(UnrecognizedExt {
@@ -335,7 +337,9 @@ impl Ed25519Cert {
         if v != 1 {
             // This would be something other than a "v1" certificate. We don't
             // understand those.
-            return Err(BytesError::BadMessage("Unrecognized certificate version"));
+            return Err(BytesError::InvalidMessage(
+                "Unrecognized certificate version".into(),
+            ));
         }
         let cert_type = r.take_u8()?.into();
         let exp_hours = r.take_u32()?;
@@ -591,8 +595,8 @@ mod test {
         assert!(e.is_err());
         assert_eq!(
             e.err().unwrap(),
-            BytesError::BadMessage(
-                "unrecognized certificate extension, with 'affects_validation' flag set."
+            BytesError::InvalidMessage(
+                "unrecognized certificate extension, with 'affects_validation' flag set.".into()
             )
         );
 

crates/tor-cert/src/rsa.rs

@@ -82,7 +82,9 @@ impl ExternallySigned<TimerangeBound<RsaCrosscert>> for UncheckedRsaCrosscert {
     fn is_well_signed(&self, k: &Self::Key) -> Result<(), Self::Error> {
         k.verify(&self.0.digest[..], &self.0.signature[..])
             .map_err(|_| {
-                tor_bytes::Error::BadMessage("Invalid signature on RSA->Ed identity crosscert")
+                tor_bytes::Error::InvalidMessage(
+                    "Invalid signature on RSA->Ed identity crosscert".into(),
+                )
             })?;
         Ok(())
     }

crates/tor-cert/tests/invalid_certs.rs

@@ -17,7 +17,7 @@ fn cant_parse() {
 
     assert_eq!(
         decode_err(&hex!("03")),
-        Error::BadMessage("Unrecognized certificate version")
+        Error::InvalidMessage("Unrecognized certificate version".into())
     );
 
     assert_eq!(
@@ -30,7 +30,7 @@ fn cant_parse() {
        FF1A5203FA27F86EF7528D89A0845D2520166E340754FFEA2AAE0F612B7CE5DA
        094A0236CDAC45034B0B6842C18E7F6B51B93A3CF7E60663B8AD061C30A62602"
         )),
-        Error::BadMessage("wrong length on Ed25519 key")
+        Error::InvalidMessage("wrong length on Ed25519 key".into())
     );
 
     assert_eq!(
@@ -43,8 +43,8 @@ fn cant_parse() {
        FF1A5203FA27F86EF7528D89A0845D2520166E340754FFEA2AAE0F612B7CE5DA
        094A0236CDAC45034B0B6842C18E7F6B51B93A3CF7E60663B8AD061C30A62602"
         )),
-        Error::BadMessage(
-            "unrecognized certificate extension, with 'affects_validation' flag set."
+        Error::InvalidMessage(
+            "unrecognized certificate extension, with 'affects_validation' flag set.".into()
         )
     );
 }

crates/tor-socksproto/src/handshake.rs

@@ -41,18 +41,20 @@ impl Readable for SocksAddr {
                 let hlen = r.take_u8()?;
                 let hostname = r.take(hlen as usize)?;
                 let hostname = std::str::from_utf8(hostname)
-                    .map_err(|_| BytesError::BadMessage("bad utf8 on hostname"))?
+                    .map_err(|_| BytesError::InvalidMessage("bad utf8 on hostname".into()))?
                     .to_string();
                 let hostname = hostname
                     .try_into()
-                    .map_err(|_| BytesError::BadMessage("hostname too long"))?;
+                    .map_err(|_| BytesError::InvalidMessage("hostname too long".into()))?;
                 Ok(SocksAddr::Hostname(hostname))
             }
             4 => {
                 let ip6: std::net::Ipv6Addr = r.extract()?;
                 Ok(SocksAddr::Ip(ip6.into()))
             }
-            _ => Err(BytesError::BadMessage("unrecognized address type.")),
+            _ => Err(BytesError::InvalidMessage(
+                "unrecognized address type.".into(),
+            )),
         }
     }
 }

crates/tor-socksproto/src/handshake/proxy.rs

@@ -124,7 +124,7 @@ impl SocksProxyHandshake {
                 .to_string();
             let hostname = hostname
                 .try_into()
-                .map_err(|_| BytesError::BadMessage("hostname too long"))?;
+                .map_err(|_| BytesError::InvalidMessage("hostname too long".into()))?;
             SocksAddr::Hostname(hostname)
         } else {
             let ip4: std::net::Ipv4Addr = ip.into();